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

[Xen-devel] [PATCH] libxl: Deprecate synchronous waiting for the device model



commit a2e653f795e14cc7d2e83c8075311757564ed751
Author: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Date:   Mon Oct 14 17:26:01 2013 +0100

    libxl: Deprecate synchronous waiting for the device model
    
    libxl__wait_for_device_model blocks, with the ctx lock held, waiting
    for a response from the device model.  If the dm doesn't respond
    quickly (for example, because it has crashed), this may block the
    whole process.  Explain this in a comment, rename the function to
    libxl__wait_for_device_model_deprecated, and explain what to use
    instead.
    
    libxl__wait_for_offspring is the core implementation for the above.
    Its name leads people to think it might be generally useful for
    waiting for children, which is far from the case.  It only waits for
    xenstore.  Also it has the problems described above.  Explain this,
    rename it to libxl__xenstore_child_wait_deprecated, and explain what
    to use instead.
    
    Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 29e66f2..eba7c0d 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -846,7 +846,7 @@ int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid)
         state = libxl__xs_read(gc, XBT_NULL, path);
         if (state != NULL && !strcmp(state, "paused")) {
             libxl__qemu_traditional_cmd(gc, domid, "continue");
-            libxl__wait_for_device_model(gc, domid, "running",
+            libxl__wait_for_device_model_deprecated(gc, domid, "running",
                                          NULL, NULL, NULL);
         }
     }
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 16a92a4..610f0c5 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1077,7 +1077,7 @@ static void devices_remove_callback(libxl__egc *egc,
     return;
 }
 
