Jun Zhu (Intern)
2010-Sep-10 13:09 UTC
RE: [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@citrix.com>
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@lists.xensource.com
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@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Jackson
2010-Sep-10 17:28 UTC
RE: [Xen-devel] [PATCH] libxl: make libxl communicate with xenstored by socket or xenbus driver-V3
Jun Zhu (Intern) writes ("RE: [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@citrix.com>
Thanks. This doesn''t apply to current xen-unstable staging,
I''m
afraid.
Also:
> + if (!xsh)
> + fprintf(stderr, "cannot connect to xenstore");
This should use the logging functions provided, not just write to
stderr.
Finally:
> From: Ian Jackson
> Sent: 09 September 2010 13:09
> To: Jun Zhu (Intern)
> Cc: Ian Campbell; xen-devel@lists.xensource.com
> Subject: RE: [Xen-devel] [PATCH] libxl: make libxl communicate with
xenstored by socket or xenbus driver
>
> [quoted message]
There is no need to paste the previous message in the thread onto the
bottom of your mail. Indeed doing so can cause confusion if it has
extra bits of patch in it.
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel