thr3ads.net - Xen devel - RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver [Sep 2010]

If this information is useful, please help other people find it:
Share via:

Jun Zhu (Intern)

2010-Sep-17 14:02 UTC

RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver

This version adds gc as a parameter to libxl__xs_open, and uses LIBXL__LOG_ERRNO
for logging. If some functions cannot use the ctx, It should transfer NULL to
libxl__xs_open to disable logging. (But it will make users difficult to find the
problem when no logging is output.)
To make consistent with other functions in libxl__xshelp.c, I use gc as its
parameter, not ctx. In the libxl_ctx_init function, I add “libxl__gc gc =
LIBXL_INIT_GC(ctx)” to get the gc of ctx. Please check this.

Signed-off-by: Jun Zhu <Jun.Zhu@citrix.com>

diff -r cca905a429aa tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Tue Sep 14 15:39:36 2010 +0100
+++ b/tools/libxl/libxl.c	Fri Sep 17 14:58:50 2010 +0100
@@ -40,6 +40,8 @@
 
 int libxl_ctx_init(libxl_ctx *ctx, int version, xentoollog_logger *lg)
 {
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
+
     if (version != LIBXL_VERSION)
         return ERROR_VERSION;
     memset(ctx, 0, sizeof(libxl_ctx));
@@ -53,12 +55,8 @@
         return ERROR_FAIL;
     }
 
-    ctx->xsh = xs_daemon_open();
-    if (!ctx->xsh)
-        ctx->xsh = xs_domain_open();
+    ctx->xsh = libxl__xs_open(&gc);
     if (!ctx->xsh) {
-        LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, errno, 
-                        "cannot connect to xenstore");
         xc_interface_close(ctx->xch);
         return ERROR_FAIL;
     }
@@ -69,7 +67,7 @@
 {
     if (ctx->xch) 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;
 }
 
@@ -901,8 +899,7 @@
         ret = libxl_domain_destroy(ctx, stubdomid, 0);
         if (ret)
             goto out;
-    }
-    else {
+    } else {
         ret = kill(atoi(pid), SIGHUP);
         if (ret < 0 && errno == ESRCH) {
             LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Device Model already
exited");
@@ -1395,21 +1392,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(NULL);
+    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 cca905a429aa tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Tue Sep 14 15:39:36 2010 +0100
+++ b/tools/libxl/libxl_device.c	Fri Sep 17 14:58:50 2010 +0100
@@ -405,7 +405,10 @@
     unsigned int num;
     char **l = NULL;
 
-    xsh = xs_daemon_open();
+    xsh = libxl__xs_open(&gc);
+    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);
     LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Device Model not ready");
     libxl__free_all(&gc);
     return -1;
diff -r cca905a429aa tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Tue Sep 14 15:39:36 2010 +0100
+++ b/tools/libxl/libxl_dom.c	Fri Sep 17 14:58:50 2010 +0100
@@ -326,14 +326,16 @@
 
     snprintf(path, sizeof(path),
"/local/domain/0/device-model/%u/logdirty/cmd", domid);
 
-    xsh = xs_daemon_open();
+    xsh = libxl__xs_open(NULL);
+    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 libxl__domain_suspend_common_callback(void *data)
diff -r cca905a429aa tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Tue Sep 14 15:39:36 2010 +0100
+++ b/tools/libxl/libxl_internal.h	Fri Sep 17 14:58:50 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(libxl__gc *gc);
+_hidden void libxl__xs_close(struct xs_handle *xsh);
 
 /* from xl_dom */
 _hidden int libxl__domain_is_hvm(libxl_ctx *ctx, uint32_t domid);
diff -r cca905a429aa tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c	Tue Sep 14 15:39:36 2010 +0100
+++ b/tools/libxl/libxl_utils.c	Fri Sep 17 14:58:50 2010 +0100
@@ -366,9 +366,11 @@
 
 
 int libxl_ctx_postfork(libxl_ctx *ctx) {
+    libxl__gc gc = LIBXL_INIT_GC(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(&gc);
+    if (!ctx->xsh)
+        return ERROR_FAIL;
     return 0;
 }
 
diff -r cca905a429aa tools/libxl/libxl_xshelp.c
--- a/tools/libxl/libxl_xshelp.c	Tue Sep 14 15:39:36 2010 +0100
+++ b/tools/libxl/libxl_xshelp.c	Fri Sep 17 14:58:50 2010 +0100
@@ -141,3 +141,21 @@
     libxl__ptr_add(gc, ret);
     return ret;
 }
+
+struct xs_handle *libxl__xs_open(libxl__gc *gc)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    struct xs_handle *xsh = NULL;
+
+    xsh = xs_daemon_open();
+    if (!xsh)
+        xsh = xs_domain_open();
+    if (!xsh && !gc)
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "cannot connect to
xenstore");
+    return xsh;
+}
+
+void libxl__xs_close(struct xs_handle *xsh)
+{
+    xs_daemon_close(xsh);
+}

