|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 4/9] libxl_internal: Create new lock for devices hotplug via QMP
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@xxxxxxxxxx>
---
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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |