Xen devel - Oct 2010 - [PATCH] libxl: remove console 0 backend directory from xenstore on devices destroy

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1286198484 -3600
# Node ID d59be7a8e1e3bc439bb01a0f89d43411210fedcb
# Parent  1cae52330eebbe448586c0a173945bc2bf8d631f
libxl: remove console 0 backend directory from xenstore on devices destroy

The is (deliberately) no frontend directory for console 0 in
xenstore. Therefore to avoid leaking
/local/domain/0/backend/console/$domid/0 check explicitly for this
backend directory and remove.

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

diff -r 1cae52330eeb -r d59be7a8e1e3 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Mon Oct 04 11:42:31 2010 +0100
+++ b/tools/libxl/libxl_device.c	Mon Oct 04 14:21:24 2010 +0100
@@ -294,7 +294,7 @@ int libxl__devices_destroy(libxl_ctx *ct
 int libxl__devices_destroy(libxl_ctx *ctx, uint32_t domid, int force)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
-    char *path, *be_path, *fe_path;
+    char *path, *be_path, *fe_path, *tmp;
     unsigned int num1, num2;
     char **l1 = NULL, **l2 = NULL;
     int i, j, n = 0, n_watches = 0;
@@ -326,6 +326,17 @@ int libxl__devices_destroy(libxl_ctx *ct
             }
         }
     }
+
+    /* console 0 has no frontend directory, check for a backend */
+    be_path = libxl__sprintf(&gc,
"/local/domain/0/backend/console/%d/0", domid);
+    tmp = xs_read(ctx->xsh, XBT_NULL, be_path, NULL);
+    if (tmp) {
+        if (libxl__device_destroy(ctx, be_path, force) > 0)
+            n_watches++;
+        flexarray_set(toremove, n++, libxl__dirname(&gc, be_path));
+        free(tmp);
+    }
+
     if (!force) {
         /* Linux-ism. Most implementations leave the timeout
          * untouched after select. Linux, however, will chip

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

On Mon, 2010-10-04 at 14:21 +0100, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # Date 1286198484 -3600
> # Node ID d59be7a8e1e3bc439bb01a0f89d43411210fedcb
> # Parent  1cae52330eebbe448586c0a173945bc2bf8d631f
> libxl: remove console 0 backend directory from xenstore on devices destroy
> 
> The is (deliberately) no frontend directory for console 0 in
> xenstore. Therefore to avoid leaking
> /local/domain/0/backend/console/$domid/0 check explicitly for this
> backend directory and remove.
Thinking about this some more it might be preferable to
write /local/domain/domid/console/{backend,backend-id} giving us a
concrete pointer to the backend we wish to destroy, this would cope
better with console backends in dom!=0 too.

I''m assuming its not a mistake that the backend exists in the first
place?

Ian.


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

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1286207114 -3600
# Node ID 9e0d9b5460dc1c3cad0f5576157ac10b4045b4de
# Parent  a2c40fa3bc6c8471e1988e445f449975cf6f3822
libxl: remove console 0 backend directory from xenstore on devices destroy

The first PV console device has an unusual frontend path for
historical reasons (/local/domain/<domid>/console rather than
/local/domain/<domid>/device/console/0).

Therefore we need to check this additional frontend path as well as
those under /local/domain/<domid>/device when tearing down all
backends or else we miss the backend for the console.

As part of this we need to ensure that the frontend directory has a
valid link to the backend, currently this link does not exist.

Fix this by allowing libxl__device_generic_add to take an optional
frontend path suffix relative to the dompath allowing us to use the
function even for setting up the special case frontend.

This also fixes the link from backend to frontend which until now gave
a dangling link to the normal device path instead of the exceptional
one.

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

diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Oct 04 15:39:30 2010 +0100
+++ b/tools/libxl/libxl.c	Mon Oct 04 16:45:14 2010 +0100
@@ -1897,7 +1897,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
 
     libxl__device_generic_add(ctx, &device,
                              libxl__xs_kvs_of_flexarray(&gc, back,
boffset),
-                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset));
+                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset), NULL);
 
     rc = 0;
 
@@ -2047,7 +2047,7 @@ int libxl_device_nic_add(libxl_ctx *ctx,
 
     libxl__device_generic_add(ctx, &device,
                              libxl__xs_kvs_of_flexarray(&gc, back,
boffset),
-                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset));
+                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset), NULL);
 
     /* FIXME: wait for plug */
     rc = 0;
@@ -2232,7 +2232,7 @@ int libxl_device_net2_add(libxl_ctx *ctx
 
     libxl__device_generic_add(ctx, &device,
                              libxl__xs_kvs_of_flexarray(&gc, back,
boffset),
-                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset));
+                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset), NULL);
 
     /* FIXME: wait for plug */
     rc = 0;