-int libxl__wait_for_device_model(libxl__gc *gc,
+int libxl__wait_for_device_model_deprecated(libxl__gc *gc,
                                  uint32_t domid, char *state,
                                  libxl__spawn_starting *spawning,
                                  int (*check_callback)(libxl__gc *gc,
@@ -1088,7 +1088,7 @@ int libxl__wait_for_device_model(libxl__gc *gc,
 {
     char *path;
     path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
-    return libxl__wait_for_offspring(gc, domid,
+    return libxl__xenstore_child_wait_deprecated(gc, domid,
                                      LIBXL_DEVICE_MODEL_START_TIMEOUT,
                                      "Device Model", path, state, spawning,
                                      check_callback, check_callback_userdata);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 6e2252a..709f820 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -955,7 +955,7 @@ int libxl__domain_suspend_device_model(libxl__gc *gc,
     case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
         LOG(DEBUG, "Saving device model state to %s", filename);
         libxl__qemu_traditional_cmd(gc, domid, "save");
-        libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL, NULL);
+        libxl__wait_for_device_model_deprecated(gc, domid, "paused", NULL, 
NULL, NULL);
         break;
     }
     case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
@@ -979,7 +979,7 @@ int libxl__domain_resume_device_model(libxl__gc *gc, 
uint32_t domid)
     switch (libxl__device_model_version_running(gc, domid)) {
     case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
         libxl__qemu_traditional_cmd(gc, domid, "continue");
-        libxl__wait_for_device_model(gc, domid, "running", NULL, NULL, NULL);
+        libxl__wait_for_device_model_deprecated(gc, domid, "running", NULL, 
NULL, NULL);
         break;
     }
     case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index 7eddaef..b6efa0f 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -157,7 +157,7 @@ out:
     return rc ? SIGTERM : 0;
 }
 
-int libxl__wait_for_offspring(libxl__gc *gc,
+int libxl__xenstore_child_wait_deprecated(libxl__gc *gc,
                                  uint32_t domid,
                                  uint32_t timeout, char *what,
                                  char *path, char *state,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 165dc00..653079a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1223,7 +1223,19 @@ _hidden int libxl__spawn_record_pid(libxl__gc*, 
libxl__spawn_state*,
                                     pid_t innerchild);
 
 /*
- * libxl__wait_for_offspring - Wait for child state
+ * libxl__xenstore_child_wait_deprecated - Wait for daemonic child IPC
+ *
+ * This is a NOT function for waiting for ordinary child processes.
+ * If you want to run (fork/exec/wait) subprocesses from libxl:
+ *  - Make your libxl entrypoint use the ao machinery
+ *  - Use libxl__ev_fork, and use the callback programming style
+ *
+ * This function is intended for interprocess communication with a
+ * service process.  If the service process does not respond quickly,
+ * the whole caller may be blocked.  Therefore this function is
+ * deprecated.  This function is currently used only by
+ * libxl__wait_for_device_model_deprecated.
+ *
  * gc: allocation pool
  * domid: guest to work with
  * timeout: how many seconds to wait for the state to appear
@@ -1240,9 +1252,9 @@ _hidden int libxl__spawn_record_pid(libxl__gc*, 
libxl__spawn_state*,
  * in xenstore, and optionally for state in path.
  * If path appears and state matches, check_callback is called.
  * If check_callback returns > 0, waiting for path or state continues.
- * Otherwise libxl__wait_for_offspring returns.
+ * Otherwise libxl__xenstore_child_wait_deprecated returns.
  */
-_hidden int libxl__wait_for_offspring(libxl__gc *gc,
+_hidden int libxl__xenstore_child_wait_deprecated(libxl__gc *gc,
                                  uint32_t domid,
                                  uint32_t timeout, char *what,
                                  char *path, char *state,
@@ -1295,12 +1307,20 @@ _hidden int libxl__need_xenpv_qemu(libxl__gc *gc,
         int nr_consoles, libxl__device_console *consoles,
         int nr_vfbs, libxl_device_vfb *vfbs,
         int nr_disks, libxl_device_disk *disks);
-  /* Caller must either: pass starting_r==0, or on successful
-   * return pass *starting_r (which will be non-0) to
-   * libxl__confirm_device_model_startup or libxl__detach_device_model. */
-_hidden int libxl__wait_for_device_model(libxl__gc *gc,
+
+/*
+ * This function will cause the whole libxl process to hang
+ * if the device model does not respond.  It is deprecated.
+ *
+ * Instead of calling this function:
+ *  - Make your libxl entrypoint use the ao machinery
+ *  - Use libxl__ev_xswatch_register, and use the callback programming
+ *    style
+ */
+_hidden int libxl__wait_for_device_model_deprecated(libxl__gc *gc,
                                 uint32_t domid, char *state,
-                                libxl__spawn_starting *spawning,
+                                libxl__spawn_starting *spawning
+                                                    /* NULL allowed */,
                                 int (*check_callback)(libxl__gc *gc,
                                                       uint32_t domid,
                                                       const char *state,
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index b9960bb..df43895 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -823,7 +823,7 @@ static int qemu_pci_add_xenstore(libxl__gc *gc, uint32_t 
domid,
     }
 
     libxl__qemu_traditional_cmd(gc, domid, "pci-ins");
-    rc = libxl__wait_for_device_model(gc, domid, NULL, NULL,
+    rc = libxl__wait_for_device_model_deprecated(gc, domid, NULL, NULL,
                                       pci_ins_check, state);
     path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/parameter",
                           domid);
@@ -851,7 +851,7 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, 
libxl_device_pci *pcidev, i
     switch (libxl__domain_type(gc, domid)) {
     case LIBXL_DOMAIN_TYPE_HVM:
         hvm = 1;
-        if (libxl__wait_for_device_model(gc, domid, "running",
+        if (libxl__wait_for_device_model_deprecated(gc, domid, "running",
                                          NULL, NULL, NULL) < 0) {
             return ERROR_FAIL;
         }
@@ -1137,7 +1137,7 @@ static int qemu_pci_remove_xenstore(libxl__gc *gc, 
uint32_t domid,
      * device-model for function 0 */
     if ( !force && (pcidev->vdevfn & 0x7) == 0 ) {
         libxl__qemu_traditional_cmd(gc, domid, "pci-rem");
-        if (libxl__wait_for_device_model(gc, domid, "pci-removed",
+        if (libxl__wait_for_device_model_deprecated(gc, domid, "pci-removed",
                                          NULL, NULL, NULL) < 0) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Device Model didn't respond in 
time");
             /* This depends on guest operating system acknowledging the
@@ -1179,7 +1179,7 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
     switch (libxl__domain_type(gc, domid)) {
     case LIBXL_DOMAIN_TYPE_HVM:
         hvm = 1;
-        if (libxl__wait_for_device_model(gc, domid, "running",
+        if (libxl__wait_for_device_model_deprecated(gc, domid, "running",
                                          NULL, NULL, NULL) < 0)
             goto out_fail;
 

_______________________________________________
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®.