[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 |