[PATCH 4/9] libxl_internal: Create new lock for devices hotplug via QMP

[Anthony PERARD <anthony.perard@citrix.com>]

The current lock `domain_userdata_lock' can't be used when modification
to a guest is done by sending command to QEMU, this is a slow process
and requires to call CTX_UNLOCK, which is not possible while holding
the `domain_userdata_lock'.

To resolve this issue, we create a new lock which can take over part of
the job of the json_lock.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

---

Quote from Ian:
> The invariant that we want to maintain is:
>
>   * Nothing may exist in the primary config without
>     a corresponding entry in libxl-json.
[...]
> How about the following scheme.  We split the libxl-json lock into
> two.  I'm going to call them the fast lock and the slow lock.
>
>   * The fast lock is the existing libxl-json lock.
>
>   * The slow lock is outside the libxl-json lock in the lock
>     hierarchy.  It is also outside the libxl_ctx lock.  It is
>     to be acquired by an ao event callback.
>
>   * No-one may read or edit the libxl-json without holding the fast
>     lock across their read operation, or their read/modify/write
>     cycle.
>
>   * However, there are special rules for thing removal/addition, for
>     things added/removed via qmp.  Call these `qmp things'.  It is
>     permissible to add or remove a qmp thing across two separate
>     acquisitions of the fast lock, one to read the old state of the
>     thing, and one to read/modify/write to update (only) the new state
>     of the thing.  This is subject to the thing add/removal rule, from
>     before, which becomes:
>
>   * You may not cause a thing to be added to the primary config
>     unless you have held the relevant thing lock continuously
>     since ensuring that the libxl-json config describes it.
>
>   * Conversely you may not cause a thing to be removed from the
>     libxl-json unless you have held the relevant thing lock
>     continuously since ensuring the thing is absent from the primary
>     config.
>
>   * The `relevant thing lock' is the slow lock for qmp things, and the
>     fast lock for other things.
>
>   * Acquiring the fast lock fails for a destroyed domain, as at
>     present.
---
 tools/libxl/libxl_internal.c | 27 +++++++++++++++++++++++++
 tools/libxl/libxl_internal.h | 39 ++++++++++++++++++++++++++++++++----
 2 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index fa0bbc3960..db281ac259 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -607,6 +607,33 @@ void libxl__update_domain_configuration(libxl__gc *gc,
     dst->b_info.video_memkb = src->b_info.video_memkb;
 }
 
+struct libxl__domain_qmp_lock {
+    libxl__generic_lock lock;
+};
+
+libxl__domain_qmp_lock *libxl__lock_domain_qmp(libxl__gc *gc,
+                                               libxl_domid domid)
+{
+    libxl__domain_qmp_lock *lock = NULL;
+    int rc;
+
+    lock = libxl__zalloc(NOGC, sizeof(*lock));
+    rc = libxl__lock_generic(gc, domid, &lock->lock,
+                             "libxl-device-changes-lock");
+    if (rc) {
+        libxl__unlock_domain_qmp(lock);
+        return NULL;
+    }
+
+    return lock;
+}
+
+void libxl__unlock_domain_qmp(libxl__domain_qmp_lock *lock)
+{
+    libxl__unlock_generic(&lock->lock);
+    free(lock);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index f1aefaf98a..43b44f2c30 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2561,6 +2561,21 @@ typedef struct libxl__ao_device libxl__ao_device;
 typedef struct libxl__multidev libxl__multidev;
 typedef void libxl__device_callback(libxl__egc*, libxl__ao_device*);
 
+/*
+ * Lock for device hotplug, qmp_lock.
+ *
+ * This lock is outside the json_lock lock in lock hierarchy.
+ * It is also outside the libxl_ctx lock.
+ * It is to be acquired by an ao event callback.
+ *
+ * It is to be acquired when adding/removing devices or making changes
+ * to them via QMP, when this is a slow operation.
+ */
+typedef struct libxl__domain_qmp_lock libxl__domain_qmp_lock;
+_hidden libxl__domain_qmp_lock *libxl__lock_domain_qmp(libxl__gc *gc,
+                                                           libxl_domid domid);
+_hidden void libxl__unlock_domain_qmp(libxl__domain_qmp_lock *lock);
+
 /* This functions sets the necessary libxl__ao_device struct values to use
  * safely inside functions. It marks the operation as "active"
  * since we need to be sure that all device status structs are set
@@ -2749,11 +2764,11 @@ struct libxl__multidev {
  * device information, in JSON files, so that we can use this JSON
  * file as a template to reconstruct domain configuration.
  *
- * In essense there are now two views of device state, one is xenstore,
- * the other is JSON file. We use xenstore as primary reference.
+ * In essense there are now two views of device state, one is the
+ * primary config (xenstore or QEMU), the other is JSON file.
  *
- * Here we maintain one invariant: every device in xenstore must have
- * an entry in JSON file.
+ * Here we maintain one invariant: every device in the primary config
+ * must have an entry in JSON file.
  *
  * All device hotplug routines should comply to following pattern:
  *   lock json config (json_lock)
@@ -2768,6 +2783,22 @@ struct libxl__multidev {
  *       end for loop
  *   unlock json config
  *
+ * Or in case QEMU is the primary config, this pattern can be use:
+ *   lock qmp (qmp_lock)
+ *      lock json config (json_lock)
+ *          read json config
+ *          update in-memory json config with new entry, replacing
+ *             any stale entry
+ *      unlock json config
+ *      apply new config to primary config
+ *      lock json config (json_lock)
+ *          read json config
+ *          update in-memory json config with new entry, replacing
+ *             any stale entry
+ *          write in-memory json config to disk
+ *      unlock json config
+ *   unlock qmp
+ *
  * Device removal routines are not touched.
  *
  * Here is the proof that we always maintain that invariant and we
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel