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] libxl: make libxl communicate with xenstored by

To: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Subject: RE: [Xen-devel] [PATCH] libxl: make libxl communicate with xenstored by socket or xenbus driver-V3
From: "Jun Zhu (Intern)" <Jun.Zhu@xxxxxxxxxx>
Date: Fri, 10 Sep 2010 14:09:28 +0100
Accept-language: en-US
Acceptlanguage: en-US
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 10 Sep 2010 06:11:26 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AQHLUOlmp2nlF+8rq0CHiZcSxwhE9Q==
Thread-topic: [Xen-devel] [PATCH] libxl: make libxl communicate with xenstored by socket or xenbus driver-V3
make libxl communicate with xenstored by socket or xenbus driver-version 3
signed-off-by: Jun Zhu <Jun.Zhu@xxxxxxxxxx>

diff -r 1831912d4109 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c       Thu Sep 02 19:12:42 2010 +0100
+++ b/tools/libxl/libxl.c       Fri Sep 10 14:04:36 2010 +0100
@@ -53,12 +53,8 @@
         return ERROR_FAIL;
     }
 
-    ctx->xsh = xs_daemon_open();
-    if (!ctx->xsh)
-        ctx->xsh = xs_domain_open();
+    ctx->xsh = libxl_xs_open();
     if (!ctx->xsh) {
-        XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, errno, 
-                        "cannot connect to xenstore");
         xc_interface_close(ctx->xch);
         return ERROR_FAIL;
     }
@@ -69,7 +65,7 @@
 {
     xc_interface_close(ctx->xch);
     libxl_version_info_destroy(&ctx->version_info);
-    if (ctx->xsh) xs_daemon_close(ctx->xsh); 
+    if (ctx->xsh) libxl_xs_close(ctx->xsh);
     return 0;
 }
 
@@ -1387,21 +1383,22 @@
 {
     libxl_device_model_starting *starting = for_spawn;
     char *kvs[3];
-    int rc;
     struct xs_handle *xsh;
 
-    xsh = xs_daemon_open();
-    /* we mustn't use the parent's handle in the child */
-
     kvs[0] = "image/device-model-pid";
     if (asprintf(&kvs[1], "%d", innerchild) < 0)
         return;
     kvs[2] = NULL;
 
-    rc = xs_writev(xsh, XBT_NULL, starting->dom_path, kvs);
-    if (rc)
-        return;
-    xs_daemon_close(xsh);
+    /* we mustn't use the parent's handle in the child */
+    xsh = libxl_xs_open();
+    if (!xsh)
+        goto out;
+    xs_writev(xsh, XBT_NULL, starting->dom_path, kvs);
+    libxl_xs_close(xsh);
+
+out:
+    free(kvs[1]);
 }
 
 static int libxl_vfb_and_vkb_from_device_model_info(libxl_ctx *ctx,
diff -r 1831912d4109 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c        Thu Sep 02 19:12:42 2010 +0100
+++ b/tools/libxl/libxl_device.c        Fri Sep 10 14:04:36 2010 +0100
@@ -405,7 +405,10 @@
     unsigned int num;
     char **l = NULL;
 
-    xsh = xs_daemon_open();
+    xsh = libxl_xs_open();
+    if (!xsh)
+        return -1;
+
     path = libxl_sprintf(&gc, "/local/domain/0/device-model/%d/state", domid);
     xs_watch(xsh, path, path);
     tv.tv_sec = LIBXL_DEVICE_MODEL_START_TIMEOUT;
@@ -427,7 +430,7 @@
 
         free(p);
         xs_unwatch(xsh, path, path);
-        xs_daemon_close(xsh);
+        libxl_xs_close(xsh);
         libxl_free_all(&gc);
         return rc;
 again:
@@ -444,7 +447,7 @@
         }
     }
     xs_unwatch(xsh, path, path);
-    xs_daemon_close(xsh);
+    libxl_xs_close(xsh);
     XL_LOG(ctx, XL_LOG_ERROR, "Device Model not ready");
     libxl_free_all(&gc);
     return -1;
diff -r 1831912d4109 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c   Thu Sep 02 19:12:42 2010 +0100
+++ b/tools/libxl/libxl_dom.c   Fri Sep 10 14:04:36 2010 +0100
@@ -283,14 +283,16 @@
 
     snprintf(path, sizeof(path), 
"/local/domain/0/device-model/%u/logdirty/cmd", domid);
 
-    xsh = xs_daemon_open();
+    xsh = libxl_xs_open();
+    if (!xsh)
+        return;
 
     if (enable)
         xs_write(xsh, XBT_NULL, path, "enable", strlen("enable"));
     else
         xs_write(xsh, XBT_NULL, path, "disable", strlen("disable"));
 
