[PATCH 1/7] libxenlight resend: refactor libxl destroy code

[PATCH 1/7] libxenlight resend: refactor libxl destroy code

[Andres Lagar-Cavilla]

[-- Attachment #1: Type: text/plain, Size: 314 bytes --]

Refactor libxl device destroy code. Abstract function
waiting for the watch on the state node to fire.
Create a generic device delete function.

Only a single LIBXL_DESTROY_TIMEOUT elapses when
waiting for destruction of all the devices of a
domain.

Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.com>


[-- Attachment #2: 01_refactor_destroy.patch --]
[-- Type: text/plain, Size: 5464 bytes --]

# HG changeset patch
# User Andres Lagar-Cavilla <andres@lagarcavilla.com>
# Date 1259765699 18000
# Node ID 68b17d151db6c9a31ca55c359742f0d3515a0a5a
# Parent  9eadd29fc3e121e40251ffe3add2c300f0d8cd23
Refactor libxl device destroy code. Abstract function
waiting for the watch on the state node to fire.
Create a generic device delete function.

Only a single LIBXL_DESTROY_TIMEOUT elapses when 
waiting for destruction of all the devices of a 
domain.

Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.com>

diff -r 9eadd29fc3e1 -r 68b17d151db6 libxl_device.c
--- a/libxl_device.c
+++ b/libxl_device.c
@@ -203,14 +203,40 @@ retry_transaction:
         return 0;
 }
 
+int wait_for_dev_destroy(struct libxl_ctx *ctx, struct timeval *tv)
+{
+    int nfds, rc;
+    unsigned int n;
+    fd_set rfds;
+    char **l1 = NULL;
+
+    rc = 1;
+    nfds = xs_fileno(ctx->xsh) + 1;
+    FD_ZERO(&rfds);
+    FD_SET(xs_fileno(ctx->xsh), &rfds);
+    if (select(nfds, &rfds, NULL, NULL, tv) > 0) {
+        l1 = xs_read_watch(ctx->xsh, &n);
+        if (l1 != NULL) {
+            char *state = libxl_xs_read(ctx, XBT_NULL, l1[XS_WATCH_PATH]);
+            if (!state || atoi(state) == 6) {
+                xs_unwatch(ctx->xsh, l1[0], l1[1]);
+                xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
+                XL_LOG(ctx, XL_LOG_DEBUG, "Destroyed device backend at %s", l1[XS_WATCH_TOKEN]);
+                rc = 0;
+            }
+            libxl_free(ctx, state);
+            free(l1);
+        }
+    }
+    return rc;
+}
+
 int libxl_devices_destroy(struct libxl_ctx *ctx, uint32_t domid, int force)
 {
     char *path, *be_path, *fe_path;
     unsigned int num1, num2;
     char **l1 = NULL, **l2 = NULL;
-    int i, j, nfds, n = 0, n_watches = 0;
-    fd_set rfds;
-    struct timeval tv;
+    int i, j, n = 0, n_watches = 0;
     flexarray_t *toremove;
 
     toremove = flexarray_make(16, 1);
@@ -238,27 +264,20 @@ int libxl_devices_destroy(struct libxl_c
         }
     }
     if (!force) {
-        nfds = xs_fileno(ctx->xsh) + 1;
-        /* Linux-ism */
+        /* Linux-ism. Most implementations leave the timeout
+         * untouched after select. Linux, however, will chip
+         * away the elapsed time from it, which is what we 
+         * need to enforce a single time span waiting for
+         * device destruction. */
+        struct timeval tv;
         tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
         tv.tv_usec = 0;
-        while (n_watches > 0 && tv.tv_sec > 0) {
-            FD_ZERO(&rfds);
-            FD_SET(xs_fileno(ctx->xsh), &rfds);
-            if (select(nfds, &rfds, NULL, NULL, &tv) > 0) {
-                l1 = xs_read_watch(ctx->xsh, &num1);
-                if (l1 != NULL) {
-                    char *state = libxl_xs_read(ctx, XBT_NULL, l1[0]);
-                    if (!state || atoi(state) == 6) {
-                        xs_unwatch(ctx->xsh, l1[0], l1[1]);
-                        xs_rm(ctx->xsh, XBT_NULL, l1[1]);
-                        XL_LOG(ctx, XL_LOG_DEBUG, "Destroyed device backend at %s", l1[1]);
-                        n_watches--;
-                    }
-                    free(l1);
-                }
-            } else
+        while (n_watches > 0) {
+            if (wait_for_dev_destroy(ctx, &tv)) {
                 break;
+            } else {
+                n_watches--;
+            }
         }
     }
     for (i = 0; i < n; i++) {
@@ -269,6 +288,41 @@ int libxl_devices_destroy(struct libxl_c
     return 0;
 }
 
+int libxl_device_del(struct libxl_ctx *ctx, libxl_device *dev, int wait)
+{
+    char *dom_path_backend, *backend_path, *hotplug_path;
+    int rc;
+
+    /* Create strings */
+    dom_path_backend    = libxl_xs_get_dompath(ctx, dev->backend_domid);
+    backend_path        = libxl_sprintf(ctx, "%s/backend/%s/%u/%d",
+                                    dom_path_backend, 
+                                    string_of_kinds[dev->backend_kind], 
+                                    dev->domid, dev->devid);
+    hotplug_path        = libxl_sprintf(ctx, "/xapi/%d/hotplug/%s/%d",
+                                    dev->domid,
+                                    string_of_kinds[dev->kind], 
+                                    dev->devid);
+    libxl_free(ctx, dom_path_backend);
+
+    rc = libxl_device_destroy(ctx, backend_path, !wait);
+    if (rc == -1) {
+        return ERROR_FAIL;
+    }
+
+    if (wait) {
+        struct timeval tv;
+        tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
+        tv.tv_usec = 0;
+        (void)wait_for_dev_destroy(ctx, &tv);
+    }
+
+    xs_rm(ctx->xsh, XBT_NULL, hotplug_path);
+    libxl_free(ctx, hotplug_path);
+    libxl_free(ctx, backend_path);
+    return 0;
+}
+
 int libxl_device_pci_flr(struct libxl_ctx *ctx, unsigned int domain, unsigned int bus,
                          unsigned int dev, unsigned int func)
 {
diff -r 9eadd29fc3e1 -r 68b17d151db6 libxl_internal.h
--- a/libxl_internal.h
+++ b/libxl_internal.h
@@ -106,6 +106,7 @@ int device_disk_dev_number(char *virtpat
 
 int libxl_device_generic_add(struct libxl_ctx *ctx, libxl_device *device,
                              char **bents, char **fents);
+int libxl_device_del(struct libxl_ctx *ctx, libxl_device *dev, int wait);
 int libxl_device_destroy(struct libxl_ctx *ctx, char *be_path, int force);
 int libxl_devices_destroy(struct libxl_ctx *ctx, uint32_t domid, int force);
 int libxl_wait_for_device_model(struct libxl_ctx *ctx,

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH 1/7] libxenlight resend: refactor libxl destroy code

[Andres Lagar-Cavilla]

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

Re-resend against 20568:aa0b5558564f, plus the patch I just submitted 
for cloning contexts. The rest of the patches in the series need no 
modifications afaik.
Thanks,
Andres
Andres Lagar-Cavilla wrote:
> Refactor libxl device destroy code. Abstract function
> waiting for the watch on the state node to fire.
> Create a generic device delete function.
>
> Only a single LIBXL_DESTROY_TIMEOUT elapses when
> waiting for destruction of all the devices of a
> domain.
>
> Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.com>
>


[-- Attachment #2: 01_refactor_destroy.patch --]
[-- Type: text/x-patch, Size: 5872 bytes --]

# HG changeset patch
# User Andres Lagar-Cavilla <andres@lagarcavilla.com>
# Date 1259791548 18000
# Node ID 889744de9da0228b007742e1feeac90ddb537d44
# Parent  e8611d9965607659d2cb16864694983f35c2d83a
Refactor libxl device destroy code. Abstract function
waiting for the watch on the state node to fire.
Create a generic device delete function.

Only a single LIBXL_DESTROY_TIMEOUT elapses when
waiting for destruction of all the devices of a
domain.

Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.com>

diff -r e8611d996560 -r 889744de9da0 libxl_device.c
--- a/libxl_device.c
+++ b/libxl_device.c
@@ -203,14 +203,40 @@ retry_transaction:
         return 0;
 }
 
+int wait_for_dev_destroy(struct libxl_ctx *ctx, struct timeval *tv)
+{
+    int nfds, rc;
+    unsigned int n;
+    fd_set rfds;
+    char **l1 = NULL;
+
+    rc = 1;
+    nfds = xs_fileno(ctx->xsh) + 1;
+    FD_ZERO(&rfds);
+    FD_SET(xs_fileno(ctx->xsh), &rfds);
+    if (select(nfds, &rfds, NULL, NULL, tv) > 0) {
+        l1 = xs_read_watch(ctx->xsh, &n);
+        if (l1 != NULL) {
+            char *state = libxl_xs_read(ctx, XBT_NULL, l1[XS_WATCH_PATH]);
+            if (!state || atoi(state) == 6) {
+                xs_unwatch(ctx->xsh, l1[0], l1[1]);
+                xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
+                XL_LOG(ctx, XL_LOG_DEBUG, "Destroyed device backend at %s", l1[XS_WATCH_TOKEN]);
+                rc = 0;
+            }
+            libxl_free(ctx, state);
+            free(l1);
+        }
+    }
+    return rc;
+}
+
 int libxl_devices_destroy(struct libxl_ctx *ctx, uint32_t domid, int force)
 {
     char *path, *be_path, *fe_path;
     unsigned int num1, num2;
     char **l1 = NULL, **l2 = NULL;
-    int i, j, nfds, n = 0, n_watches = 0;
-    fd_set rfds;
-    struct timeval tv;
+    int i, j, n = 0, n_watches = 0;
     flexarray_t *toremove;
     struct libxl_ctx clone;
 
@@ -247,27 +273,20 @@ int libxl_devices_destroy(struct libxl_c
         }
     }
     if (!force) {
-        nfds = xs_fileno(clone.xsh) + 1;
-        /* Linux-ism */
+        /* Linux-ism. Most implementations leave the timeout
+         * untouched after select. Linux, however, will chip
+         * away the elapsed time from it, which is what we 
+         * need to enforce a single time span waiting for
+         * device destruction. */
+        struct timeval tv;
         tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
         tv.tv_usec = 0;
-        while (n_watches > 0 && tv.tv_sec > 0) {
-            FD_ZERO(&rfds);
-            FD_SET(xs_fileno(clone.xsh), &rfds);
-            if (select(nfds, &rfds, NULL, NULL, &tv) > 0) {
-                l1 = xs_read_watch(clone.xsh, &num1);
-                if (l1 != NULL) {
-                    char *state = libxl_xs_read(&clone, XBT_NULL, l1[0]);
-                    if (!state || atoi(state) == 6) {
-                        xs_unwatch(clone.xsh, l1[0], l1[1]);
-                        xs_rm(clone.xsh, XBT_NULL, l1[1]);
-                        XL_LOG(&clone, XL_LOG_DEBUG, "Destroyed device backend at %s", l1[1]);
-                        n_watches--;
-                    }
-                    free(l1);
-                }
-            } else
+        while (n_watches > 0) {
+            if (wait_for_dev_destroy(&clone, &tv)) {
                 break;
+            } else {
+                n_watches--;
+            }
         }
     }
     for (i = 0; i < n; i++) {
@@ -280,6 +299,52 @@ int libxl_devices_destroy(struct libxl_c
     return 0;
 }
 
+int libxl_device_del(struct libxl_ctx *ctx, libxl_device *dev, int wait)
+{
+    char *dom_path_backend, *backend_path, *hotplug_path;
+    int rc;
+    struct libxl_ctx clone;
+
+    if (libxl_clone_context(ctx, &clone)) {
+        XL_LOG(ctx, XL_LOG_ERROR, "Out of memory when cloning context");
+        return ERROR_NOMEM;
+    }
+    clone.xsh = xs_daemon_open();
+
+    /* Create strings */
+    dom_path_backend    = libxl_xs_get_dompath(&clone, dev->backend_domid);
+    backend_path        = libxl_sprintf(&clone, "%s/backend/%s/%u/%d",
+                                    dom_path_backend, 
+                                    string_of_kinds[dev->backend_kind], 
+                                    dev->domid, dev->devid);
+    hotplug_path        = libxl_sprintf(&clone, "/xapi/%d/hotplug/%s/%d",
+                                    dev->domid,
+                                    string_of_kinds[dev->kind], 
+                                    dev->devid);
+    libxl_free(&clone, dom_path_backend);
+
+    rc = libxl_device_destroy(&clone, backend_path, !wait);
+    if (rc == -1) {
+        libxl_discard_cloned_context(&clone);
+        xs_daemon_close(clone.xsh);
+        return ERROR_FAIL;
+    }
+
+    if (wait) {
+        struct timeval tv;
+        tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
+        tv.tv_usec = 0;
+        (void)wait_for_dev_destroy(&clone, &tv);
+    }
+
+    xs_rm(clone.xsh, XBT_NULL, hotplug_path);
+    libxl_free(&clone, hotplug_path);
+    libxl_free(&clone, backend_path);
+    libxl_discard_cloned_context(&clone);
+    xs_daemon_close(clone.xsh);
+    return 0;
+}
+
 int libxl_device_pci_flr(struct libxl_ctx *ctx, unsigned int domain, unsigned int bus,
                          unsigned int dev, unsigned int func)
 {
diff -r e8611d996560 -r 889744de9da0 libxl_internal.h
--- a/libxl_internal.h
+++ b/libxl_internal.h
@@ -109,6 +109,7 @@ int device_disk_dev_number(char *virtpat
 
 int libxl_device_generic_add(struct libxl_ctx *ctx, libxl_device *device,
                              char **bents, char **fents);
+int libxl_device_del(struct libxl_ctx *ctx, libxl_device *dev, int wait);
 int libxl_device_destroy(struct libxl_ctx *ctx, char *be_path, int force);
 int libxl_devices_destroy(struct libxl_ctx *ctx, uint32_t domid, int force);
 int libxl_wait_for_device_model(struct libxl_ctx *ctx,

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH 1/7] libxenlight resend: refactor libxl destroy code

[Stefano Stabellini]

On Wed, 2 Dec 2009, Andres Lagar-Cavilla wrote:
> Re-resend against 20568:aa0b5558564f, plus the patch I just submitted 
> for cloning contexts. The rest of the patches in the series need no 
> modifications afaik.

The patch looks good apart from the fact that you should move 

clone.xsh = xs_daemon_open();

into libxl_clone_context, see my comment to your previous patch.

[PATCH 1/7] libxenlight meta-resend: refactor libxl destroy code

[Andres Lagar-Cavilla]

[-- Attachment #1: Type: text/plain, Size: 315 bytes --]

Refactor libxl device destroy code. Abstract function
waiting for the watch on the state node to fire.
Create a generic device delete function.

Only a single LIBXL_DESTROY_TIMEOUT elapses when
waiting for destruction of all the devices of a
domain.

Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.com>



[-- Attachment #2: 01_refactor_destroy.patch --]
[-- Type: text/x-patch, Size: 5776 bytes --]

# HG changeset patch
# User Andres Lagar-Cavilla <andres@lagarcavilla.com>
# Date 1259848362 18000
# Node ID ff941d6ded127d8d0bb6e754654140d9e1dfb0b1
# Parent  eeb2f35cd9a7a5a6c4e38c1ec5995524035375fa
Refactor libxl device destroy code. Abstract function
waiting for the watch on the state node to fire.
Create a generic device delete function.

Only a single LIBXL_DESTROY_TIMEOUT elapses when
waiting for destruction of all the devices of a
domain.

Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.com>

diff -r eeb2f35cd9a7 -r ff941d6ded12 libxl_device.c
--- a/libxl_device.c
+++ b/libxl_device.c
@@ -203,14 +203,40 @@ retry_transaction:
         return 0;
 }
 
+int wait_for_dev_destroy(struct libxl_ctx *ctx, struct timeval *tv)
+{
+    int nfds, rc;
+    unsigned int n;
+    fd_set rfds;
+    char **l1 = NULL;
+
+    rc = 1;
+    nfds = xs_fileno(ctx->xsh) + 1;
+    FD_ZERO(&rfds);
+    FD_SET(xs_fileno(ctx->xsh), &rfds);
+    if (select(nfds, &rfds, NULL, NULL, tv) > 0) {
+        l1 = xs_read_watch(ctx->xsh, &n);
+        if (l1 != NULL) {
+            char *state = libxl_xs_read(ctx, XBT_NULL, l1[XS_WATCH_PATH]);
+            if (!state || atoi(state) == 6) {
+                xs_unwatch(ctx->xsh, l1[0], l1[1]);
+                xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
+                XL_LOG(ctx, XL_LOG_DEBUG, "Destroyed device backend at %s", l1[XS_WATCH_TOKEN]);
+                rc = 0;
+            }
+            libxl_free(ctx, state);
+            free(l1);
+        }
+    }
+    return rc;
+}
+
 int libxl_devices_destroy(struct libxl_ctx *ctx, uint32_t domid, int force)
 {
     char *path, *be_path, *fe_path;
     unsigned int num1, num2;
     char **l1 = NULL, **l2 = NULL;
-    int i, j, nfds, n = 0, n_watches = 0;
-    fd_set rfds;
-    struct timeval tv;
+    int i, j, n = 0, n_watches = 0;
     flexarray_t *toremove;
     struct libxl_ctx clone;
 
@@ -245,27 +271,20 @@ int libxl_devices_destroy(struct libxl_c
         }
     }
     if (!force) {
-        nfds = xs_fileno(clone.xsh) + 1;
-        /* Linux-ism */
+        /* Linux-ism. Most implementations leave the timeout
+         * untouched after select. Linux, however, will chip
+         * away the elapsed time from it, which is what we 
+         * need to enforce a single time span waiting for
+         * device destruction. */
+        struct timeval tv;
         tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
         tv.tv_usec = 0;
-        while (n_watches > 0 && tv.tv_sec > 0) {
-            FD_ZERO(&rfds);
-            FD_SET(xs_fileno(clone.xsh), &rfds);
-            if (select(nfds, &rfds, NULL, NULL, &tv) > 0) {
-                l1 = xs_read_watch(clone.xsh, &num1);
-                if (l1 != NULL) {
-                    char *state = libxl_xs_read(&clone, XBT_NULL, l1[0]);
-                    if (!state || atoi(state) == 6) {
-                        xs_unwatch(clone.xsh, l1[0], l1[1]);
-                        xs_rm(clone.xsh, XBT_NULL, l1[1]);
-                        XL_LOG(&clone, XL_LOG_DEBUG, "Destroyed device backend at %s", l1[1]);
-                        n_watches--;
-                    }
-                    free(l1);
-                }
-            } else
+        while (n_watches > 0) {
+            if (wait_for_dev_destroy(&clone, &tv)) {
                 break;
+            } else {
+                n_watches--;
+            }
         }
     }
     for (i = 0; i < n; i++) {
@@ -277,6 +296,49 @@ int libxl_devices_destroy(struct libxl_c
     return 0;
 }
 
+int libxl_device_del(struct libxl_ctx *ctx, libxl_device *dev, int wait)
+{
+    char *dom_path_backend, *backend_path, *hotplug_path;
+    int rc;
+    struct libxl_ctx clone;
+
+    if (libxl_clone_context_xs(ctx, &clone)) {
+        XL_LOG(ctx, XL_LOG_ERROR, "Out of memory when cloning context");
+        return ERROR_NOMEM;
+    }
+
+    /* Create strings */
+    dom_path_backend    = libxl_xs_get_dompath(&clone, dev->backend_domid);
+    backend_path        = libxl_sprintf(&clone, "%s/backend/%s/%u/%d",
+                                    dom_path_backend, 
+                                    string_of_kinds[dev->backend_kind], 
+                                    dev->domid, dev->devid);
+    hotplug_path        = libxl_sprintf(&clone, "/xapi/%d/hotplug/%s/%d",
+                                    dev->domid,
+                                    string_of_kinds[dev->kind], 
+                                    dev->devid);
+    libxl_free(&clone, dom_path_backend);
+
+    rc = libxl_device_destroy(&clone, backend_path, !wait);
+    if (rc == -1) {
+        libxl_discard_cloned_context_xs(&clone);
+        return ERROR_FAIL;
+    }
+
+    if (wait) {
+        struct timeval tv;
+        tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
+        tv.tv_usec = 0;
+        (void)wait_for_dev_destroy(&clone, &tv);
+    }
+
+    xs_rm(clone.xsh, XBT_NULL, hotplug_path);
+    libxl_free(&clone, hotplug_path);
+    libxl_free(&clone, backend_path);
+    libxl_discard_cloned_context_xs(&clone);
+    return 0;
+}
+
 int libxl_device_pci_flr(struct libxl_ctx *ctx, unsigned int domain, unsigned int bus,
                          unsigned int dev, unsigned int func)
 {
diff -r eeb2f35cd9a7 -r ff941d6ded12 libxl_internal.h
--- a/libxl_internal.h
+++ b/libxl_internal.h
@@ -124,6 +124,7 @@ int device_disk_dev_number(char *virtpat
 
 int libxl_device_generic_add(struct libxl_ctx *ctx, libxl_device *device,
                              char **bents, char **fents);
+int libxl_device_del(struct libxl_ctx *ctx, libxl_device *dev, int wait);
 int libxl_device_destroy(struct libxl_ctx *ctx, char *be_path, int force);
 int libxl_devices_destroy(struct libxl_ctx *ctx, uint32_t domid, int force);
 int libxl_wait_for_device_model(struct libxl_ctx *ctx,

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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