|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v3 09/15] libxl: synchronise configuration when we hotplug a device
We update JSON version first, then write to xenstore, so that we
maintain the following invariant: any device which is present in
xenstore has a corresponding entry in JSON.
The workflow is as followed:
lock json config
read json config
update in-memory json config with new entry, rewrite
if there's stale entry
for loop
open xs transaction
check device existence, abort if it exists
write in-memory json config to disk
commit xs transaction
end for loop
unlock json config
Please see comment in libxl_internal.h for correctness proof.
As those routines are called both during domain creation and device
hotplug, we add a flag to indicate whether we need to update JSON
config. This flag is only set to true when we hotplug a device. We
cannot update JSON config during domain creation as JSON config is
committed to disk only when domain creation finishes.
Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
---
tools/libxl/libxl.c | 99 +++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 131 ++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_pci.c | 24 ++++++++
3 files changed, 254 insertions(+)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ad3495a..fa7d5d5 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1894,6 +1894,13 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t
domid,
libxl__device *device;
int rc;
xs_transaction_t t = XBT_NULL;
+ libxl_domain_config d_config;
+ libxl_device_vtpm vtpm_saved;
+ libxl__carefd *lock = NULL;
+
+ libxl_domain_config_init(&d_config);
+ libxl_device_vtpm_init(&vtpm_saved);
+ libxl_device_vtpm_copy(CTX, &vtpm_saved, vtpm);
rc = libxl__device_vtpm_setdefault(gc, vtpm);
if (rc) goto out;
@@ -1908,6 +1915,8 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t
domid,
}
}
+ libxl__update_config_vtpm(gc, &vtpm_saved, vtpm);
+
GCNEW(device);
rc = libxl__device_from_vtpm(gc, domid, vtpm, device);
if ( rc != 0 ) goto out;
@@ -1933,6 +1942,19 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t
domid,
flexarray_append(front, "handle");
flexarray_append(front, GCSPRINTF("%d", vtpm->devid));
+ if (aodev->update_json) {
+ lock = libxl__lock_domain_userdata(gc, domid);
+ if (!lock) {
+ rc = ERROR_LOCK_FAIL;
+ goto out;
+ }
+
+ rc = libxl__get_domain_configuration(gc, domid, &d_config);
+ if (rc) goto out;
+
+ DEVICE_ADD(vtpm, vtpms, domid, &vtpm_saved, COMPARE_DEVID, &d_config);
+ }
+
for (;;) {
rc = libxl__xs_transaction_start(gc, &t);
if (rc) goto out;
@@ -1946,6 +1968,11 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t
domid,
goto out;
}
+ if (aodev->update_json) {
+ rc = libxl__set_domain_configuration(gc, domid, &d_config);
+ if (rc) goto out;
+ }
+
libxl__device_generic_add(gc, t, device,
libxl__xs_kvs_of_flexarray(gc, back,
back->count),
@@ -1965,6 +1992,9 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t
domid,
rc = 0;
out:
libxl__xs_transaction_abort(gc, &t);
+ if (lock) libxl__unlock_domain_userdata(lock);
+ libxl_device_vtpm_dispose(&vtpm_saved);
+ libxl_domain_config_dispose(&d_config);
aodev->rc = rc;
if(rc) aodev->callback(egc, aodev);
return;
@@ -2197,6 +2227,13 @@ static void device_disk_add(libxl__egc *egc, uint32_t
domid,
int rc;
libxl_ctx *ctx = gc->owner;
xs_transaction_t t = XBT_NULL;
+ libxl_domain_config d_config;
+ libxl_device_disk disk_saved;
+ libxl__carefd *lock = NULL;
+
+ libxl_domain_config_init(&d_config);
+ libxl_device_disk_init(&disk_saved);
+ libxl_device_disk_copy(ctx, &disk_saved, disk);
libxl_domain_type type = libxl__domain_type(gc, domid);
if (type == LIBXL_DOMAIN_TYPE_INVALID) {
@@ -2204,6 +2241,29 @@ static void device_disk_add(libxl__egc *egc, uint32_t
domid,
goto out;
}
+ /*
+ * get_vdev != NULL -> local attach
+ * get_vdev == NULL -> block attach
+ *
+ * We don't care about local attach state because it's only
+ * intermediate state. What's nice is that vdev won't change in
+ * "block-attach" case so the macro works as expected -- it can
+ * safely skip an existing entry. See similar check in the for
+ * loop below.
+ */
+ if (!get_vdev && aodev->update_json) {
+ lock = libxl__lock_domain_userdata(gc, domid);
+ if (!lock) {
+ rc = ERROR_LOCK_FAIL;
+ goto out;
+ }
+
+ rc = libxl__get_domain_configuration(gc, domid, &d_config);
+ if (rc) goto out;
+
+ DEVICE_ADD(disk, disks, domid, &disk_saved, COMPARE_DISK, &d_config);
+ }
+
for (;;) {
rc = libxl__xs_transaction_start(gc, &t);
if (rc) goto out;
@@ -2356,6 +2416,11 @@ static void device_disk_add(libxl__egc *egc, uint32_t
domid,
}
}
+ if (!get_vdev && aodev->update_json) {
+ rc = libxl__set_domain_configuration(gc, domid, &d_config);
+ if (rc) goto out;
+ }
+
libxl__device_generic_add(gc, t, device,
libxl__xs_kvs_of_flexarray(gc, back,
back->count),
libxl__xs_kvs_of_flexarray(gc, front,
front->count),
@@ -2374,6 +2439,9 @@ static void device_disk_add(libxl__egc *egc, uint32_t
domid,
out:
libxl__xs_transaction_abort(gc, &t);
+ if (lock) libxl__unlock_domain_userdata(lock);
+ libxl_device_disk_dispose(&disk_saved);
+ libxl_domain_config_dispose(&d_config);
aodev->rc = rc;
if (rc) aodev->callback(egc, aodev);
return;
@@ -3030,6 +3098,13 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t
domid,
libxl__device *device;
int rc;
xs_transaction_t t = XBT_NULL;
+ libxl_domain_config d_config;
+ libxl_device_nic nic_saved;
+ libxl__carefd *lock = NULL;
+
+ libxl_domain_config_init(&d_config);
+ libxl_device_nic_init(&nic_saved);
+ libxl_device_nic_copy(CTX, &nic_saved, nic);
rc = libxl__device_nic_setdefault(gc, nic, domid);
if (rc) goto out;
@@ -3044,6 +3119,8 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t
domid,
}
}
+ libxl__update_config_nic(gc, &nic_saved, nic);
+
GCNEW(device);
rc = libxl__device_from_nic(gc, domid, nic, device);
if ( rc != 0 ) goto out;
@@ -3101,6 +3178,19 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t
domid,
flexarray_append(front, libxl__sprintf(gc,
LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
+ if (aodev->update_json) {
+ lock = libxl__lock_domain_userdata(gc, domid);
+ if (!lock) {
+ rc = ERROR_LOCK_FAIL;
+ goto out;
+ }
+
+ rc = libxl__get_domain_configuration(gc, domid, &d_config);
+ if (rc) goto out;
+
+ DEVICE_ADD(nic, nics, domid, &nic_saved, COMPARE_DEVID, &d_config);
+ }
+
for (;;) {
rc = libxl__xs_transaction_start(gc, &t);
if (rc) goto out;
@@ -3114,6 +3204,11 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t
domid,
goto out;
}
+ if (aodev->update_json) {
+ rc = libxl__set_domain_configuration(gc, domid, &d_config);
+ if (rc) goto out;
+ }
+
libxl__device_generic_add(gc, t, device,
libxl__xs_kvs_of_flexarray(gc, back,
back->count),
@@ -3133,6 +3228,9 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t
domid,
rc = 0;
out:
libxl__xs_transaction_abort(gc, &t);
+ if (lock) libxl__unlock_domain_userdata(lock);
+ libxl_device_nic_dispose(&nic_saved);
+ libxl_domain_config_dispose(&d_config);
aodev->rc = rc;
if (rc) aodev->callback(egc, aodev);
return;
@@ -3686,6 +3784,7 @@ DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)
GCNEW(aodev); \
libxl__prepare_ao_device(ao, aodev); \
aodev->callback = device_addrm_aocomplete; \
+ aodev->update_json = true; \
libxl__device_##type##_add(egc, domid, type, aodev); \
\
return AO_INPROGRESS; \
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index fafef5a..2546f88 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2146,6 +2146,8 @@ struct libxl__ao_device {
int num_exec;
/* for calling hotplug scripts */
libxl__async_exec_state aes;
+ /* If we need to udpate JSON config */
+ bool update_json;
};
/*
@@ -2271,6 +2273,78 @@ struct libxl__multidev {
*
* Once finished, aodev->callback will be executed.
*/
+/*
+ * As of Xen 4.5 we maintain various infomation, including hotplug
+ * device information, in JSON files, so that we can use this JSON
+ * file as a template to reconstruct domain configuration.
+ *
+ * In essnse there are now two views of device state, one is xenstore,
+ * the other is JSON file. We use xenstore as primary reference.
+ *
+ * Here we maintain one invariant: every device in xenstore must have
+ * an entry in JSON file.
+ *
+ * All device hotplug routines should comply to following pattern:
+ * lock json config (json_lock)
+ * read json config
+ * update in-memory json config with new entry, rewrite
+ * if there's stale entry
+ * for loop -- xs transaction
+ * open xs transaction
+ * check device existence, abort if it exists
+ * write in-memory json config to disk
+ * commit xs transaction
+ * end for loop
+ * unlock json config
+ *
+ * Device removal routines are not touched.
+ *
+ * Here is the proof that we always maintain that invariant and we
+ * don't leak files during interaction of hotplug thread and other
+ * threads / processes.
+ *
+ * # Safe against parallel add
+ *
+ * When another thread / process tries to add same device, it's
+ * blocked by json_lock. The loser of two threads will bail at
+ * existence check, so that we don't overwrite anything.
+ *
+ * # Safe against domain destruction
+ *
+ * When another thread / process tries to destroy domain, it's blocked
+ * by json_lock. If domain destruction thread is loser, it deletes
+ * every userdata file after it requires the lock. If hotplug thread
+ * is loser, it bails at acquiring lock, no device is added. Either
+ * way, no file is leaked.
+ *
+ * # Safe against parallel removal
+ *
+ * When another thread / process tries to remove a device, it's _NOT_
+ * blocked by json_lock, but xenstore transaction can help maintain
+ * invariant. The removal threads either a) sees that device in
+ * xenstore, b) doesn't see that device in xenstore.
+ *
+ * In a), it sees that device in xenstore. At that point hotplug is
+ * already finished (both JSON and xenstore change committed). So that
+ * device can be safely removed. JSON entry is left untouched and
+ * becomes stale, but this is a valid state -- next time when a
+ * device with same identifier gets added, the stale entry gets
+ * overwritten.
+ *
+ * In b), it doesn't see that device in xenstore, but it will commence
+ * anyway. Eventually a force removal is initiated, which will forcely
+ * remove xenstore entry.
+ *
+ * If hotplug threads creates xenstore entry (therefore JSON entry as
+ * well) before force removal, that xenstore entry is removed. We're
+ * left with JSON stale entry but not xenstore entry, which is a valid
+ * state.
+ *
+ * If hotplug thread has not created xenstore entry when the removal
+ * is committed, we're obviously safe. Hotplug thread will add in
+ * xenstore entry afterwards. We have both JSON and xenstore entry,
+ * it's a valid state.
+ */
_hidden void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
libxl_device_disk *disk,
libxl__ao_device *aodev);
@@ -3273,6 +3347,63 @@ static inline void libxl__update_config_vtpm(libxl__gc
*gc,
libxl_uuid_copy(CTX, &dst->uuid, &src->uuid);
}
+/* Macros used to compare device identifier. Returns true if the two
+ * devices have same identifier. */
+#define COMPARE_DEVID(a, b) ((a)->devid == (b)->devid)
+#define COMPARE_DISK(a, b) (!strcmp((a)->vdev, (b)->vdev))
+#define COMPARE_PCI(a, b) ((a)->func == (b)->func && \
+ (a)->bus == (b)->bus && \
+ (a)->dev == (b)->dev)
+
+/* DEVICE_ADD
+ *
+ * Add a device in libxl_domain_config structure
+ *
+ * It takes 6 parameters:
+ * type: the type of the device, say nic, vtpm, disk, pci etc
+ * ptr: pointer to the start of the array, the array must be
+ * of type libxl_device_#type
+ * domid: domain id of target domain
+ * dev: the device that is to be added / removed / updated
+ * compare: the COMPARE_* macro used to compare @dev's identifier to
+ * those in the array pointed to by @ptr
+ * d_config: pointer to template domain config
+ *
+ * For most device types (nic, vtpm), the array pointer @ptr can be
+ * derived from @type, pci device being the exception, hence we need
+ * to have @ptr.
+ *
+ * If there is already a device with the same identifier in d_config,
+ * that entry is updated.
+ */
+#define DEVICE_ADD(type, ptr, domid, dev, compare, d_config) \
+ ({ \
+ int DA_x; \
+ libxl_device_##type *DA_p = NULL; \
+ \
+ /* Check for existing device */ \
+ for (DA_x = 0; DA_x < (d_config)->num_##ptr; DA_x++) { \
+ if (compare(&(d_config)->ptr[DA_x], (dev))) { \
+ DA_p = &(d_config)->ptr[DA_x]; \
+ break; \
+ } \
+ } \
+ \
+ if (!DA_p) { \
+ (d_config)->ptr = \
+ libxl__realloc(NOGC, (d_config)->ptr, \
+ ((d_config)->num_##ptr + 1) * \
+ sizeof(libxl_device_##type)); \
+ DA_p = &(d_config)->ptr[(d_config)->num_##ptr]; \
+ (d_config)->num_##ptr++; \
+ } else { \
+ libxl_device_##type##_dispose(DA_p); \
+ } \
+ \
+ libxl_device_##type##_init(DA_p); \
+ libxl_device_##type##_copy(CTX, DA_p, (dev)); \
+ })
+
#endif
/*
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 38a9642..b87819f 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -126,6 +126,13 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,
uint32_t domid, libxl_d
xs_transaction_t t;
libxl__device *device;
int rc;
+ libxl_domain_config d_config;
+ libxl_device_pci pcidev_saved;
+ libxl__carefd *lock = NULL;
+
+ libxl_domain_config_init(&d_config);
+ libxl_device_pci_init(&pcidev_saved);
+ libxl_device_pci_copy(CTX, &pcidev_saved, pcidev);
be_path = libxl__sprintf(gc, "%s/backend/pci/%d/0",
libxl__xs_get_dompath(gc, 0), domid);
num_devs = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/num_devs",
be_path));
@@ -153,6 +160,17 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,
uint32_t domid, libxl_d
GCNEW(device);
libxl__device_from_pcidev(gc, domid, pcidev, device);
+ lock = libxl__lock_domain_userdata(gc, domid);
+ if (!lock) {
+ rc = ERROR_LOCK_FAIL;
+ goto out;
+ }
+
+ rc = libxl__get_domain_configuration(gc, domid, &d_config);
+ if (rc) goto out;
+
+ DEVICE_ADD(pci, pcidevs, domid, &pcidev_saved, COMPARE_PCI, &d_config);
+
for (;;) {
rc = libxl__xs_transaction_start(gc, &t);
if (rc) goto out;
@@ -165,6 +183,9 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,
uint32_t domid, libxl_d
goto out;
}
+ rc = libxl__set_domain_configuration(gc, domid, &d_config);
+ if (rc) goto out;
+
libxl__xs_writev(gc, t, be_path,
libxl__xs_kvs_of_flexarray(gc, back, back->count));
@@ -175,6 +196,9 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,
uint32_t domid, libxl_d
out:
libxl__xs_transaction_abort(gc, &t);
+ if (lock) libxl__unlock_domain_userdata(lock);
+ libxl_device_pci_dispose(&pcidev_saved);
+ libxl_domain_config_dispose(&d_config);
return rc;
}
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |