From: Wei Liu <wei.liu2@citrix.com>
To: xen-devel@lists.xen.org
Cc: Wei Liu <wei.liu2@citrix.com>,
ian.jackson@eu.citrix.com, ian.campbell@citrix.com
Subject: [PATCH v1 05/10] libxl: synchronise configuration when we hotplug a device
Date: Thu, 10 Jul 2014 15:32:20 +0100 [thread overview]
Message-ID: <1405002745-5034-6-git-send-email-wei.liu2@citrix.com> (raw)
In-Reply-To: <1405002745-5034-1-git-send-email-wei.liu2@citrix.com>
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 locking pattern is as followed:
1. lock JSON domain config
2. write JSON entry
3. write xenstore entry for a device
4. unlock JSON domain config
And we ensure in code that JSON entry is always written before the
xenstore entry is written. That is, if 2 fails, 3 will not be executed.
We also ensure that if 2 and 3 are invoked in a loop, the previous
added JSON entry is removed before writing new one.
The locking patten is designed like this, because when adding a disk,
the caller might specify get_vdev which will get called in the middle of
a xenstore transaction to determine the identifier of a disk. Other
devices don't have this property, but I make them follow the same
pattern for consistency.
As we don't have a JSON config file for libxl toolstack domain
(currently Dom0) we need to skip JSON manipulation for it.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
tools/libxl/libxl.c | 96 ++++++++++++++++++++++++++++++++++++++--
tools/libxl/libxl_internal.h | 99 ++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_pci.c | 18 ++++++++
3 files changed, 210 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index e1ee7bf..5f9f94a 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1904,6 +1904,11 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
flexarray_t *back;
libxl__device *device;
unsigned int rc;
+ libxl_device_vtpm vtpm_saved;
+ int lock = -1;
+
+ 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;
@@ -1917,6 +1922,8 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
goto out;
}
}
+ vtpm_saved.devid = vtpm->devid;
+ libxl__update_config_vtpm(gc, &vtpm_saved, vtpm);
GCNEW(device);
rc = libxl__device_from_vtpm(gc, domid, vtpm, device);
@@ -1943,17 +1950,30 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
flexarray_append(front, "handle");
flexarray_append(front, GCSPRINTF("%d", vtpm->devid));
+ LOCK_DOMAIN_JSON_CONFIG(gc, domid, lock, rc);
+ if (rc) goto out;
+
+ DEVICE_ADD_JSON(vtpm, vtpms, num_vtpms, domid, &vtpm_saved,
+ COMPARE_DEVID, rc);
+ if (rc) {
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
+ goto out;
+ }
+
libxl__device_generic_add(gc, XBT_NULL, device,
libxl__xs_kvs_of_flexarray(gc, back, back->count),
libxl__xs_kvs_of_flexarray(gc, front, front->count),
NULL);
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
+
aodev->dev = device;
aodev->action = LIBXL__DEVICE_ACTION_ADD;
libxl__wait_device_connection(egc, aodev);
rc = 0;
out:
+ libxl_device_vtpm_dispose(&vtpm_saved);
aodev->rc = rc;
if(rc) aodev->callback(egc, aodev);
return;
@@ -2186,6 +2206,10 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
int rc;
libxl_ctx *ctx = gc->owner;
xs_transaction_t t = XBT_NULL;
+ int lock = -1;
+ libxl_device_disk disk_saved;
+
+ libxl_device_disk_init(&disk_saved);
libxl_domain_type type = libxl__domain_type(gc, domid);
if (type == LIBXL_DOMAIN_TYPE_INVALID) {
@@ -2193,21 +2217,56 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
goto out;
}
+ libxl_device_disk_copy(CTX, &disk_saved, disk);
+
+ LOCK_DOMAIN_JSON_CONFIG(gc, domid, lock, rc);
+ if (rc) goto out;
+
for (;;) {
rc = libxl__xs_transaction_start(gc, &t);
- if (rc) goto out;
+ if (rc) {
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
+ goto out;
+ }
+
+ /* This is a loop, if disk has been added to JSON before, we
+ * need to remove it, then add it later, because disk->vdev
+ * might change.
+ */
+ DEVICE_REMOVE_JSON(disk, disks, num_disks, domid, &disk_saved,
+ COMPARE_DISK, rc);
+ if (rc) {
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
+ goto out;
+ }
if (get_vdev) {
assert(get_vdev_user);
disk->vdev = get_vdev(gc, get_vdev_user, t);
if (disk->vdev == NULL) {
rc = ERROR_FAIL;
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
goto out;
}
}
+ if (strcmp(disk_saved.vdev, disk->vdev)) {
+ free(disk_saved.vdev);
+ disk_saved.vdev = libxl__strdup(NOGC, disk->vdev);
+ }
+
+ DEVICE_ADD_JSON(disk, disks, num_disks, domid, &disk_saved,
+ COMPARE_DISK, rc);
+ if (rc) {
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
+ goto out;
+ }
+
rc = libxl__device_disk_setdefault(gc, disk);
- if (rc) goto out;
+ if (rc) {
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
+ goto out;
+ }
front = flexarray_make(gc, 16, 1);
back = flexarray_make(gc, 16, 1);
@@ -2217,6 +2276,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
if (rc != 0) {
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
" virtual disk identifier %s", disk->vdev);
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
goto out;
}
@@ -2257,6 +2317,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
if (!dev) {
LOG(ERROR, "failed to get blktap devpath for %p\n",
disk->pdev_path);
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
rc = ERROR_FAIL;
goto out;
}
@@ -2281,6 +2342,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
default:
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
rc = ERROR_INVAL;
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
goto out;
}
@@ -2343,9 +2405,14 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
rc = libxl__xs_transaction_commit(gc, &t);
if (!rc) break;
- if (rc < 0) goto out;
+ if (rc < 0) {
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
+ goto out;
+ }
}
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
+
aodev->dev = device;
aodev->action = LIBXL__DEVICE_ACTION_ADD;
libxl__wait_device_connection(egc, aodev);
@@ -2353,6 +2420,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
rc = 0;
out:
+ libxl_device_disk_dispose(&disk_saved);
libxl__xs_transaction_abort(gc, &t);
aodev->rc = rc;
if (rc) aodev->callback(egc, aodev);
@@ -3009,6 +3077,11 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
flexarray_t *back;
libxl__device *device;
unsigned int rc;
+ int lock = -1;
+ libxl_device_nic nic_saved;
+
+ 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;
@@ -3023,6 +3096,9 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
}
}
+ nic_saved.devid = nic->devid;
+ libxl__update_config_nic(gc, &nic_saved, nic);
+
GCNEW(device);
rc = libxl__device_from_nic(gc, domid, nic, device);
if ( rc != 0 ) goto out;
@@ -3079,17 +3155,31 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
flexarray_append(front, "mac");
flexarray_append(front, libxl__sprintf(gc,
LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
+
+ LOCK_DOMAIN_JSON_CONFIG(gc, domid, lock, rc);
+ if (rc) goto out;
+
+ DEVICE_ADD_JSON(nic, nics, num_nics, domid, nic, COMPARE_DEVID, rc);
+
+ if (rc) {
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
+ goto out;
+ }
+
libxl__device_generic_add(gc, XBT_NULL, device,
libxl__xs_kvs_of_flexarray(gc, back, back->count),
libxl__xs_kvs_of_flexarray(gc, front, front->count),
NULL);
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
+
aodev->dev = device;
aodev->action = LIBXL__DEVICE_ACTION_ADD;
libxl__wait_device_connection(egc, aodev);
rc = 0;
out:
+ libxl_device_nic_dispose(&nic_saved);
aodev->rc = rc;
if (rc) aodev->callback(egc, aodev);
return;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 72d21ad..f27a6e98 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3254,6 +3254,105 @@ static inline void libxl__update_config_vtpm(libxl__gc *gc,
libxl_uuid_copy(CTX, &dst->uuid, &src->uuid);
}
+/* Macro used to add the new device to JSON template */
+#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)
+
+#define LOCK_DOMAIN_JSON_CONFIG(gc, domid, lock, rc) \
+ do { \
+ if (domid != LIBXL_TOOLSTACK_DOMID) \
+ rc = libxl__lock_domain_configuration(gc, domid, &lock); \
+ else \
+ rc = 0; \
+ } while (0)
+
+#define UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock) \
+ do { \
+ if (domid != LIBXL_TOOLSTACK_DOMID) \
+ libxl__unlock_domain_configuration(gc, domid, &lock); \
+ } while (0)
+
+#define DEVICE_ADD_JSON(type, ptr, cnt, domid, dev, compare, rc) \
+ do { \
+ int x; \
+ libxl_domain_config d_config; \
+ libxl_device_##type *p; \
+ \
+ if (domid == LIBXL_TOOLSTACK_DOMID) \
+ break; \
+ \
+ libxl_domain_config_init(&d_config); \
+ \
+ rc = libxl__get_domain_configuration(gc, (domid), &d_config); \
+ if (rc) \
+ goto add_done; \
+ \
+ /* Check for duplicated device */ \
+ for (x = 0; x < d_config.cnt; x++) { \
+ if (compare(&d_config.ptr[x], (dev))) { \
+ rc = 0; \
+ goto add_done; \
+ } \
+ } \
+ \
+ d_config.ptr = \
+ libxl__realloc(gc, d_config.ptr, \
+ (d_config.cnt + 1) * \
+ sizeof(libxl_device_##type)); \
+ p = &d_config.ptr[d_config.cnt]; \
+ d_config.cnt++; \
+ libxl_device_##type##_copy(CTX, p, (dev)); \
+ \
+ rc = libxl__set_domain_configuration(gc, (domid), &d_config); \
+ \
+ add_done: \
+ libxl_domain_config_dispose(&d_config); \
+ } while (0)
+
+#define DEVICE_REMOVE_JSON(type, ptr, cnt, domid, dev, compare, rc) \
+ do { \
+ int i, j; \
+ libxl_device_##type *p = dev; \
+ libxl_domain_config d_config; \
+ bool removed = false; \
+ \
+ if (domid == LIBXL_TOOLSTACK_DOMID) \
+ break; \
+ \
+ libxl_domain_config_init(&d_config); \
+ \
+ rc = libxl__get_domain_configuration(gc, (domid), &d_config); \
+ if (rc) \
+ goto remove_done; \
+ \
+ for (i = j = 0; i < d_config.cnt; i++) { \
+ if (!compare(&d_config.ptr[i], p)) { \
+ if (i != j) { \
+ libxl_device_##type##_dispose(&d_config.ptr[j]); \
+ d_config.ptr[j] = d_config.ptr[i]; \
+ removed = true; \
+ } \
+ j++; \
+ } \
+ } \
+ \
+ if (!removed) /* no matching entry found */ \
+ goto remove_done; \
+ \
+ d_config.ptr = \
+ libxl__realloc(NOGC, d_config.ptr, \
+ j * sizeof(libxl_device_##type)); \
+ d_config.cnt = j; \
+ \
+ rc = libxl__set_domain_configuration(gc, (domid), &d_config); \
+ \
+ remove_done: \
+ libxl_domain_config_dispose(&d_config); \
+ } while (0)
+
#endif
/*
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 44d0453..d0d8f90 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1034,6 +1034,10 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcide
libxl_device_pci *assigned;
int num_assigned, i, rc;
int stubdomid = 0;
+ int lock = -1;
+ libxl_device_pci pcidev_saved;
+
+ libxl_device_pci_init(&pcidev_saved);
if (libxl__domain_type(gc, domid) == LIBXL_DOMAIN_TYPE_HVM) {
rc = xc_test_assign_device(ctx->xch, domid, pcidev_encode_bdf(pcidev));
@@ -1047,6 +1051,8 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcide
}
}
+ libxl_device_pci_copy(CTX, &pcidev_saved, pcidev);
+
rc = libxl__device_pci_setdefault(gc, pcidev);
if (rc) goto out;
@@ -1104,6 +1110,15 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcide
pfunc_mask = (1 << pcidev->func);
}
+ LOCK_DOMAIN_JSON_CONFIG(gc, domid, lock, rc);
+ if (rc) goto out;
+
+ DEVICE_ADD_JSON(pci, pcidevs, num_pcidevs, domid, pcidev, COMPARE_PCI, rc);
+ if (rc) {
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
+ goto out;
+ }
+
for(rc = 0, i = 7; i >= 0; --i) {
if ( (1 << i) & pfunc_mask ) {
if ( pcidev->vfunc_mask == pfunc_mask ) {
@@ -1121,7 +1136,10 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcide
}
}
+ UNLOCK_DOMAIN_JSON_CONFIG(gc, domid, lock);
+
out:
+ libxl_device_pci_dispose(&pcidev_saved);
return rc;
}
--
1.7.10.4
next prev parent reply other threads:[~2014-07-10 14:32 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-10 14:32 [PATCH v1 00/10] libxl: synchronise domain configuration Wei Liu
2014-07-10 14:32 ` [PATCH v1 01/10] libxl: libxl-json format and internal functions to get / set it Wei Liu
2014-07-16 16:11 ` Ian Campbell
2014-07-16 16:44 ` Wei Liu
2014-07-24 18:09 ` Ian Jackson
2014-07-24 18:29 ` Ian Jackson
2014-07-25 10:30 ` Wei Liu
2014-07-25 14:51 ` Ian Jackson
2014-07-10 14:32 ` [PATCH v1 02/10] libxl_internal: functions to lock / unlock domain configuration Wei Liu
2014-07-16 16:15 ` Ian Campbell
2014-07-16 16:44 ` Wei Liu
2014-07-17 11:29 ` Ian Campbell
2014-07-17 11:41 ` Wei Liu
2014-07-17 11:48 ` Ian Campbell
2014-07-24 18:24 ` Ian Jackson
2014-07-25 10:36 ` Wei Liu
2014-07-10 14:32 ` [PATCH v1 03/10] libxl: store a copy of vanilla domain configuration when creating domain Wei Liu
2014-07-16 16:18 ` Ian Campbell
2014-07-16 16:47 ` Wei Liu
2014-07-17 11:06 ` Ian Campbell
2014-07-17 11:46 ` Wei Liu
2014-07-24 18:52 ` Ian Jackson
2014-07-25 10:53 ` Wei Liu
2014-07-25 15:01 ` Ian Jackson
2014-07-25 15:43 ` Wei Liu
2014-07-25 17:14 ` Ian Jackson
2014-07-25 17:34 ` Wei Liu
2014-07-25 18:31 ` Ian Jackson
2014-07-25 19:47 ` Wei Liu
2014-07-28 9:42 ` Ian Campbell
2014-07-28 9:50 ` Ian Jackson
2014-07-10 14:32 ` [PATCH v1 04/10] libxl: separate device add/rm complete callbacks Wei Liu
2014-07-16 16:26 ` Ian Campbell
2014-07-16 16:48 ` Wei Liu
2014-07-10 14:32 ` Wei Liu [this message]
2014-07-16 16:48 ` [PATCH v1 05/10] libxl: synchronise configuration when we hotplug a device Ian Campbell
2014-07-16 17:12 ` Wei Liu
2014-07-17 11:44 ` Ian Campbell
2014-07-17 14:13 ` Wei Liu
2014-07-18 8:49 ` Ian Campbell
2014-07-18 11:22 ` Wei Liu
2014-07-18 12:20 ` Ian Campbell
2014-07-18 13:41 ` Wei Liu
2014-07-18 13:44 ` Ian Campbell
2014-07-25 16:06 ` Ian Jackson
2014-07-25 16:40 ` Wei Liu
2014-07-25 17:11 ` Ian Jackson
2014-07-25 17:19 ` Wei Liu
2014-07-10 14:32 ` [PATCH v1 06/10] libxl: synchronise configuration when we remove/destroy " Wei Liu
2014-07-16 16:58 ` Ian Campbell
2014-07-10 14:32 ` [PATCH v1 07/10] libxl: make libxl_cd_insert "eject" + "insert" Wei Liu
2014-07-17 10:44 ` Ian Campbell
2014-07-17 14:20 ` Wei Liu
2014-07-10 14:32 ` [PATCH v1 08/10] libxl: introduce libxl_get_memory_static_max Wei Liu
2014-07-17 10:47 ` Ian Campbell
2014-07-17 12:02 ` Wei Liu
2014-07-17 13:59 ` Ian Campbell
2014-07-29 13:39 ` Ian Jackson
2014-07-10 14:32 ` [PATCH v1 09/10] libxl: introduce libxl_retrieve_domain_configuration Wei Liu
2014-07-17 10:59 ` Ian Campbell
2014-07-17 12:11 ` Wei Liu
2014-07-17 14:02 ` Ian Campbell
2014-07-17 14:28 ` Wei Liu
2014-07-18 8:52 ` Ian Campbell
2014-07-18 11:17 ` Wei Liu
2014-07-29 15:31 ` Ian Jackson
2014-07-29 15:29 ` Ian Jackson
2014-07-10 14:32 ` [PATCH v1 10/10] xl: use libxl_retrieve_domain_configuration and JSON format Wei Liu
2014-07-17 11:13 ` Ian Campbell
2014-07-17 12:14 ` Wei Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1405002745-5034-6-git-send-email-wei.liu2@citrix.com \
--to=wei.liu2@citrix.com \
--cc=ian.campbell@citrix.com \
--cc=ian.jackson@eu.citrix.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).