From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: [PATCH v1 05/10] libxl: synchronise configuration when we hotplug a device Date: Thu, 10 Jul 2014 15:32:20 +0100 Message-ID: <1405002745-5034-6-git-send-email-wei.liu2@citrix.com> References: <1405002745-5034-1-git-send-email-wei.liu2@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1405002745-5034-1-git-send-email-wei.liu2@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xen.org Cc: Wei Liu , ian.jackson@eu.citrix.com, ian.campbell@citrix.com List-Id: xen-devel@lists.xenproject.org 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 --- 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