@@ -2324,36 +2324,13 @@ int libxl_device_console_add(libxl_ctx *
 int libxl_device_console_add(libxl_ctx *ctx, uint32_t domid,
libxl_device_console *console)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
+    char *frontend_path = NULL;
     flexarray_t *front;
     flexarray_t *back;
     unsigned int boffset = 0;
     unsigned int foffset = 0;
     libxl__device device;
     int rc;
-
-    if (console->build_state) {
-        xs_transaction_t t;
-        char **ents = (char **) libxl__calloc(&gc, 11, sizeof(char *));
-        ents[0] = "console/port";
-        ents[1] = libxl__sprintf(&gc, "%"PRIu32,
console->build_state->console_port);
-        ents[2] = "console/ring-ref";
-        ents[3] = libxl__sprintf(&gc, "%lu",
console->build_state->console_mfn);
-        ents[4] = "console/limit";
-        ents[5] = libxl__sprintf(&gc, "%d",
LIBXL_XENCONSOLE_LIMIT);
-        ents[6] = "console/type";
-        if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
-            ents[7] = "xenconsoled";
-        else
-            ents[7] = "ioemu";
-        ents[8] = "console/output";
-        ents[9] = console->output;
-retry_transaction:
-        t = xs_transaction_start(ctx->xsh);
-        libxl__xs_writev(&gc, t, libxl__xs_get_dompath(&gc,
console->domid), ents);
-        if (!xs_transaction_end(ctx->xsh, t, 0))
-            if (errno == EAGAIN)
-                goto retry_transaction;
-    }
 
     front = flexarray_make(16, 1);
     if (!front) {
@@ -2384,29 +2361,40 @@ retry_transaction:
     flexarray_set(back, boffset++, "protocol");
     flexarray_set(back, boffset++, LIBXL_XENCONSOLE_PROTOCOL);
 
-    /* if devid == 0 do not add the frontend to device/console/ because
-     * it has already been added to console/ */
-    if (device.devid > 0) {
-        flexarray_set(front, foffset++, "backend-id");
-        flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d",
console->backend_domid));
+    flexarray_set(front, foffset++, "backend-id");
+    flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d",
console->backend_domid));
+    flexarray_set(front, foffset++, "limit");
+    flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d",
LIBXL_XENCONSOLE_LIMIT));
+    flexarray_set(front, foffset++, "type");
+    if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
+        flexarray_set(front, foffset++, "xenconsoled");
+    else
+        flexarray_set(front, foffset++, "ioemu");
+    flexarray_set(front, foffset++, "output");
+    flexarray_set(front, foffset++, console->output);
+
+    if (device.devid == 0) {
+        if (console->build_state == NULL) {
+            rc = ERROR_INVAL;
+            goto out_free;
+        }
+        frontend_path = "console";
+
+        flexarray_set(front, foffset++, "port");
+        flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%"PRIu32, console->build_state->console_port));
+        flexarray_set(front, foffset++, "ring-ref");
+        flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%lu", console->build_state->console_mfn));
+    } else {
         flexarray_set(front, foffset++, "state");
         flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d",
1));
-        flexarray_set(front, foffset++, "limit");
-        flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d",
LIBXL_XENCONSOLE_LIMIT));
         flexarray_set(front, foffset++, "protocol");
         flexarray_set(front, foffset++, LIBXL_XENCONSOLE_PROTOCOL);
-        flexarray_set(front, foffset++, "type");
-        if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
-            flexarray_set(front, foffset++, "xenconsoled");
-        else
-            flexarray_set(front, foffset++, "ioemu");
-        flexarray_set(front, foffset++, "output");
-        flexarray_set(front, foffset++, console->output);
     }
 
     libxl__device_generic_add(ctx, &device,
                              libxl__xs_kvs_of_flexarray(&gc, back,
boffset),
-                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset));
+                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset),
+                             frontend_path);
     rc = 0;
 out_free:
     flexarray_free(back);
@@ -2461,7 +2449,7 @@ int libxl_device_vkb_add(libxl_ctx *ctx,
 
     libxl__device_generic_add(ctx, &device,
                              libxl__xs_kvs_of_flexarray(&gc, back,
boffset),
-                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset));
+                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset), NULL);
     rc = 0;
 out_free:
     flexarray_free(back);
@@ -2722,7 +2710,7 @@ int libxl_device_vfb_add(libxl_ctx *ctx,
 
     libxl__device_generic_add(ctx, &device,
                              libxl__xs_kvs_of_flexarray(&gc, back,
boffset),
-                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset));
+                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset), NULL);
     rc = 0;
 out_free:
     flexarray_free(front);
diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Mon Oct 04 15:39:30 2010 +0100
+++ b/tools/libxl/libxl_device.c	Mon Oct 04 16:45:14 2010 +0100
@@ -40,10 +40,10 @@ static const char *string_of_kinds[] = {
 };
 
 int libxl__device_generic_add(libxl_ctx *ctx, libxl__device *device,
-                             char **bents, char **fents)
+                              char **bents, char **fents, char *frontend_path)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
-    char *dom_path_backend, *dom_path, *frontend_path, *backend_path;
+    char *dom_path_backend, *dom_path, *backend_path;
     xs_transaction_t t;
     struct xs_permissions frontend_perms[2];
     struct xs_permissions backend_perms[2];
@@ -57,8 +57,12 @@ int libxl__device_generic_add(libxl_ctx 
     dom_path_backend = libxl__xs_get_dompath(&gc,
device->backend_domid);
     dom_path = libxl__xs_get_dompath(&gc, device->domid);
 
-    frontend_path = libxl__sprintf(&gc, "%s/device/%s/%d",
-                                  dom_path, string_of_kinds[device->kind],
device->devid);
+    if (frontend_path)
+        frontend_path = libxl__sprintf(&gc, "%s/%s",
+                                       dom_path, frontend_path);
+    else
+        frontend_path = libxl__sprintf(&gc, "%s/device/%s/%d",
+                                       dom_path,
string_of_kinds[device->kind], device->devid);
     backend_path = libxl__sprintf(&gc, "%s/backend/%s/%u/%d",
                                  dom_path_backend,
string_of_kinds[device->backend_kind], device->domid, device->devid);
 
@@ -326,6 +330,16 @@ int libxl__devices_destroy(libxl_ctx *ct
             }
         }
     }
