WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] xl: Return error when no userdata exists

To: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xl: Return error when no userdata exists
From: Jim Fehlig <jfehlig@xxxxxxxxxx>
Date: Thu, 27 Jan 2011 12:10:12 -0700
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Thu, 27 Jan 2011 11:13:06 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <19777.49190.822476.585064@xxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <aa57d7b164e246fb4cf8.1296149649@xxxxxxxxxxxxxxxxxxxxxxxxx> <19777.49190.822476.585064@xxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird 2.0.0.24 (X11/20100302)
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