[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] xl: Return error when no userdata exists
Ian Jackson wrote: > Jim Fehlig writes ("[Xen-devel] [PATCH] xl: Return error when no userdata > exists"): > >> xl: Return error when no userdata exists >> >> The libvirt libxenlight driver will store its own userdata with >> id 'libvirt-xml', but currently libxl_userdata_retrieve() does >> not fail on non-existent userdata due to inverted error check. >> > > Nonexistent userdata is the same as zero-length userdata, according to > the specification. From libxl.h: > > int libxl_userdata_store(libxl_ctx *ctx, uint32_t domid, > const char *userdata_userid, > const uint8_t *data, int datalen); > /* If datalen==0, data is not used and the user data for > * that domain and userdata_userid is deleted. */ > > int libxl_userdata_retrieve(libxl_ctx *ctx, uint32_t domid, > const char *userdata_userid, > uint8_t **data_r, int *datalen_r); > /* On successful return, *data_r is from malloc. > * If there is no data for that domain and userdata_userid, > * *data_r and *datalen_r will be set to 0. > * data_r and datalen_r may be 0. > * On error return, *data_r and *datalen_r are undefined. > */ > > So I think to detect absence of userdata you should be checking > *data_r or *datalen_r on successful return from > libxl_userdata_retrieve. > Okay. > You are however correct in that there is a bug in the error handling: > if libxl_read_file_contents fails (ie, e!=0) for a reason other than > ENOENT, libxl_userdata_retrieve improperly ignores it. > Right. > I think the patch below fixes this. > > Do you agree ? > Yes. Acked-by: Jim Fehlig <jfehlig@xxxxxxxxxx> Thanks, Jim > Thanks, > Ian. > > > libxl: correct error path in libxl_userdata_retrieve > > Firstly, if libxl_read_file_contents fails, it doesn't really leave > *data and *datalen_r undefined - it leaves them unchanged. Tighten up > the spec for the benefit of libxl_userdata_retrieve. > > Secondly, libxl_userdata_retrieve ignored errors, assuming they were > all ENOENT. Instead it should fail on unexpected errors. > > Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> > > > diff -r 6067a17114bc tools/libxl/libxl_dom.c > --- a/tools/libxl/libxl_dom.c Thu Jan 27 16:17:27 2011 +0000 > +++ b/tools/libxl/libxl_dom.c Thu Jan 27 18:55:41 2011 +0000 > @@ -671,7 +671,10 @@ int libxl_userdata_retrieve(libxl_ctx *c > } > > e = libxl_read_file_contents(ctx, filename, data_r ? &data : 0, > &datalen); > - > + if (e && errno != ENOENT) { > + rc = ERROR_FAIL; > + goto out; > + } > if (!e && !datalen) { > LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "userdata file %s is empty", > filename); > if (data_r) assert(!*data_r); > diff -r 6067a17114bc tools/libxl/libxl_utils.h > --- a/tools/libxl/libxl_utils.h Thu Jan 27 16:17:27 2011 +0000 > +++ b/tools/libxl/libxl_utils.h Thu Jan 27 18:55:41 2011 +0000 > @@ -36,7 +36,7 @@ int libxl_read_file_contents(libxl_ctx * > /* Reads the contents of the plain file filename into a mallocd > * buffer. Returns 0 or errno. Any errors other than ENOENT are logged. > * If the file is empty, *data_r and *datalen_r are set to 0. > - * On error, *data_r and *datalen_r are undefined. > + * On error, *data_r and *datalen_r are unchanged. > * data_r and/or datalen_r may be 0. > */ > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxxxxxxxx > http://lists.xensource.com/xen-devel > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |