Xen devel - Dec 2011 - [PATCH 0 of 3 v2] libxl: fix domain destruction with libxl hotplug script calling

When calling hotplug scripts from libxl we have to make sure the
device is disconnected before attepmting to execute hotplug scripts.

This patch series sets frontend status to 6 when a domain is
destroyed, so the devices are disconnected and then execute hotplug
scripts.

Also the syntax of the libxl_domain_destroy has been changed to always
force the destruction.

This series should be applied after "libxl: add support for hotplug
script calling from libxl".

Changes from v1:

 * Remove foce parameter from libxl__devices_destroy and clean up 
   unnecessary code in function.

Please review, Roger.

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1323079583 -3600
# Node ID bc90cfd8dd220d69d09cf94a3d39ff3cef76d021
# Parent  274fa4aea2a30fb82228513f969d7cb807813bb8
libxl: set frontend status to 6 on domain destroy

Set frontend status to 6 on domain destruction and wait for devices to
be disconnected before executing hotplug scripts.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r 274fa4aea2a3 -r bc90cfd8dd22 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Dec 01 16:01:31 2011 +0100
+++ b/tools/libxl/libxl_device.c	Mon Dec 05 11:06:23 2011 +0100
@@ -513,10 +513,7 @@ int libxl__device_destroy(libxl__gc *gc,
     char *be_path = libxl__device_backend_path(gc, dev);
     char *fe_path = libxl__device_frontend_path(gc, dev);
 
-    /* 
-     * Run hotplug scripts, which will probably not be able to
-     * execute successfully since the device may still be plugged
-     */
+    libxl__xs_write(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s",
fe_path, "state"), "6");
     libxl__device_execute_hotplug(gc, dev, DISCONNECT);
 
     xs_rm(ctx->xsh, XBT_NULL, be_path);

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1323079605 -3600
# Node ID c0d51df66b829995c4eb3902b5b9914c710a6c01
# Parent  bc90cfd8dd220d69d09cf94a3d39ff3cef76d021
libxl: remove force parameter from libxl_domain_destroy

Since a destroy is considered a forced shutdown, there''s no point in
passing a force parameter. All the occurences of this function have
been replaced with the proper syntax.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Dec 05 11:06:23 2011 +0100
+++ b/tools/libxl/libxl.c	Mon Dec 05 11:06:45 2011 +0100
@@ -718,7 +718,7 @@ int libxl_event_get_disk_eject_info(libx
     return 1;
 }
 
-int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid, int force)
+int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     libxl_dominfo dominfo;
@@ -767,7 +767,7 @@ int libxl_domain_destroy(libxl_ctx *ctx,
 
         libxl__qmp_cleanup(&gc, domid);
     }
-    if (libxl__devices_destroy(&gc, domid, force) < 0)
+    if (libxl__devices_destroy(&gc, domid, 1) < 0)
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "libxl__devices_destroy failed
for %d", domid);
 
     vm_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc,
"%s/vm", dom_path));
diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Dec 05 11:06:23 2011 +0100
+++ b/tools/libxl/libxl.h	Mon Dec 05 11:06:45 2011 +0100
@@ -269,7 +269,7 @@ int libxl_domain_suspend(libxl_ctx *ctx,
                           uint32_t domid, int fd);
 int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid);
 int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid, int req);
-int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid, int force);
+int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid);
 int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid,
libxl_domain_create_info *info, const char *name_suffix, libxl_uuid new_uuid);
 
 /* get max. number of cpus supported by hypervisor */
diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Dec 05 11:06:23 2011 +0100
+++ b/tools/libxl/libxl_create.c	Mon Dec 05 11:06:45 2011 +0100
@@ -646,7 +646,7 @@ static int do_domain_create(libxl__gc *g
 
 error_out:
     if (domid)
-        libxl_domain_destroy(ctx, domid, 0);
+        libxl_domain_destroy(ctx, domid);
 
     return ret;
 }
diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Mon Dec 05 11:06:23 2011 +0100
+++ b/tools/libxl/libxl_dm.c	Mon Dec 05 11:06:45 2011 +0100
@@ -917,7 +917,7 @@ int libxl__destroy_device_model(libxl__g
             goto out;
         }
         LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Device model is a stubdom,