+
+    /* console 0 frontend directory is not under
/local/domain/<domid>/device */
+    fe_path = libxl__sprintf(&gc, "/local/domain/%d/console",
domid);
+    be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc,
"%s/backend", fe_path));
+    if (be_path && strcmp(be_path, "")) {
+        if (libxl__device_destroy(ctx, be_path, force) > 0)
+            n_watches++;
+        flexarray_set(toremove, n++, libxl__dirname(&gc, be_path));
+    }
+
     if (!force) {
         /* Linux-ism. Most implementations leave the timeout
          * untouched after select. Linux, however, will chip
diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Mon Oct 04 15:39:30 2010 +0100
+++ b/tools/libxl/libxl_internal.h	Mon Oct 04 16:45:14 2010 +0100
@@ -171,7 +171,7 @@ _hidden int libxl__device_disk_dev_numbe
 _hidden int libxl__device_disk_dev_number(char *virtpath);
 
 _hidden int libxl__device_generic_add(libxl_ctx *ctx, libxl__device *device,
-                             char **bents, char **fents);
+                                      char **bents, char **fents, char
*frontend_path);
 _hidden int libxl__device_del(libxl_ctx *ctx, libxl__device *dev, int wait);
 _hidden int libxl__device_destroy(libxl_ctx *ctx, char *be_path, int force);
 _hidden int libxl__devices_destroy(libxl_ctx *ctx, uint32_t domid, int force);
diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c	Mon Oct 04 15:39:30 2010 +0100
+++ b/tools/libxl/libxl_pci.c	Mon Oct 04 16:45:14 2010 +0100
@@ -280,7 +280,7 @@ static int libxl_create_pci_backend(libx
 
     libxl__device_generic_add(ctx, &device,
                              libxl__xs_kvs_of_flexarray(gc, back, boffset),
-                             libxl__xs_kvs_of_flexarray(gc, front, foffset));
+                             libxl__xs_kvs_of_flexarray(gc, front, foffset),
NULL);
 
     flexarray_free(back);
     flexarray_free(front);

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

The patch is good but I think it would be probably cleaner to add a
function like:

libxl__get_fe_path(libxl__device_kinds kind, int devid)

that would be called by libxl__device_generic_add to figure out the fe
path, rather than adding yet another parameter to the function,
parameter that in most cases is NULL anyway.



On Mon, 4 Oct 2010, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # Date 1286207114 -3600
> # Node ID 9e0d9b5460dc1c3cad0f5576157ac10b4045b4de
> # Parent  a2c40fa3bc6c8471e1988e445f449975cf6f3822
> libxl: remove console 0 backend directory from xenstore on devices destroy
> 
> The first PV console device has an unusual frontend path for
> historical reasons (/local/domain/<domid>/console rather than
> /local/domain/<domid>/device/console/0).
> 
> Therefore we need to check this additional frontend path as well as
> those under /local/domain/<domid>/device when tearing down all
> backends or else we miss the backend for the console.
> 
> As part of this we need to ensure that the frontend directory has a
> valid link to the backend, currently this link does not exist.
> 
> Fix this by allowing libxl__device_generic_add to take an optional
> frontend path suffix relative to the dompath allowing us to use the
> function even for setting up the special case frontend.
> 
> This also fixes the link from backend to frontend which until now gave
> a dangling link to the normal device path instead of the exceptional
> one.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c       Mon Oct 04 15:39:30 2010 +0100
> +++ b/tools/libxl/libxl.c       Mon Oct 04 16:45:14 2010 +0100
> @@ -1897,7 +1897,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
> 
>      libxl__device_generic_add(ctx, &device,
>                               libxl__xs_kvs_of_flexarray(&gc, back,
boffset),
> -                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset));
> +                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset), NULL);
> 
>      rc = 0;
> 
> @@ -2047,7 +2047,7 @@ int libxl_device_nic_add(libxl_ctx *ctx,
> 
>      libxl__device_generic_add(ctx, &device,
>                               libxl__xs_kvs_of_flexarray(&gc, back,
boffset),
> -                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset));
> +                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset), NULL);
> 
>      /* FIXME: wait for plug */
>      rc = 0;
> @@ -2232,7 +2232,7 @@ int libxl_device_net2_add(libxl_ctx *ctx
> 
>      libxl__device_generic_add(ctx, &device,
>                               libxl__xs_kvs_of_flexarray(&gc, back,
boffset),
> -                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset));
> +                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset), NULL);
> 
>      /* FIXME: wait for plug */
>      rc = 0;
> @@ -2324,36 +2324,13 @@ int libxl_device_console_add(libxl_ctx *
>  int libxl_device_console_add(libxl_ctx *ctx, uint32_t domid,
libxl_device_console *console)
>  {
>      libxl__gc gc = LIBXL_INIT_GC(ctx);
> +    char *frontend_path = NULL;
>      flexarray_t *front;
>      flexarray_t *back;
>      unsigned int boffset = 0;
>      unsigned int foffset = 0;
>      libxl__device device;
>      int rc;
> -
> -    if (console->build_state) {
> -        xs_transaction_t t;
> -        char **ents = (char **) libxl__calloc(&gc, 11, sizeof(char
*));
> -        ents[0] = "console/port";
> -        ents[1] = libxl__sprintf(&gc, "%"PRIu32,
console->build_state->console_port);
> -        ents[2] = "console/ring-ref";
> -        ents[3] = libxl__sprintf(&gc, "%lu",
console->build_state->console_mfn);
> -        ents[4] = "console/limit";
> -        ents[5] = libxl__sprintf(&gc, "%d",
LIBXL_XENCONSOLE_LIMIT);
> -        ents[6] = "console/type";
> -        if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
> -            ents[7] = "xenconsoled";
> -        else
> -            ents[7] = "ioemu";
> -        ents[8] = "console/output";
> -        ents[9] = console->output;
> -retry_transaction:
> -        t = xs_transaction_start(ctx->xsh);
> -        libxl__xs_writev(&gc, t, libxl__xs_get_dompath(&gc,
console->domid), ents);
> -        if (!xs_transaction_end(ctx->xsh, t, 0))
> -            if (errno == EAGAIN)
> -                goto retry_transaction;
> -    }
> 
>      front = flexarray_make(16, 1);
>      if (!front) {
> @@ -2384,29 +2361,40 @@ retry_transaction:
>      flexarray_set(back, boffset++, "protocol");
>      flexarray_set(back, boffset++, LIBXL_XENCONSOLE_PROTOCOL);
> 
> -    /* if devid == 0 do not add the frontend to device/console/ because
> -     * it has already been added to console/ */
> -    if (device.devid > 0) {
> -        flexarray_set(front, foffset++, "backend-id");
> -        flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%d", console->backend_domid));
> +    flexarray_set(front, foffset++, "backend-id");
> +    flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%d", console->backend_domid));
> +    flexarray_set(front, foffset++, "limit");
> +    flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%d", LIBXL_XENCONSOLE_LIMIT));
> +    flexarray_set(front, foffset++, "type");
> +    if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
> +        flexarray_set(front, foffset++, "xenconsoled");
> +    else
> +        flexarray_set(front, foffset++, "ioemu");
> +    flexarray_set(front, foffset++, "output");
> +    flexarray_set(front, foffset++, console->output);
> +
> +    if (device.devid == 0) {
> +        if (console->build_state == NULL) {
> +            rc = ERROR_INVAL;
> +            goto out_free;
> +        }
> +        frontend_path = "console";
> +
> +        flexarray_set(front, foffset++, "port");
> +        flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%"PRIu32, console->build_state->console_port));
> +        flexarray_set(front, foffset++, "ring-ref");
> +        flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%lu", console->build_state->console_mfn));
> +    } else {
>          flexarray_set(front, foffset++, "state");
>          flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%d", 1));
> -        flexarray_set(front, foffset++, "limit");
> -        flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%d", LIBXL_XENCONSOLE_LIMIT));
>          flexarray_set(front, foffset++, "protocol");
>          flexarray_set(front, foffset++, LIBXL_XENCONSOLE_PROTOCOL);
> -        flexarray_set(front, foffset++, "type");
> -        if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
> -            flexarray_set(front, foffset++, "xenconsoled");
> -        else
> -            flexarray_set(front, foffset++, "ioemu");
> -        flexarray_set(front, foffset++, "output");
> -        flexarray_set(front, foffset++, console->output);
>      }
> 
>      libxl__device_generic_add(ctx, &device,
>                               libxl__xs_kvs_of_flexarray(&gc, back,
boffset),
> -                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset));
> +                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset),
> +                             frontend_path);
>      rc = 0;
>  out_free:
>      flexarray_free(back);
> @@ -2461,7 +2449,7 @@ int libxl_device_vkb_add(libxl_ctx *ctx,
> 
>      libxl__device_generic_add(ctx, &device,
>                               libxl__xs_kvs_of_flexarray(&gc, back,
boffset),
> -                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset));
> +                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset), NULL);
>      rc = 0;
>  out_free:
>      flexarray_free(back);
> @@ -2722,7 +2710,7 @@ int libxl_device_vfb_add(libxl_ctx *ctx,
> 
>      libxl__device_generic_add(ctx, &device,
>                               libxl__xs_kvs_of_flexarray(&gc, back,
boffset),
> -                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset));
> +                             libxl__xs_kvs_of_flexarray(&gc, front,
foffset), NULL);
>      rc = 0;
>  out_free:
>      flexarray_free(front);
> diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_device.c
> --- a/tools/libxl/libxl_device.c        Mon Oct 04 15:39:30 2010 +0100
> +++ b/tools/libxl/libxl_device.c        Mon Oct 04 16:45:14 2010 +0100
> @@ -40,10 +40,10 @@ static const char *string_of_kinds[] = {
>  };
> 
>  int libxl__device_generic_add(libxl_ctx *ctx, libxl__device *device,
> -                             char **bents, char **fents)
> +                              char **bents, char **fents, char
*frontend_path)
>  {
>      libxl__gc gc = LIBXL_INIT_GC(ctx);
> -    char *dom_path_backend, *dom_path, *frontend_path, *backend_path;
> +    char *dom_path_backend, *dom_path, *backend_path;
>      xs_transaction_t t;
>      struct xs_permissions frontend_perms[2];
>      struct xs_permissions backend_perms[2];
> @@ -57,8 +57,12 @@ int libxl__device_generic_add(libxl_ctx
>      dom_path_backend = libxl__xs_get_dompath(&gc,
device->backend_domid);
>      dom_path = libxl__xs_get_dompath(&gc, device->domid);
> 
> -    frontend_path = libxl__sprintf(&gc, "%s/device/%s/%d",
> -                                  dom_path,
string_of_kinds[device->kind], device->devid);
> +    if (frontend_path)
> +        frontend_path = libxl__sprintf(&gc, "%s/%s",
> +                                       dom_path, frontend_path);
> +    else
> +        frontend_path = libxl__sprintf(&gc,
"%s/device/%s/%d",
> +                                       dom_path,
string_of_kinds[device->kind], device->devid);
>      backend_path = libxl__sprintf(&gc,
"%s/backend/%s/%u/%d",
>                                   dom_path_backend,
string_of_kinds[device->backend_kind], device->domid, device->devid);
> 
> @@ -326,6 +330,16 @@ int libxl__devices_destroy(libxl_ctx *ct
>              }
>          }
>      }
> +
> +    /* console 0 frontend directory is not under
/local/domain/<domid>/device */
> +    fe_path = libxl__sprintf(&gc,
"/local/domain/%d/console", domid);
> +    be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc,
"%s/backend", fe_path));
> +    if (be_path && strcmp(be_path, "")) {
> +        if (libxl__device_destroy(ctx, be_path, force) > 0)
> +            n_watches++;
> +        flexarray_set(toremove, n++, libxl__dirname(&gc, be_path));
> +    }
> +
>      if (!force) {
>          /* Linux-ism. Most implementations leave the timeout
>           * untouched after select. Linux, however, will chip
> diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h      Mon Oct 04 15:39:30 2010 +0100
> +++ b/tools/libxl/libxl_internal.h      Mon Oct 04 16:45:14 2010 +0100
> @@ -171,7 +171,7 @@ _hidden int libxl__device_disk_dev_numbe
>  _hidden int libxl__device_disk_dev_number(char *virtpath);
> 
>  _hidden int libxl__device_generic_add(libxl_ctx *ctx, libxl__device
*device,
> -                             char **bents, char **fents);
> +                                      char **bents, char **fents, char
*frontend_path);
>  _hidden int libxl__device_del(libxl_ctx *ctx, libxl__device *dev, int
wait);
>  _hidden int libxl__device_destroy(libxl_ctx *ctx, char *be_path, int
force);
>  _hidden int libxl__devices_destroy(libxl_ctx *ctx, uint32_t domid, int
force);
> diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_pci.c
> --- a/tools/libxl/libxl_pci.c   Mon Oct 04 15:39:30 2010 +0100
> +++ b/tools/libxl/libxl_pci.c   Mon Oct 04 16:45:14 2010 +0100
> @@ -280,7 +280,7 @@ static int libxl_create_pci_backend(libx
> 
>      libxl__device_generic_add(ctx, &device,
>                               libxl__xs_kvs_of_flexarray(gc, back,
boffset),
> -                             libxl__xs_kvs_of_flexarray(gc, front,
foffset));
> +                             libxl__xs_kvs_of_flexarray(gc, front,
foffset), NULL);
> 
>      flexarray_free(back);
>      flexarray_free(front);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tue, 2010-10-05 at 14:39 +0100, Stefano Stabellini
wrote:
> The patch is good but I think it would be probably cleaner to add a
> function like:
> 
> libxl__get_fe_path(libxl__device_kinds kind, int devid)
> 
> that would be called by libxl__device_generic_add to figure out the fe
> path, rather than adding yet another parameter to the function,
> parameter that in most cases is NULL anyway.
Yes, that''s a good idea.
> 
> 
> 
> On Mon, 4 Oct 2010, Ian Campbell wrote:
> > # HG changeset patch
> > # User Ian Campbell <ian.campbell@citrix.com>
> > # Date 1286207114 -3600
> > # Node ID 9e0d9b5460dc1c3cad0f5576157ac10b4045b4de
> > # Parent  a2c40fa3bc6c8471e1988e445f449975cf6f3822
> > libxl: remove console 0 backend directory from xenstore on devices
destroy
> > 
> > The first PV console device has an unusual frontend path for
> > historical reasons (/local/domain/<domid>/console rather than
> > /local/domain/<domid>/device/console/0).
> > 
> > Therefore we need to check this additional frontend path as well as
> > those under /local/domain/<domid>/device when tearing down all
> > backends or else we miss the backend for the console.
> > 
> > As part of this we need to ensure that the frontend directory has a
> > valid link to the backend, currently this link does not exist.
> > 
> > Fix this by allowing libxl__device_generic_add to take an optional
> > frontend path suffix relative to the dompath allowing us to use the
> > function even for setting up the special case frontend.
> > 
> > This also fixes the link from backend to frontend which until now gave
> > a dangling link to the normal device path instead of the exceptional
> > one.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl.c
> > --- a/tools/libxl/libxl.c       Mon Oct 04 15:39:30 2010 +0100
> > +++ b/tools/libxl/libxl.c       Mon Oct 04 16:45:14 2010 +0100
> > @@ -1897,7 +1897,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
> > 
> >      libxl__device_generic_add(ctx, &device,
> >                               libxl__xs_kvs_of_flexarray(&gc,
back, boffset),
> > -                             libxl__xs_kvs_of_flexarray(&gc,
front, foffset));
> > +                             libxl__xs_kvs_of_flexarray(&gc,
front, foffset), NULL);
> > 
> >      rc = 0;
> > 
> > @@ -2047,7 +2047,7 @@ int libxl_device_nic_add(libxl_ctx *ctx,
> > 
> >      libxl__device_generic_add(ctx, &device,
> >                               libxl__xs_kvs_of_flexarray(&gc,
back, boffset),
> > -                             libxl__xs_kvs_of_flexarray(&gc,
front, foffset));
> > +                             libxl__xs_kvs_of_flexarray(&gc,
front, foffset), NULL);
> > 
> >      /* FIXME: wait for plug */
> >      rc = 0;
> > @@ -2232,7 +2232,7 @@ int libxl_device_net2_add(libxl_ctx *ctx
> > 
> >      libxl__device_generic_add(ctx, &device,
> >                               libxl__xs_kvs_of_flexarray(&gc,
back, boffset),
> > -                             libxl__xs_kvs_of_flexarray(&gc,
front, foffset));
> > +                             libxl__xs_kvs_of_flexarray(&gc,
front, foffset), NULL);
> > 
> >      /* FIXME: wait for plug */
> >      rc = 0;
> > @@ -2324,36 +2324,13 @@ int libxl_device_console_add(libxl_ctx *
> >  int libxl_device_console_add(libxl_ctx *ctx, uint32_t domid,
libxl_device_console *console)
> >  {
> >      libxl__gc gc = LIBXL_INIT_GC(ctx);
> > +    char *frontend_path = NULL;
> >      flexarray_t *front;
> >      flexarray_t *back;
> >      unsigned int boffset = 0;
> >      unsigned int foffset = 0;
> >      libxl__device device;
> >      int rc;
> > -
> > -    if (console->build_state) {
> > -        xs_transaction_t t;
> > -        char **ents = (char **) libxl__calloc(&gc, 11,
sizeof(char *));
> > -        ents[0] = "console/port";
> > -        ents[1] = libxl__sprintf(&gc, "%"PRIu32,
console->build_state->console_port);
> > -        ents[2] = "console/ring-ref";
> > -        ents[3] = libxl__sprintf(&gc, "%lu",
console->build_state->console_mfn);
> > -        ents[4] = "console/limit";
> > -        ents[5] = libxl__sprintf(&gc, "%d",
LIBXL_XENCONSOLE_LIMIT);
> > -        ents[6] = "console/type";
> > -        if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
> > -            ents[7] = "xenconsoled";
> > -        else
> > -            ents[7] = "ioemu";
> > -        ents[8] = "console/output";
> > -        ents[9] = console->output;
> > -retry_transaction:
> > -        t = xs_transaction_start(ctx->xsh);
> > -        libxl__xs_writev(&gc, t, libxl__xs_get_dompath(&gc,
console->domid), ents);
> > -        if (!xs_transaction_end(ctx->xsh, t, 0))
> > -            if (errno == EAGAIN)
> > -                goto retry_transaction;
> > -    }
> > 
> >      front = flexarray_make(16, 1);
> >      if (!front) {
> > @@ -2384,29 +2361,40 @@ retry_transaction:
> >      flexarray_set(back, boffset++, "protocol");
> >      flexarray_set(back, boffset++, LIBXL_XENCONSOLE_PROTOCOL);
> > 
> > -    /* if devid == 0 do not add the frontend to device/console/
because
> > -     * it has already been added to console/ */
> > -    if (device.devid > 0) {
> > -        flexarray_set(front, foffset++, "backend-id");
> > -        flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%d", console->backend_domid));
> > +    flexarray_set(front, foffset++, "backend-id");
> > +    flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%d", console->backend_domid));
> > +    flexarray_set(front, foffset++, "limit");
> > +    flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%d", LIBXL_XENCONSOLE_LIMIT));
> > +    flexarray_set(front, foffset++, "type");
> > +    if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
> > +        flexarray_set(front, foffset++, "xenconsoled");
> > +    else
> > +        flexarray_set(front, foffset++, "ioemu");
> > +    flexarray_set(front, foffset++, "output");
> > +    flexarray_set(front, foffset++, console->output);
> > +
> > +    if (device.devid == 0) {
> > +        if (console->build_state == NULL) {
> > +            rc = ERROR_INVAL;
> > +            goto out_free;
> > +        }
> > +        frontend_path = "console";
> > +
> > +        flexarray_set(front, foffset++, "port");
> > +        flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%"PRIu32, console->build_state->console_port));
> > +        flexarray_set(front, foffset++, "ring-ref");
> > +        flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%lu", console->build_state->console_mfn));
> > +    } else {
> >          flexarray_set(front, foffset++, "state");
> >          flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%d", 1));
> > -        flexarray_set(front, foffset++, "limit");
> > -        flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%d", LIBXL_XENCONSOLE_LIMIT));
> >          flexarray_set(front, foffset++, "protocol");
> >          flexarray_set(front, foffset++, LIBXL_XENCONSOLE_PROTOCOL);
> > -        flexarray_set(front, foffset++, "type");
> > -        if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
> > -            flexarray_set(front, foffset++, "xenconsoled");
> > -        else
> > -            flexarray_set(front, foffset++, "ioemu");
> > -        flexarray_set(front, foffset++, "output");
> > -        flexarray_set(front, foffset++, console->output);
> >      }
> > 
> >      libxl__device_generic_add(ctx, &device,
> >                               libxl__xs_kvs_of_flexarray(&gc,
back, boffset),
> > -                             libxl__xs_kvs_of_flexarray(&gc,
front, foffset));
> > +                             libxl__xs_kvs_of_flexarray(&gc,
front, foffset),
> > +                             frontend_path);
> >      rc = 0;
> >  out_free:
> >      flexarray_free(back);
> > @@ -2461,7 +2449,7 @@ int libxl_device_vkb_add(libxl_ctx *ctx,
> > 
> >      libxl__device_generic_add(ctx, &device,
> >                               libxl__xs_kvs_of_flexarray(&gc,
back, boffset),
> > -                             libxl__xs_kvs_of_flexarray(&gc,
front, foffset));
> > +                             libxl__xs_kvs_of_flexarray(&gc,
front, foffset), NULL);
> >      rc = 0;
> >  out_free:
> >      flexarray_free(back);
> > @@ -2722,7 +2710,7 @@ int libxl_device_vfb_add(libxl_ctx *ctx,
> > 
> >      libxl__device_generic_add(ctx, &device,
> >                               libxl__xs_kvs_of_flexarray(&gc,
back, boffset),
> > -                             libxl__xs_kvs_of_flexarray(&gc,
front, foffset));
> > +                             libxl__xs_kvs_of_flexarray(&gc,
front, foffset), NULL);
> >      rc = 0;
> >  out_free:
> >      flexarray_free(front);
> > diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_device.c
> > --- a/tools/libxl/libxl_device.c        Mon Oct 04 15:39:30 2010 +0100
> > +++ b/tools/libxl/libxl_device.c        Mon Oct 04 16:45:14 2010 +0100
> > @@ -40,10 +40,10 @@ static const char *string_of_kinds[] = {
> >  };
> > 
> >  int libxl__device_generic_add(libxl_ctx *ctx, libxl__device *device,
> > -                             char **bents, char **fents)
> > +                              char **bents, char **fents, char
*frontend_path)
> >  {
> >      libxl__gc gc = LIBXL_INIT_GC(ctx);
> > -    char *dom_path_backend, *dom_path, *frontend_path, *backend_path;
> > +    char *dom_path_backend, *dom_path, *backend_path;
> >      xs_transaction_t t;
> >      struct xs_permissions frontend_perms[2];
> >      struct xs_permissions backend_perms[2];
> > @@ -57,8 +57,12 @@ int libxl__device_generic_add(libxl_ctx
> >      dom_path_backend = libxl__xs_get_dompath(&gc,
device->backend_domid);
> >      dom_path = libxl__xs_get_dompath(&gc, device->domid);
> > 
> > -    frontend_path = libxl__sprintf(&gc,
"%s/device/%s/%d",
> > -                                  dom_path,
string_of_kinds[device->kind], device->devid);
> > +    if (frontend_path)
> > +        frontend_path = libxl__sprintf(&gc, "%s/%s",
> > +                                       dom_path, frontend_path);
> > +    else
> > +        frontend_path = libxl__sprintf(&gc,
"%s/device/%s/%d",
> > +                                       dom_path,
string_of_kinds[device->kind], device->devid);
> >      backend_path = libxl__sprintf(&gc,
"%s/backend/%s/%u/%d",
> >                                   dom_path_backend,
string_of_kinds[device->backend_kind], device->domid, device->devid);
> > 
> > @@ -326,6 +330,16 @@ int libxl__devices_destroy(libxl_ctx *ct
> >              }
> >          }
> >      }
> > +
> > +    /* console 0 frontend directory is not under
/local/domain/<domid>/device */
> > +    fe_path = libxl__sprintf(&gc,
"/local/domain/%d/console", domid);
> > +    be_path = libxl__xs_read(&gc, XBT_NULL,
libxl__sprintf(&gc, "%s/backend", fe_path));
> > +    if (be_path && strcmp(be_path, "")) {
> > +        if (libxl__device_destroy(ctx, be_path, force) > 0)
> > +            n_watches++;
> > +        flexarray_set(toremove, n++, libxl__dirname(&gc,
be_path));
> > +    }
> > +
> >      if (!force) {
> >          /* Linux-ism. Most implementations leave the timeout
> >           * untouched after select. Linux, however, will chip
> > diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_internal.h
> > --- a/tools/libxl/libxl_internal.h      Mon Oct 04 15:39:30 2010 +0100
> > +++ b/tools/libxl/libxl_internal.h      Mon Oct 04 16:45:14 2010 +0100
> > @@ -171,7 +171,7 @@ _hidden int libxl__device_disk_dev_numbe
> >  _hidden int libxl__device_disk_dev_number(char *virtpath);
> > 
> >  _hidden int libxl__device_generic_add(libxl_ctx *ctx, libxl__device
*device,
> > -                             char **bents, char **fents);
> > +                                      char **bents, char **fents,
char *frontend_path);
> >  _hidden int libxl__device_del(libxl_ctx *ctx, libxl__device *dev, int
wait);
> >  _hidden int libxl__device_destroy(libxl_ctx *ctx, char *be_path, int
force);
> >  _hidden int libxl__devices_destroy(libxl_ctx *ctx, uint32_t domid,
int force);
> > diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_pci.c
> > --- a/tools/libxl/libxl_pci.c   Mon Oct 04 15:39:30 2010 +0100
> > +++ b/tools/libxl/libxl_pci.c   Mon Oct 04 16:45:14 2010 +0100
> > @@ -280,7 +280,7 @@ static int libxl_create_pci_backend(libx
> > 
> >      libxl__device_generic_add(ctx, &device,
> >                               libxl__xs_kvs_of_flexarray(gc, back,
boffset),
> > -                             libxl__xs_kvs_of_flexarray(gc, front,
foffset));
> > +                             libxl__xs_kvs_of_flexarray(gc, front,
foffset), NULL);
> > 
> >      flexarray_free(back);
> >      flexarray_free(front);
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> > 


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

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1286288339 -3600
# Node ID 0788bb897f692681036e33c0047e8cbb63565c35
# Parent  a2c40fa3bc6c8471e1988e445f449975cf6f3822
libxl: remove console 0 backend directory from xenstore on devices destroy

The first PV console device has an unusual frontend path for
historical reasons (/local/domain/<domid>/console rather than
/local/domain/<domid>/device/console/0).

Therefore we need to check this additional frontend path as well as
those under /local/domain/<domid>/device when tearing down all
backends or else we miss the backend for the console.

As part of this we need to ensure that the frontend directory has a
valid link to the backend, currently this link does not exist.

Fix this by adding libxl__device_frontend_path to return the correct
frontend path, accounting for the special case, and use it from
libxl__device_add_generic. Also add libxl__device_backend_path for
consistency.

This also allows console 0 setup to follow essentially the same code
path as other consoles within libxl_device_console_add, reducing the
special casing required there.

This also fixes the link from backend to frontend which until now gave
a dangling link to the normal device path instead of the exceptional
one.

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

diff -r a2c40fa3bc6c -r 0788bb897f69 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Oct 04 15:39:30 2010 +0100
+++ b/tools/libxl/libxl.c	Tue Oct 05 15:18:59 2010 +0100
@@ -2331,30 +2331,6 @@ int libxl_device_console_add(libxl_ctx *
     libxl__device device;
     int rc;
 
-    if (console->build_state) {
-        xs_transaction_t t;
-        char **ents = (char **) libxl__calloc(&gc, 11, sizeof(char *));
-        ents[0] = "console/port";
-        ents[1] = libxl__sprintf(&gc, "%"PRIu32,
console->build_state->console_port);
-        ents[2] = "console/ring-ref";
-        ents[3] = libxl__sprintf(&gc, "%lu",
console->build_state->console_mfn);
-        ents[4] = "console/limit";
-        ents[5] = libxl__sprintf(&gc, "%d",
LIBXL_XENCONSOLE_LIMIT);
-        ents[6] = "console/type";
-        if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
-            ents[7] = "xenconsoled";
-        else
-            ents[7] = "ioemu";
-        ents[8] = "console/output";
-        ents[9] = console->output;
-retry_transaction:
-        t = xs_transaction_start(ctx->xsh);
-        libxl__xs_writev(&gc, t, libxl__xs_get_dompath(&gc,
console->domid), ents);
-        if (!xs_transaction_end(ctx->xsh, t, 0))
-            if (errno == EAGAIN)
-                goto retry_transaction;
-    }
-
     front = flexarray_make(16, 1);
     if (!front) {
         rc = ERROR_NOMEM;
@@ -2384,24 +2360,32 @@ retry_transaction:
     flexarray_set(back, boffset++, "protocol");
     flexarray_set(back, boffset++, LIBXL_XENCONSOLE_PROTOCOL);
 
-    /* if devid == 0 do not add the frontend to device/console/ because
-     * it has already been added to console/ */
-    if (device.devid > 0) {
-        flexarray_set(front, foffset++, "backend-id");
-        flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d",
console->backend_domid));
+    flexarray_set(front, foffset++, "backend-id");
+    flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d",
console->backend_domid));
+    flexarray_set(front, foffset++, "limit");
+    flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d",
LIBXL_XENCONSOLE_LIMIT));
+    flexarray_set(front, foffset++, "type");
+    if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
+        flexarray_set(front, foffset++, "xenconsoled");
+    else
+        flexarray_set(front, foffset++, "ioemu");
+    flexarray_set(front, foffset++, "output");
+    flexarray_set(front, foffset++, console->output);
+
+    if (device.devid == 0) {
+        if (console->build_state == NULL) {
+            rc = ERROR_INVAL;
+            goto out_free;
+        }
+        flexarray_set(front, foffset++, "port");
+        flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%"PRIu32, console->build_state->console_port));
+        flexarray_set(front, foffset++, "ring-ref");
+        flexarray_set(front, foffset++, libxl__sprintf(&gc,
"%lu", console->build_state->console_mfn));
+    } else {
         flexarray_set(front, foffset++, "state");
         flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d",
1));
-        flexarray_set(front, foffset++, "limit");
-        flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d",
LIBXL_XENCONSOLE_LIMIT));
         flexarray_set(front, foffset++, "protocol");
         flexarray_set(front, foffset++, LIBXL_XENCONSOLE_PROTOCOL);