Jun Zhu
Citrix Systems UK
________________________________________

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

Jun Zhu (Intern)

2010-Sep-17 14:12 UTC

head link

RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver

Sorry. The V5 version has one place conflicting with another patch. I will make
a new one soon.

Jun Zhu
Citrix Systems UK
________________________________________
From: Jun Zhu (Intern)
Sent: 17 September 2010 10:02
To: Ian Campbell
Cc: Ian Jackson; xen-devel@lists.xensource.com
Subject: RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored
by socket or xenbus driver

This version adds gc as a parameter to libxl__xs_open, and uses LIBXL__LOG_ERRNO
for logging. If some functions cannot use the ctx, It should transfer NULL to
libxl__xs_open to disable logging. (But it will make users difficult to find the
problem when no logging is output.)
To make consistent with other functions in libxl__xshelp.c, I use gc as its
parameter, not ctx. In the libxl_ctx_init function, I add “libxl__gc gc =
LIBXL_INIT_GC(ctx)” to get the gc of ctx. Please check this.

Signed-off-by: Jun Zhu <Jun.Zhu@citrix.com>

diff -r cca905a429aa tools/libxl/libxl.c
--- a/tools/libxl/libxl.c       Tue Sep 14 15:39:36 2010 +0100
+++ b/tools/libxl/libxl.c       Fri Sep 17 14:58:50 2010 +0100
@@ -40,6 +40,8 @@

 int libxl_ctx_init(libxl_ctx *ctx, int version, xentoollog_logger *lg)
 {
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
+
     if (version != LIBXL_VERSION)
         return ERROR_VERSION;
     memset(ctx, 0, sizeof(libxl_ctx));
@@ -53,12 +55,8 @@
         return ERROR_FAIL;
     }

-    ctx->xsh = xs_daemon_open();
-    if (!ctx->xsh)
-        ctx->xsh = xs_domain_open();
+    ctx->xsh = libxl__xs_open(&gc);
     if (!ctx->xsh) {
-        LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, errno,
-                        "cannot connect to xenstore");
         xc_interface_close(ctx->xch);
         return ERROR_FAIL;
     }
@@ -69,7 +67,7 @@
 {
     if (ctx->xch) 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;
 }

@@ -901,8 +899,7 @@
         ret = libxl_domain_destroy(ctx, stubdomid, 0);
         if (ret)
             goto out;
