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
|
Home