-        flexarray_set(front, foffset++, "type");
-        if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
-            flexarray_set(front, foffset++, "xenconsoled");
-        else
-            flexarray_set(front, foffset++, "ioemu");
-        flexarray_set(front, foffset++, "output");
-        flexarray_set(front, foffset++, console->output);
     }
 
     libxl__device_generic_add(ctx, &device,
diff -r a2c40fa3bc6c -r 0788bb897f69 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Mon Oct 04 15:39:30 2010 +0100
+++ b/tools/libxl/libxl_device.c	Tue Oct 05 15:18:59 2010 +0100
@@ -39,11 +39,32 @@ static const char *string_of_kinds[] = {
     [DEVICE_CONSOLE] = "console",
 };
 
+static char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device)
+{
+    char *dom_path = libxl__xs_get_dompath(gc, device->domid);
+
+    /* Console 0 is a special case */
+    if (device->kind == DEVICE_CONSOLE && device->devid == 0)
+        return libxl__sprintf(gc, "%s/console", dom_path);
+
+    return libxl__sprintf(gc, "%s/device/%s/%d", dom_path,
+                          string_of_kinds[device->kind], device->devid);
+}
+
+static char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device)
+{
+    char *dom_path = libxl__xs_get_dompath(gc, device->backend_domid);
+
+    return libxl__sprintf(gc, "%s/backend/%s/%u/%d", dom_path,
+                          string_of_kinds[device->backend_kind],
+                          device->domid, device->devid);
+}
+
 int libxl__device_generic_add(libxl_ctx *ctx, libxl__device *device,
                              char **bents, char **fents)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
