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
From: "Jun Zhu (Intern)" <Jun.Zhu@xxxxxxxxxx>
Date: Wed, 8 Sep 2010 20:20:34 +0100
Accept-language: en-US
Acceptlanguage: en-US
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 08 Sep 2010 12:24:46 -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: AQHLT4ta3LElj6lzOUeEtQe1onMgVw==
Thread-topic: [Xen-devel] [PATCH] libxl: make libxl communicate with xenstored by socket or xenbus driver
I make another one.

exporting patch:
# HG changeset patch
# User Jun Zhu <Jun.Zhu@xxxxxxxxxx>
# Date 1283973408 -3600
# Node ID c54e3f3df3ed2edf4e09a004267df81deb999cc5
# Parent  1831912d4109731e78c01be40ec70b5fae804d30
Make libxl use xenbus dev to communicate with xenstored if socket communication 
does not exists.

diff -r 1831912d4109 -r c54e3f3df3ed tools/libxl/libxl.c
--- a/tools/libxl/libxl.c       Thu Sep 02 19:12:42 2010 +0100
+++ b/tools/libxl/libxl.c       Wed Sep 08 20:16:48 2010 +0100
@@ -53,9 +53,7 @@
         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");
@@ -69,7 +67,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 +1385,25 @@
 {
     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)
+       {
+               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]);
 }
 
 static int libxl_vfb_and_vkb_from_device_model_info(libxl_ctx *ctx,
diff -r 1831912d4109 -r c54e3f3df3ed 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        Wed Sep 08 20:16:48 2010 +0100
@@ -405,7 +405,14 @@
     unsigned int num;
     char **l = NULL;
 
-    xsh = xs_daemon_open();
+       xsh = libxl_xs_open();
+       if (!xsh)
+       {
+               XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, errno,
+                               "cannot connect to xenstore");
+               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 +434,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 +451,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 -r c54e3f3df3ed 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   Wed Sep 08 20:16:48 2010 +0100
@@ -283,14 +283,19 @@
 
     snprintf(path, sizeof(path), 
"/local/domain/0/device-model/%u/logdirty/cmd", domid);
 
-    xsh = xs_daemon_open();
+    xsh = libxl_xs_open();
+       if (!xsh)
+       {
+               fprintf(stderr, "cannot connect to xenstore");
+               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 -r c54e3f3df3ed 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      Wed Sep 08 20:16:48 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 -r c54e3f3df3ed 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 Wed Sep 08 20:16:48 2010 +0100
@@ -367,9 +367,14 @@
 
 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;
-    return 0;
+       ctx->xsh = libxl_xs_open();
+       if (!ctx->xsh)
+       {
+               XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, errno,
+                               "cannot connect to xenstore");
+               return ERROR_FAIL;
+       }
+       return 0;
 }
 
 pid_t libxl_fork(libxl_ctx *ctx)
diff -r 1831912d4109 -r c54e3f3df3ed 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        Wed Sep 08 20:16:48 2010 +0100
@@ -141,3 +141,19 @@
     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();
+
+       return xsh;
+}
+
+void libxl_xs_close(struct xs_handle *xsh)
+{
+       xs_daemon_close(xsh);
+}



Jun Zhu
Citrix Systems UK
________________________________________
From: Ian Jackson
Sent: 08 September 2010 11:31
To: Jun Zhu (Intern)
Cc: Ian Campbell; xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: ð´: [Xen-devel] [PATCH] libxl: make libxl c        ommunicate with 
xenstored by socket or xenb     us driver

Jun Zhu (Intern) writes ("ð´: [Xen-devel] [PATCH] libxl: make libxl c 
ommunicate with xenstored by socket or xenb us driver"):
> libxl uses socket or xenbus dev to communicate with xenstored.

Thanks for this patch.  I have a few comments:

> @@ -1387,10 +1385,8 @@
>  {
>      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";
> @@ -1398,10 +1394,11 @@
>          return;
>      kvs[2] = NULL;
>
> -    rc = xs_writev(xsh, XBT_NULL, starting->dom_path, kvs);
> -    if (rc)
> -        return;
> -    xs_daemon_close(xsh);
> +    xsh = libxl_xs_open();
> +    if (!xsh)
> +     fprintf(stderr, "cannot connect to xenstore");
> +    xs_writev(xsh, XBT_NULL, starting->dom_path, kvs);
> +    libxl_xs_close(xsh);
>  }

It's not clear to me why you're moving the open call about.  Your
error handling is incorrect, as passing a null xsh to xs_writev will
cause a segfault.  Also, while you're there you should probably fix
the memory leak: this function always seems to leak kvs[0].

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

Logging is not sufficient; you need to set rc and "goto out", or
return an error code, too.  This pattern occurs a number of times in
your patch.

Finally, your patch seems to have been truncated at some point.
patch(1) complains:
  patch: **** malformed patch at line 172:

Thanks,
Ian.

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

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