-    }
-    else {
+    } else {
         ret = kill(atoi(pid), SIGHUP);
         if (ret < 0 && errno == ESRCH) {
             LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Device Model already
exited");
@@ -1395,21 +1392,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(NULL);
+    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 cca905a429aa tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c        Tue Sep 14 15:39:36 2010 +0100
+++ b/tools/libxl/libxl_device.c        Fri Sep 17 14:58:50 2010 +0100
@@ -405,7 +405,10 @@
     unsigned int num;
     char **l = NULL;

-    xsh = xs_daemon_open();
+    xsh = libxl__xs_open(&gc);
+    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);
     LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Device Model not ready");
     libxl__free_all(&gc);
     return -1;
diff -r cca905a429aa tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c   Tue Sep 14 15:39:36 2010 +0100
+++ b/tools/libxl/libxl_dom.c   Fri Sep 17 14:58:50 2010 +0100
@@ -326,14 +326,16 @@

     snprintf(path, sizeof(path),
"/local/domain/0/device-model/%u/logdirty/cmd", domid);

-    xsh = xs_daemon_open();
+    xsh = libxl__xs_open(NULL);
+    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 libxl__domain_suspend_common_callback(void *data)
diff -r cca905a429aa tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h      Tue Sep 14 15:39:36 2010 +0100
+++ b/tools/libxl/libxl_internal.h      Fri Sep 17 14:58:50 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(libxl__gc *gc);
+_hidden void libxl__xs_close(struct xs_handle *xsh);

 /* from xl_dom */
 _hidden int libxl__domain_is_hvm(libxl_ctx *ctx, uint32_t domid);
diff -r cca905a429aa tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c Tue Sep 14 15:39:36 2010 +0100
+++ b/tools/libxl/libxl_utils.c Fri Sep 17 14:58:50 2010 +0100
@@ -366,9 +366,11 @@


 int libxl_ctx_postfork(libxl_ctx *ctx) {
+    libxl__gc gc = LIBXL_INIT_GC(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(&gc);
+    if (!ctx->xsh)
+        return ERROR_FAIL;
     return 0;
 }

diff -r cca905a429aa tools/libxl/libxl_xshelp.c
--- a/tools/libxl/libxl_xshelp.c        Tue Sep 14 15:39:36 2010 +0100
+++ b/tools/libxl/libxl_xshelp.c        Fri Sep 17 14:58:50 2010 +0100
@@ -141,3 +141,21 @@
     libxl__ptr_add(gc, ret);
     return ret;
 }
+
+struct xs_handle *libxl__xs_open(libxl__gc *gc)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    struct xs_handle *xsh = NULL;
+
+    xsh = xs_daemon_open();
+    if (!xsh)
+        xsh = xs_domain_open();
+    if (!xsh && !gc)
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "cannot connect to
xenstore");
+    return xsh;
+}
+
+void libxl__xs_close(struct xs_handle *xsh)
+{
+    xs_daemon_close(xsh);
+}

Jun Zhu
Citrix Systems UK
________________________________________

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

Gianni Tedesco

2010-Sep-17 14:21 UTC

head link

RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver

On Fri, 2010-09-17 at 15:12 +0100, Jun Zhu (Intern)
wrote:> Sorry. The V5 version has one place conflicting with another patch. I will
make a new one soon.
> 
> Jun Zhu
> Citrix Systems UK
> ________________________________________
> From: Jun Zhu (Intern)
> Sent: 17 September 2010 10:02
> To: Ian Campbell
> Cc: Ian Jackson; xen-devel@lists.xensource.com
> Subject: RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with
xenstored by socket or xenbus driver
> 
> This version adds gc as a parameter to libxl__xs_open, and uses
LIBXL__LOG_ERRNO for logging. If some functions cannot use the ctx, It should
transfer NULL to libxl__xs_open to disable logging. (But it will make users
difficult to find the problem when no logging is output.)
> To make consistent with other functions in libxl__xshelp.c, I use gc as its
parameter, not ctx. In the libxl_ctx_init function, I add “libxl__gc gc =
LIBXL_INIT_GC(ctx)” to get the gc of ctx. Please check this.
> 
> Signed-off-by: Jun Zhu <Jun.Zhu@citrix.com>
> 
> diff -r cca905a429aa tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c       Tue Sep 14 15:39:36 2010 +0100
> +++ b/tools/libxl/libxl.c       Fri Sep 17 14:58:50 2010 +0100
> @@ -40,6 +40,8 @@
> 
>  int libxl_ctx_init(libxl_ctx *ctx, int version, xentoollog_logger *lg)
>  {
> +    libxl__gc gc = LIBXL_INIT_GC(ctx);
> +
>      if (version != LIBXL_VERSION)
>          return ERROR_VERSION;
>      memset(ctx, 0, sizeof(libxl_ctx));
> @@ -53,12 +55,8 @@
  +        libxl__free_all(&gc);>          return ERROR_FAIL;
>      }
This leaks memory in success and both failure paths. That to be fixed in
next patch too I hope? :)


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

