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/
Home Products Support Community News


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

To: Jim Fehlig <jfehlig@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xl: Return error when no userdata exists
From: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Date: Thu, 27 Jan 2011 18:57:42 +0000
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Thu, 27 Jan 2011 10:58:50 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <aa57d7b164e246fb4cf8.1296149649@xxxxxxxxxxxxxxxxxxxxxxxxx>
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>
Newsgroups: chiark.mail.xen.devel
References: <aa57d7b164e246fb4cf8.1296149649@xxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Jim Fehlig writes ("[Xen-devel] [PATCH] xl: Return error when no userdata 
> 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

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 ?


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", 
         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