domid=%d", stubdomid);
-        ret = libxl_domain_destroy(ctx, stubdomid, 0);
+        ret = libxl_domain_destroy(ctx, stubdomid);
         if (ret)
             goto out;
     } else {
diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Mon Dec 05 11:06:23 2011 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Mon Dec 05 11:06:45 2011 +0100
@@ -1200,7 +1200,7 @@ static int handle_domain_death(libxl_ctx
         /* fall-through */
     case LIBXL_ACTION_ON_SHUTDOWN_DESTROY:
         LOG("Domain %d needs to be cleaned up: destroying the
domain", domid);
-        libxl_domain_destroy(ctx, domid, 0);
+        libxl_domain_destroy(ctx, domid);
         break;
 
     case LIBXL_ACTION_ON_SHUTDOWN_COREDUMP_DESTROY:
@@ -1698,7 +1698,7 @@ start:
 error_out:
     release_lock();
     if (libxl_domid_valid_guest(domid))
-        libxl_domain_destroy(ctx, domid, 0);
+        libxl_domain_destroy(ctx, domid);
 
 out:
     if (logfile != 2)
@@ -2168,7 +2168,7 @@ static void destroy_domain(const char *p
         fprintf(stderr, "Cannot destroy privileged domain 0.\n\n");
         exit(-1);
     }
-    rc = libxl_domain_destroy(ctx, domid, 0);
+    rc = libxl_domain_destroy(ctx, domid);
     if (rc) { fprintf(stderr,"destroy failed (rc=%d)\n",rc);
exit(-1); }
 }
 
@@ -2414,7 +2414,7 @@ static int save_domain(const char *p, co
     if (checkpoint)
         libxl_domain_unpause(ctx, domid);
     else
-        libxl_domain_destroy(ctx, domid, 0);
+        libxl_domain_destroy(ctx, domid);
 
     exit(0);
 }
@@ -2647,7 +2647,7 @@ static void migrate_domain(const char *d
     }
 
     fprintf(stderr, "migration sender: Target reports successful
startup.\n");
-    libxl_domain_destroy(ctx, domid, 1); /* bang! */
+    libxl_domain_destroy(ctx, domid); /* bang! */
     fprintf(stderr, "Migration successful.\n");
     exit(0);
 
@@ -2764,7 +2764,7 @@ static void migrate_receive(int debug, i
     if (rc) {
         fprintf(stderr, "migration target: Failure, destroying our
copy.\n");
 
-        rc2 = libxl_domain_destroy(ctx, domid, 1);
+        rc2 = libxl_domain_destroy(ctx, domid);
         if (rc2) {
             fprintf(stderr, "migration target: Failed to destroy our
copy"
                     " (code %d).\n", rc2);
diff -r bc90cfd8dd22 -r c0d51df66b82 tools/python/xen/lowlevel/xl/xl.c
--- a/tools/python/xen/lowlevel/xl/xl.c	Mon Dec 05 11:06:23 2011 +0100
+++ b/tools/python/xen/lowlevel/xl/xl.c	Mon Dec 05 11:06:45 2011 +0100
@@ -437,10 +437,10 @@ static PyObject *pyxl_domain_shutdown(Xl
 
 static PyObject *pyxl_domain_destroy(XlObject *self, PyObject *args)
 {
-    int domid, force = 1;
-    if ( !PyArg_ParseTuple(args, "i|i", &domid, &force) )
+    int domid;
+    if ( !PyArg_ParseTuple(args, "i", &domid) )
         return NULL;
-    if ( libxl_domain_destroy(self->ctx, domid, force) ) {
+    if ( libxl_domain_destroy(self->ctx, domid) ) {
         PyErr_SetString(xl_error_obj, "cannot destroy domain");
         return NULL;
     }

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1323081693 -3600
# Node ID 2c0f1676a3667b9070832999818447e51143c110
# Parent  c0d51df66b829995c4eb3902b5b9914c710a6c01
libxl: remove force parameter from libxl__devices_destroy

Remove the force flag, and always use forced destruction.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r c0d51df66b82 -r 2c0f1676a366 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Dec 05 11:06:45 2011 +0100
+++ b/tools/libxl/libxl.c	Mon Dec 05 11:41:33 2011 +0100
@@ -767,7 +767,7 @@ int libxl_domain_destroy(libxl_ctx *ctx,
 
         libxl__qmp_cleanup(&gc, domid);
     }
-    if (libxl__devices_destroy(&gc, domid, 1) < 0)
+    if (libxl__devices_destroy(&gc, domid) < 0)
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "libxl__devices_destroy failed
for %d", domid);
 
     vm_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc,
"%s/vm", dom_path));
diff -r c0d51df66b82 -r 2c0f1676a366 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Mon Dec 05 11:06:45 2011 +0100
+++ b/tools/libxl/libxl_device.c	Mon Dec 05 11:41:33 2011 +0100
@@ -524,13 +524,13 @@ int libxl__device_destroy(libxl__gc *gc,
     return 0;
 }
 
-int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force)
+int libxl__devices_destroy(libxl__gc *gc, uint32_t domid)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char *path;
     unsigned int num_kinds, num_devs;
     char **kinds = NULL, **devs = NULL;
-    int i, j, n_watches = 0;
+    int i, j;
     libxl__device dev;
     libxl__device_kind kind;
 
@@ -561,16 +561,7 @@ int libxl__devices_destroy(libxl__gc *gc
                 dev.kind = kind;
                 dev.devid = atoi(devs[j]);
 
-                if (force) {
-                    libxl__device_destroy(gc, &dev);
-                } else {
-                    int rc = libxl__device_remove(gc, &dev, 0);
-                    if (rc < 0)
-                        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                                   "cannot remove device %s\n",
path);
-                    else
-                        n_watches += rc;
-                }
+                libxl__device_destroy(gc, &dev);
             }
         }
     }
@@ -584,37 +575,9 @@ int libxl__devices_destroy(libxl__gc *gc
         dev.kind = LIBXL__DEVICE_KIND_CONSOLE;
         dev.devid = 0;
 
-        if (force) {
-            libxl__device_destroy(gc, &dev);
-        } else {
-            int rc = libxl__device_remove(gc, &dev, 0);
-            if (rc < 0)
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                           "cannot remove device %s\n", path);
-            else
-                n_watches += rc;
-        }
+        libxl__device_destroy(gc, &dev);
     }
 
-    if (!force) {
-        /* 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) {
-            if (libxl__wait_for_device_state(gc, &tv, XenbusStateClosed,
-                                             destroy_device) < 0) {
-                /* function returned ERROR_* */
-                break;
-            } else {
-                n_watches--;
-            }
-        }
-    }
 out:
     return 0;
 }
diff -r c0d51df66b82 -r 2c0f1676a366 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Mon Dec 05 11:06:45 2011 +0100
+++ b/tools/libxl/libxl_internal.h	Mon Dec 05 11:41:33 2011 +0100
@@ -252,7 +252,7 @@ _hidden int libxl__parse_backend_path(li
                                       libxl__device *dev);
 _hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev, int wait);
 _hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
-_hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force);
+_hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid);
 _hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
 
 /* Handler for the libxl__wait_for_device_state callback */