Ian Campbell

2010-Sep-17 14:27 UTC

head link

RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver

On Fri, 2010-09-17 at 15:02 +0100, Jun Zhu (Intern)
wrote:> This version adds gc as a parameter to libxl__xs_open, and uses
LIBXL__LOG_ERRNO for logging. If some functions cannot use the ctx, It should
transfer NULL to libxl__xs_open to disable logging. (But it will make users
difficult to find the problem when no logging is output.)
> To make consistent with other functions in libxl__xshelp.c, I use gc as its
parameter, not ctx. In the libxl_ctx_init function, I add “libxl__gc gc =
LIBXL_INIT_GC(ctx)” to get the gc of ctx. Please check this.
> 
> Signed-off-by: Jun Zhu <Jun.Zhu@citrix.com>
> 
> diff -r cca905a429aa tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Tue Sep 14 15:39:36 2010 +0100
> +++ b/tools/libxl/libxl.c	Fri Sep 17 14:58:50 2010 +0100
> @@ -40,6 +40,8 @@
>  
>  int libxl_ctx_init(libxl_ctx *ctx, int version, xentoollog_logger *lg)
>  {
> +    libxl__gc gc = LIBXL_INIT_GC(ctx);
> +
If you add one of these then you also need to add a libxl__free_all(&gc)
on every exit path from the function.

Special care needs to be taken in libxl_ctx_init to ensure the ctx is
initialised enough to be able to create a gc. I suspect it is safe as is
but perhaps moving the "gc = LIBXL_INIT_GC(ctx)" later in the function
will prevent accidents? e.g. put it just before this:

+    ctx->xsh = libxl__xs_open(&gc);
     if (!ctx->xsh) {
         xc_interface_close(ctx->xch);
         return ERROR_FAIL;

> diff -r cca905a429aa tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c	Tue Sep 14 15:39:36 2010 +0100
> +++ b/tools/libxl/libxl_dom.c	Fri Sep 17 14:58:50 2010 +0100
> @@ -326,14 +326,16 @@
>  
>      snprintf(path, sizeof(path),
"/local/domain/0/device-model/%u/logdirty/cmd", domid);
>  
> -    xsh = xs_daemon_open();
> +    xsh = libxl__xs_open(NULL);
> +    if (!xsh)
> +        return;
This is libxl__domain_suspend_common_switch_qemu_logdirty[0] which is
used as a callback from xc_domain_suspend. IMHO any function which takes
a callback should also take a void * closure, which in this case could
be used to pass the context from the caller of xc_domain_save to this
function. Given that xc does not do this I suppose this is fine as is.

[0] BTW please add this to your ~/.hgrc to automatically annotate
functions in diffs:
"""
        [diff]
        showfunc = True
"""
> diff -r cca905a429aa tools/libxl/libxl_utils.c
> --- a/tools/libxl/libxl_utils.c	Tue Sep 14 15:39:36 2010 +0100
> +++ b/tools/libxl/libxl_utils.c	Fri Sep 17 14:58:50 2010 +0100
> @@ -366,9 +366,11 @@
>  
> 
>  int libxl_ctx_postfork(libxl_ctx *ctx) {
> +    libxl__gc gc = LIBXL_INIT_GC(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(&gc);
> +    if (!ctx->xsh)
> +        return ERROR_FAIL;
>      return 0;
>  } 
Another place where libxl__free_all(&gc) is needed.



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

Ian Campbell

2010-Sep-17 15:05 UTC

head link

RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver

On Fri, 2010-09-17 at 15:27 +0100, Ian Campbell wrote:> 
> This is libxl__domain_suspend_common_switch_qemu_logdirty[0] which is
> used as a callback from xc_domain_suspend. IMHO any function which
> takes
> a callback should also take a void * closure, which in this case could
> be used to pass the context from the caller of xc_domain_save to this
> function. 
e.g. this (untested) patch:

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1284735849 -3600
# Node ID ef0d82c6be32792167870d77d9655efa268e642f
# Parent  e8d0ecb9746aa9273e5a36cf561fc1bd8bce3714
tools: add closure to xc_domain_save switch_qemu_logdirty callback

Also move the function into struct save_callbacks with the others.

Use this in libxl to pass the save context to
libxl__domain_suspend_common_switch_qemu_logdirty allowing us to reuse
the parents xenstore handle, gc context etc.

Also add an apparently missing libxl__free_all to
libxl__domain_suspend_common.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r e8d0ecb9746a -r ef0d82c6be32 tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Fri Sep 17 14:41:26 2010 +0100
+++ b/tools/libxc/xc_domain_save.c	Fri Sep 17 16:04:09 2010 +0100
@@ -879,7 +879,7 @@ int xc_domain_save(xc_interface *xch, in
 int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t
max_iters,
                    uint32_t max_factor, uint32_t flags,
                    struct save_callbacks* callbacks,
-                   int hvm, void (*switch_qemu_logdirty)(int, unsigned))
+                   int hvm)
 {
     xc_dominfo_t info;
     DECLARE_DOMCTL;
@@ -1015,7 +1015,7 @@ int xc_domain_save(xc_interface *xch, in
 
         /* Enable qemu-dm logging dirty pages to xen */
         if ( hvm )
-            switch_qemu_logdirty(dom, 1);
+            callbacks->switch_qemu_logdirty(dom, 1, callbacks->data);
     }
     else
     {
@@ -1876,7 +1876,7 @@ int xc_domain_save(xc_interface *xch, in
                                NULL, 0, NULL, 0, NULL) < 0 )
             DPRINTF("Warning - couldn''t disable shadow
mode");
         if ( hvm )
-            switch_qemu_logdirty(dom, 0);
+            callbacks->switch_qemu_logdirty(dom, 0, callbacks->data);
     }
 
     if ( live_shinfo )
diff -r e8d0ecb9746a -r ef0d82c6be32 tools/libxc/xenguest.h
--- a/tools/libxc/xenguest.h	Fri Sep 17 14:41:26 2010 +0100
+++ b/tools/libxc/xenguest.h	Fri Sep 17 16:04:09 2010 +0100
@@ -40,6 +40,8 @@ struct save_callbacks {
      * 1: take another checkpoint */
     int (*checkpoint)(void* data);
 
+    void (*switch_qemu_logdirty)(int domid, unsigned enable, void *data); /*
HVM only */
+
     /* to be provided as the first argument to each callback function */
     void* data;
 };
@@ -55,7 +57,7 @@ int xc_domain_save(xc_interface *xch, in
 int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t
max_iters,
                    uint32_t max_factor, uint32_t flags /* XCFLAGS_xxx */,
                    struct save_callbacks* callbacks,
-                   int hvm, void (*switch_qemu_logdirty)(int, unsigned)); /*
HVM only */
+                   int hvm);
 
 
 /**
diff -r e8d0ecb9746a -r ef0d82c6be32 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Fri Sep 17 14:41:26 2010 +0100
+++ b/tools/libxl/libxl_dom.c	Fri Sep 17 16:04:09 2010 +0100
@@ -325,21 +325,18 @@ struct suspendinfo {
     unsigned int flags;
 };
 
-static void libxl__domain_suspend_common_switch_qemu_logdirty(int domid,
unsigned int enable)
+static void libxl__domain_suspend_common_switch_qemu_logdirty(int domid,
unsigned int enable, void *data)
 {
-    struct xs_handle *xsh;
-    char path[64];
+    struct suspendinfo *si = data;
+    libxl_ctx *ctx = libxl__gc_owner(si->gc);
+    char *path;
 
-    snprintf(path, sizeof(path),
"/local/domain/0/device-model/%u/logdirty/cmd", domid);
-
-    xsh = xs_daemon_open();
+    path = libxl__sprintf(si->gc,
"/local/domain/0/device-model/%u/logdirty/cmd", domid);
 
     if (enable)
-        xs_write(xsh, XBT_NULL, path, "enable",
strlen("enable"));
+        xs_write(ctx->xsh, XBT_NULL, path, "enable",
strlen("enable"));
     else
-        xs_write(xsh, XBT_NULL, path, "disable",
strlen("disable"));
-
-    xs_daemon_close(xsh);
+        xs_write(ctx->xsh, XBT_NULL, path, "disable",
strlen("disable"));
 }
 
 static int libxl__domain_suspend_common_callback(void *data)
@@ -437,11 +434,11 @@ int libxl__domain_suspend_common(libxl_c
 
     memset(&callbacks, 0, sizeof(callbacks));
     callbacks.suspend = libxl__domain_suspend_common_callback;
+    callbacks.switch_qemu_logdirty =
libxl__domain_suspend_common_switch_qemu_logdirty;
     callbacks.data = &si;
 
     xc_domain_save(ctx->xch, fd, domid, 0, 0, flags,
-                   &callbacks, hvm,
-                   &libxl__domain_suspend_common_switch_qemu_logdirty);
+                   &callbacks, hvm);
 
     if (si.suspend_eventchn > 0)
         xc_suspend_evtchn_release(ctx->xch, si.xce, domid,
si.suspend_eventchn);
@@ -450,6 +447,7 @@ int libxl__domain_suspend_common(libxl_c
 
     rc = 0;
 out:
+    libxl__free_all(&gc);
     return rc;
 }
 
diff -r e8d0ecb9746a -r ef0d82c6be32 tools/xcutils/xc_save.c
--- a/tools/xcutils/xc_save.c	Fri Sep 17 14:41:26 2010 +0100
+++ b/tools/xcutils/xc_save.c	Fri Sep 17 16:04:09 2010 +0100
@@ -102,7 +102,7 @@ static int suspend(void* data)
  * active buffer. */
 
 
-static void switch_qemu_logdirty(int domid, unsigned int enable)
+static void switch_qemu_logdirty(int domid, unsigned int enable, void *data)
 {
     struct xs_handle *xs;
     char *path, *p, *ret_str, *cmd_str, **watch;
@@ -205,9 +205,9 @@ main(int argc, char **argv)
     }
     memset(&callbacks, 0, sizeof(callbacks));
     callbacks.suspend = suspend;
+    callbacks.switch_qemu_logdirty = switch_qemu_logdirty;
     ret = xc_domain_save(si.xch, io_fd, si.domid, maxit, max_f, si.flags, 
-                         &callbacks, !!(si.flags & XCFLAGS_HVM),
-                         &switch_qemu_logdirty);
+                         &callbacks, !!(si.flags & XCFLAGS_HVM));
 
     if (si.suspend_evtchn > 0)
 	 xc_suspend_evtchn_release(si.xch, si.xce, si.domid, si.suspend_evtchn);



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

Ian Jackson

2010-Sep-17 16:04 UTC

head link

RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver

Ian Campbell writes ("RE: [Xen-devel] [PATCH V5] libxl: make libxl
communicate with xenstored by socket or xenbus
driver"):> e.g. this (untested) patch:
...> tools: add closure to xc_domain_save switch_qemu_logdirty callback
This looks like a nice patch to me.  I haven''t tested it either though
:-).

Jun, if you''d like to resubmit your socket/xenbus patch on top of
Ian''s logdirty closure patch, then you''ll end up testing
Ian''s patch
at the same time which would be ideal.  Let us know if you have any
trouble with that.

Thanks,
Ian.

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

Jun Zhu (Intern)

2010-Sep-20 08:34 UTC

head link

RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver

Fine. I will improve my patch and test them together.

Jun Zhu
Citrix Systems UK

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

Xen devel - Sep 2010 - RE: [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver

RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver

RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver

RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver

RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver

RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver

RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver

RE: [Xen-devel] [PATCH V5] libxl: make libxl communicate with xenstored by socket or xenbus driver