-    char *dom_path_backend, *dom_path, *frontend_path, *backend_path;
+    char *frontend_path, *backend_path;
     xs_transaction_t t;
     struct xs_permissions frontend_perms[2];
     struct xs_permissions backend_perms[2];
@@ -54,13 +75,8 @@ int libxl__device_generic_add(libxl_ctx 
         goto out;
     }
 
-    dom_path_backend = libxl__xs_get_dompath(&gc,
device->backend_domid);
-    dom_path = libxl__xs_get_dompath(&gc, device->domid);
-
-    frontend_path = libxl__sprintf(&gc, "%s/device/%s/%d",
-                                  dom_path, string_of_kinds[device->kind],
device->devid);
-    backend_path = libxl__sprintf(&gc, "%s/backend/%s/%u/%d",
-                                 dom_path_backend,
string_of_kinds[device->backend_kind], device->domid, device->devid);
+    frontend_path = libxl__device_frontend_path(&gc, device);
+    backend_path = libxl__device_backend_path(&gc, device);
 
     frontend_perms[0].id = device->domid;
     frontend_perms[0].perms = XS_PERM_NONE;
@@ -326,6 +342,16 @@ int libxl__devices_destroy(libxl_ctx *ct
             }
         }
     }
+
+    /* console 0 frontend directory is not under
/local/domain/<domid>/device */
+    fe_path = libxl__sprintf(&gc, "/local/domain/%d/console",
domid);
+    be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc,
"%s/backend", fe_path));
+    if (be_path && strcmp(be_path, "")) {
+        if (libxl__device_destroy(ctx, be_path, force) > 0)
+            n_watches++;
+        flexarray_set(toremove, n++, libxl__dirname(&gc, be_path));
+    }
+
     if (!force) {
         /* Linux-ism. Most implementations leave the timeout
          * untouched after select. Linux, however, will chip
@@ -356,15 +382,11 @@ int libxl__device_del(libxl_ctx *ctx, li
 int libxl__device_del(libxl_ctx *ctx, libxl__device *dev, int wait)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
-    char *dom_path_backend, *backend_path;
+    char *backend_path;
     int rc;
 
-    /* Create strings */
-    dom_path_backend    = libxl__xs_get_dompath(&gc,
dev->backend_domid);
-    backend_path        = libxl__sprintf(&gc,
"%s/backend/%s/%u/%d",
-                                    dom_path_backend, 
-                                    string_of_kinds[dev->backend_kind], 
-                                    dev->domid, dev->devid);
+    backend_path = libxl__device_backend_path(&gc, dev);
+
     rc = libxl__device_destroy(ctx, backend_path, !wait);
     if (rc == -1) {
         rc = ERROR_FAIL;

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

On Tue, 5 Oct 2010, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # Date 1286288339 -3600
> # Node ID 0788bb897f692681036e33c0047e8cbb63565c35
> # Parent  a2c40fa3bc6c8471e1988e445f449975cf6f3822
> libxl: remove console 0 backend directory from xenstore on devices destroy
> 
> The first PV console device has an unusual frontend path for
> historical reasons (/local/domain/<domid>/console rather than
> /local/domain/<domid>/device/console/0).
> 
> Therefore we need to check this additional frontend path as well as
> those under /local/domain/<domid>/device when tearing down all
> backends or else we miss the backend for the console.
> 
> As part of this we need to ensure that the frontend directory has a
> valid link to the backend, currently this link does not exist.
> 
> Fix this by adding libxl__device_frontend_path to return the correct
> frontend path, accounting for the special case, and use it from
> libxl__device_add_generic. Also add libxl__device_backend_path for
> consistency.
> 
> This also allows console 0 setup to follow essentially the same code
> path as other consoles within libxl_device_console_add, reducing the
> special casing required there.
> 
> This also fixes the link from backend to frontend which until now gave
> a dangling link to the normal device path instead of the exceptional
> one.
> 
applied, thanks

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