-    xs_daemon_close(xsh);
+    libxl_xs_close(xsh);
 }
 
 static int core_suspend_callback(void *data)
diff -r 1831912d4109 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h      Thu Sep 02 19:12:42 2010 +0100
+++ b/tools/libxl/libxl_internal.h      Fri Sep 10 14:04:36 2010 +0100
@@ -138,6 +138,8 @@
 _hidden char *libxl_xs_get_dompath(libxl_gc *gc, uint32_t domid); // logs errs
 _hidden char *libxl_xs_read(libxl_gc *gc, xs_transaction_t t, char *path);
 _hidden char **libxl_xs_directory(libxl_gc *gc, xs_transaction_t t, char 
*path, unsigned int *nb);
+_hidden struct xs_handle *libxl_xs_open(void);
+_hidden void libxl_xs_close(struct xs_handle *xsh);
 
 /* from xl_dom */
 _hidden int is_hvm(libxl_ctx *ctx, uint32_t domid);
diff -r 1831912d4109 tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c Thu Sep 02 19:12:42 2010 +0100
+++ b/tools/libxl/libxl_utils.c Fri Sep 10 14:04:36 2010 +0100
@@ -367,8 +367,9 @@
 
 int libxl_ctx_postfork(libxl_ctx *ctx) {
     if (ctx->xsh) xs_daemon_destroy_postfork(ctx->xsh);
-    ctx->xsh = xs_daemon_open();
-    if (!ctx->xsh) return ERROR_FAIL;
+    ctx->xsh = libxl_xs_open();
+    if (!ctx->xsh)
+        return ERROR_FAIL;
     return 0;
 }
 
diff -r 1831912d4109 tools/libxl/libxl_xshelp.c
--- a/tools/libxl/libxl_xshelp.c        Thu Sep 02 19:12:42 2010 +0100
+++ b/tools/libxl/libxl_xshelp.c        Fri Sep 10 14:04:36 2010 +0100
@@ -141,3 +141,20 @@
     libxl_ptr_add(gc, ret);
     return ret;
 }
+
+struct xs_handle *libxl_xs_open()
+{
+    struct xs_handle *xsh = NULL;
+
+    xsh = xs_daemon_open();
+    if (!xsh)
+        xsh = xs_domain_open();
+    if (!xsh)
+        fprintf(stderr, "cannot connect to xenstore");
+    return xsh;
+}
+
+void libxl_xs_close(struct xs_handle *xsh)
+{
+    xs_daemon_close(xsh);
+}


Jun Zhu
Citrix Systems UK
________________________________________
From: Ian Jackson
Sent: 09 September 2010 13:09
To: Jun Zhu (Intern)
Cc: Ian Campbell; xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: RE: [Xen-devel] [PATCH] libxl: make libxl communicate with xenstored 
by socket or xenbus driver

Jun Zhu (Intern) writes ("RE: [Xen-devel] [PATCH] libxl: make libxl communicate 
with xenstored by socket or xenbus driver"):
> I make another one.

Thanks.  (It helps if you say "v2" in the Subject line.)

> -    rc = xs_writev(xsh, XBT_NULL, starting->dom_path, kvs);
> -    if (rc)
> -        return;
> -    xs_daemon_close(xsh);
> +     /* we mustn't use the parent's handle in the child */
> +     xsh = libxl_xs_open();

This, and some of the other parts of your patch, have what seem to be
unintended whitespace and formatting changes.  Can you please make
sure that:
  * Your indents are exactly 4 spaces
  * You don't put any tabs in (your MUA may mangle them, and anyway
     we don't like tabs)
  * Curly backets (aka braces) should go on the same line as the
    if and else.

> +    if (!xsh)
> +     {
> +             fprintf(stderr, "cannot connect to xenstore");
> +             free(kvs[1]);
> +             return;
> +     }
> +    xs_writev(xsh, XBT_NULL, starting->dom_path, kvs);
> +    libxl_xs_close(xsh);
> +
> +     free(kvs[1]);

This would be better written with the "goto out" idiom, I think ?

> -    ctx->xsh = xs_daemon_open();
> -    if (!ctx->xsh) return ERROR_FAIL;
> -    return 0;
> +     ctx->xsh = libxl_xs_open();
> +     if (!ctx->xsh)
> +     {
> +             XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, errno,
> +                             "cannot connect to xenstore");

This logging, if it is appropriate, should surely be done in
libxl_xs_open, rather than coded out in each call ?

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>