Jim Fehlig
2011-Jan-27 17:34 UTC
[Xen-devel] [PATCH] xl: Return error when no userdata exists
# HG changeset patch
# User Jim Fehlig <jfehlig@novell.com>
# Date 1296149593 25200
# Node ID aa57d7b164e246fb4cf8e971b890bfeb3a287fa9
# Parent b59f04eb89786e5ae6cb99c5f5dcd8e3790bc3eb
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.
Fix error checking of libxl_read_file_contents() results.
Signed-off-by: Jim Fehlig <jfehlig@novell.com>
diff -r b59f04eb8978 -r aa57d7b164e2 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c Fri Jan 21 18:06:23 2011 +0000
+++ b/tools/libxl/libxl_dom.c Thu Jan 27 10:33:13 2011 -0700
@@ -672,7 +672,7 @@
e = libxl_read_file_contents(ctx, filename, data_r ? &data : 0,
&datalen);
- if (!e && !datalen) {
+ if (e && !datalen) {
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "userdata file %s is
empty", filename);
if (data_r) assert(!*data_r);
rc = ERROR_FAIL;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jan-27 18:57 UTC
Re: [Xen-devel] [PATCH] xl: Return error when no userdata exists
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.
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.
I think the patch below fixes this.
Do you agree ?
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@eu.citrix.com>
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@lists.xensource.com
http://lists.xensource.com/xen-devel
Jim Fehlig
2011-Jan-27 19:10 UTC
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@novell.com> 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@eu.citrix.com> > > > 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@lists.xensource.com > http://lists.xensource.com/xen-devel >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jan-28 16:44 UTC
Re: [Xen-devel] [PATCH] xl: Return error when no userdata exists
Jim Fehlig writes ("Re: [Xen-devel] [PATCH] xl: Return error when no
userdata exists"):> Ian Jackson wrote:
> > I think the patch below fixes this.
>
> Yes.
>
> Acked-by: Jim Fehlig <jfehlig@novell.com>
Thanks, applied.
Do let us know what else you discover that needs fixing. I won''t be
surprised if the API needs changes, in which case we''ll put those in
4.2.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel