[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v8 10/15] libxl: add option to choose who executes hotplug scripts



Ian Campbell wrote:
On Wed, 2012-07-04 at 12:59 +0100, Roger Pau Monne wrote:
Add and option to xl.conf file to decide if hotplug scripts are
executed from the toolstack (xl) or from udev as it used to be in the
past.

This option is only introduced in this patch, but it has no effect
since the code to call hotplug scripts from libxl is introduced in a
latter patch.

This choice will be saved in "libxl/disable_udev", as specified in the
DISABLE_UDEV_PATH constant.

Changes since v2:

  * Change atoi(...) to !!atoi(...) to prevent returning negative
    values from xenstore (which will be handled as errors).

  * Check for errors on the return value of libxl__hotplug_settings.

Changes since v1:

  * Used an auxiliary function to check for the current setting.

Cc: Ian Jackson<ian.jackson@xxxxxxxxxxxxx>
Acked-by: Ian Jackson<ian.jackson@xxxxxxxxxxxxx>
Signed-off-by: Roger Pau Monne<roger.pau@xxxxxxxxxx>
---
  docs/man/xl.conf.pod.5       |    8 ++++++++
  tools/examples/xl.conf       |    5 +++++
  tools/libxl/libxl_create.c   |   38 +++++++++++++++++++++++++++++++++++++-
  tools/libxl/libxl_dm.c       |    4 ++++
  tools/libxl/libxl_internal.c |   19 +++++++++++++++++++
  tools/libxl/libxl_internal.h |    3 +++
  tools/libxl/libxl_types.idl  |    1 +
  tools/libxl/xl.c             |    4 ++++
  tools/libxl/xl.h             |    1 +
  tools/libxl/xl_cmdimpl.c     |    1 +
  10 files changed, 83 insertions(+), 1 deletions(-)

diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index 149430c..23932be 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -55,6 +55,14 @@ default.

  Default: C<1>

+=item B<run_hotplug_scripts=BOOLEAN>
+
+If disabled hotplug scripts will be called from udev, as it used to
+be in the previous releases. With the default option, hotplug scripts
+will be launched by xl directly.
+
+Default: C<1>
+
  =item B<lockfile="PATH">

  Sets the path to the lock file used by xl to serialise certain
diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf
index ebf057c..28ab796 100644
--- a/tools/examples/xl.conf
+++ b/tools/examples/xl.conf
@@ -15,3 +15,8 @@

  # first block device to be used for temporary VM disk mounts
  #blkdev_start="xvda"
+
+# default option to run hotplug scripts from xl
+# if disabled the old behaviour will be used, and hotplug scripts will be
+# launched by udev.
+#run_hotplug_scripts=1
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 6f4c865..c584c4e 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -70,6 +70,8 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
          libxl_defbool_setdefault(&c_info->oos, true);
      }

+    libxl_defbool_setdefault(&c_info->run_hotplug_scripts, true);
+
      return 0;
  }

@@ -382,7 +384,7 @@ int libxl__domain_make(libxl__gc *gc, 
libxl_domain_create_info *info,
                         uint32_t *domid)
  {
      libxl_ctx *ctx = libxl__gc_owner(gc);
-    int flags, ret, rc;
+    int flags, ret, rc, nb_vm;
      char *uuid_string;
      char *dom_path, *vm_path, *libxl_path;
      struct xs_permissions roperm[2];
@@ -503,6 +505,40 @@ retry_transaction:
              libxl__sprintf(gc, "%s/hvmloader/generation-id-address", 
dom_path),
                          rwperm, ARRAY_SIZE(rwperm));

+    if (libxl_list_vm(ctx,&nb_vm)<  0) {

This will leak the returned list of vms...

Fixed.


+        LOG(ERROR, "cannot get number of running guests");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    int hotplug_setting = libxl__hotplug_settings(gc, t);
+    if (hotplug_setting<  0) {
+        LOG(ERROR, "unable to get current hotplug scripts execution setting");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    if (libxl_defbool_val(info->run_hotplug_scripts) != hotplug_setting&&
+        (nb_vm - 1)) {
+        LOG(ERROR, "cannot change hotplug execution option once set, "
+                    "please shutdown all guests before changing it");
+        rc = ERROR_FAIL;
+        goto out;
+    }

+
+    if (libxl_defbool_val(info->run_hotplug_scripts)) {
+        rc = libxl__xs_write(gc, t, DISABLE_UDEV_PATH, "1");
+        if (rc) {
+            LOGE(ERROR, "unable to write %s = 1", DISABLE_UDEV_PATH);
+            goto out;
+        }
+    } else {
+        rc = xs_rm(ctx->xsh, t, DISABLE_UDEV_PATH);
+        if (rc) {
+            LOGE(ERROR, "unable to delete %s", DISABLE_UDEV_PATH);
+            goto out;
+        }
+    }

You could use libxl__xs_{write,rm}_checked here I think.

Yes, when this was written libxl__xs_{write,rm}_checked wasn't there. Now I've changed it, although the rest of the code in this function still use xs_rm or libxl_xs_write.


+
      xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/uuid", vm_path), 
uuid_string, strlen(uuid_string));
      xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/name", vm_path), info->name, 
strlen(info->name));

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index b4a5f1f..612e9db 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -762,6 +762,10 @@ void libxl__spawn_stub_dm(libxl__egc *egc, 
libxl__stub_dm_spawn_state *sdss)
      dm_config->nics = guest_config->nics;
      dm_config->num_nics = guest_config->num_nics;

+    libxl_defbool_set(
+&dm_config->c_info.run_hotplug_scripts,
+        libxl_defbool_val(guest_config->c_info.run_hotplug_scripts));

This is the same as:

dm_config->c_info.run_hotplug_scripts = 
guest_config->c_info.run_hotplug_scripts;

Changed, thanks for the comments.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.