[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH v2 11/18] 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 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.

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          |   64 ++++++++++++++++++
 tools/libxl/libxl_internal.h |  151 ++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_pci.c      |   20 ++++++
 3 files changed, 235 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index e6fe238..184e126 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1888,6 +1888,11 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t 
domid,
     flexarray_t *back;
     libxl__device *device;
     int rc;
+    libxl_device_vtpm vtpm_saved;
+    libxl__carefd *lock = NULL;
+
+    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;
@@ -1902,6 +1907,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;
@@ -1936,6 +1943,18 @@ 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_data(gc, domid);
+        if (!lock) {
+            rc = ERROR_LOCK_FAIL;
+            goto out;
+        }
+
+        rc = DEVICE_ADD_JSON(vtpm, vtpms, num_vtpms, domid, &vtpm_saved,
+                             COMPARE_DEVID);
+        if (rc) 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),
@@ -1947,6 +1966,8 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t 
domid,
 
     rc = 0;
 out:
+    if (lock) libxl__unlock_domain_data(lock);
+    libxl_device_vtpm_dispose(&vtpm_saved);
     aodev->rc = rc;
     if(rc) aodev->callback(egc, aodev);
     return;
@@ -2186,6 +2207,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;
+    libxl_device_disk disk_saved;
+    libxl__carefd *lock = NULL;
+
+    libxl_device_disk_init(&disk_saved);
 
     libxl_domain_type type = libxl__domain_type(gc, domid);
     if (type == LIBXL_DOMAIN_TYPE_INVALID) {
@@ -2198,6 +2223,8 @@ static void device_disk_add(libxl__egc *egc, uint32_t 
domid,
      * we need to check if this device already exists in xenstore.
      */
     if (!get_vdev) {
+        libxl_device_disk_copy(CTX, &disk_saved, disk);
+
         /* Construct libxl__device from libxl_device_disk */
         libxl__device_disk_setdefault(gc, disk);
         GCNEW(device);
@@ -2216,6 +2243,18 @@ static void device_disk_add(libxl__egc *egc, uint32_t 
domid,
             rc = ERROR_DEVICE_EXISTS;
             goto out;
         }
+
+        if (aodev && aodev->update_json) {
+            lock = libxl__lock_domain_data(gc, domid);
+            if (!lock) {
+                rc = ERROR_LOCK_FAIL;
+                goto out;
+            }
+
+            rc = DEVICE_ADD_JSON(disk, disks, num_disks, domid,
+                                 &disk_saved, COMPARE_DISK);
+            if (rc) goto out;
+        }
     }
 
     for (;;) {
@@ -2378,6 +2417,8 @@ static void device_disk_add(libxl__egc *egc, uint32_t 
domid,
     rc = 0;
 
 out:
+    if (lock) libxl__unlock_domain_data(lock);
+    libxl_device_disk_dispose(&disk_saved);
     libxl__xs_transaction_abort(gc, &t);
     aodev->rc = rc;
     if (rc) aodev->callback(egc, aodev);
@@ -3034,6 +3075,11 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t 
domid,
     flexarray_t *back;
     libxl__device *device;
     int rc;
+    libxl_device_nic nic_saved;
+    libxl__carefd *lock = NULL;
+
+    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;
@@ -3048,6 +3094,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;
@@ -3113,6 +3161,19 @@ 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)));
+
+    if (aodev->update_json) {
+        lock = libxl__lock_domain_data(gc, domid);
+        if (!lock) {
+            rc = ERROR_LOCK_FAIL;
+            goto out;
+        }
+
+        rc = DEVICE_ADD_JSON(nic, nics, num_nics, domid, &nic_saved,
+                             COMPARE_DEVID);
+        if (rc) 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),
@@ -3124,6 +3185,8 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t 
domid,
 
     rc = 0;
 out:
+    if (lock) libxl__unlock_domain_data(lock);
+    libxl_device_nic_dispose(&nic_saved);
     aodev->rc = rc;
     if (rc) aodev->callback(egc, aodev);
     return;
@@ -3702,6 +3765,7 @@ DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)
                                                                         \
         GCNEW(aodev);                                                   \
         libxl__prepare_ao_device(ao, aodev);                            \
+        aodev->update_json = true;                                      \
         aodev->callback = device_add_aocomplete;                        \
         libxl__device_##type##_add(egc, domid, type, aodev);            \
                                                                         \
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 1b44c2c..a5eb7d4 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2144,6 +2144,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;
 };
 
 /*
@@ -3271,6 +3273,155 @@ 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)
+
+/* Expression int DEVICE_{ADD,REMOVE,UPDATE}_JSON
+ *
+ * Add / remove / update a device in JSON config file. Caller
+ * must hold data store lock before invoking these macros.
+ *
+ * They take 6 parameters:
+ *  type:    the type of the device, say nic, vtpm, disk, pci etc
+ *  ptr:     the pointer to array inside libxl_domain_config
+ *  cnt:     the counter of array
+ *  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
+ *
+ * Return 0 if no error occurs, ERROR_* otherwise.
+ *
+ * For most device types (nic, vtpm), the array name @ptr and array
+ * counter @cnt can be derived from @type, pci device being the
+ * exception, hence we need to have @ptr and @cnt.
+ */
+
+/* Add a device to JSON config. If there is already device with the
+ * same identifier in JSON config, no action will be taken.
+ * Use the "update" macro to update JSON config of a device.
+ */
+#define DEVICE_ADD_JSON(type, ptr, cnt, domid, dev, compare)            \
+    ({                                                                  \
+        int DAJ_x, DAJ_rc;                                              \
+        libxl_domain_config DAJ_d_config;                               \
+        libxl_device_##type *DAJ_p;                                     \
+                                                                        \
+        libxl_domain_config_init(&DAJ_d_config);                        \
+                                                                        \
+        DAJ_rc = libxl__get_domain_configuration(gc, (domid),           \
+                                             &DAJ_d_config);            \
+        if (DAJ_rc)                                                     \
+            goto DAJ_out;                                               \
+                                                                        \
+        /* Check for duplicated device */                               \
+        for (DAJ_x = 0; DAJ_x < DAJ_d_config.cnt; DAJ_x++) {            \
+            if (compare(&DAJ_d_config.ptr[DAJ_x], (dev))) {             \
+                DAJ_rc = 0;                                             \
+                goto DAJ_out;                                           \
+            }                                                           \
+        }                                                               \
+                                                                        \
+        DAJ_d_config.ptr =                                              \
+            libxl__realloc(NOGC, DAJ_d_config.ptr,                      \
+                           (DAJ_d_config.cnt + 1) *                     \
+                           sizeof(libxl_device_##type));                \
+        DAJ_p = &DAJ_d_config.ptr[DAJ_d_config.cnt];                    \
+        DAJ_d_config.cnt++;                                             \
+        libxl_device_##type##_copy(CTX, DAJ_p, (dev));                  \
+                                                                        \
+        DAJ_rc = libxl__set_domain_configuration(gc, (domid),           \
+                                                 &DAJ_d_config);        \
+                                                                        \
+    DAJ_out:                                                            \
+        libxl_domain_config_dispose(&DAJ_d_config);                     \
+        DAJ_rc;                                                         \
+    })
+
+/* Remove a device from JSON config. */
+#define DEVICE_REMOVE_JSON(type, ptr, cnt, domid, dev, compare)         \
+    ({                                                                  \
+        int DRJ_i, DRJ_j, DRJ_rc;                                       \
+        libxl_device_##type *DRJ_p = dev;                               \
+        libxl_domain_config DRJ_d_config;                               \
+                                                                        \
+        libxl_domain_config_init(&DRJ_d_config);                        \
+                                                                        \
+        DRJ_rc = libxl__get_domain_configuration(gc, (domid),           \
+                                                 &DRJ_d_config);        \
+        if (DRJ_rc)                                                     \
+            goto DRJ_out;                                               \
+                                                                        \
+        /* This loop may shuffle down the array */                      \
+        for (DRJ_i = DRJ_j = 0; DRJ_i < DRJ_d_config.cnt; DRJ_i++) {    \
+            if (!compare(&DRJ_d_config.ptr[DRJ_i], DRJ_p)) {            \
+                if (DRJ_i != DRJ_j) {                                   \
+                    libxl_device_##type DRJ_tmp;                        \
+                    DRJ_tmp = DRJ_d_config.ptr[DRJ_j];                  \
+                    DRJ_d_config.ptr[DRJ_j] = DRJ_d_config.ptr[DRJ_i];  \
+                    DRJ_d_config.ptr[DRJ_i] = DRJ_tmp;                  \
+                }                                                       \
+                DRJ_j++;                                                \
+            }                                                           \
+        }                                                               \
+                                                                        \
+        if (DRJ_i == DRJ_j) /* no matching entry found */               \
+            goto DRJ_out;                                               \
+                                                                        \
+        libxl_device_##type##_dispose(&DRJ_d_config.ptr[DRJ_j]);        \
+        DRJ_d_config.ptr =                                              \
+            libxl__realloc(NOGC, DRJ_d_config.ptr,                      \
+                           DRJ_j * sizeof(libxl_device_##type));        \
+        DRJ_d_config.cnt = DRJ_j;                                       \
+                                                                        \
+        DRJ_rc = libxl__set_domain_configuration(gc, (domid),           \
+                                                 &DRJ_d_config);        \
+                                                                        \
+    DRJ_out:                                                            \
+        libxl_domain_config_dispose(&DRJ_d_config);                     \
+        DRJ_rc;                                                         \
+    })
+
+/* Search device list for device with the same identifier as "dev",
+ * update that device with the content pointed to by "dev".
+ */
+#define DEVICE_UPDATE_JSON(type, ptr, cnt, domid, dev, compare)         \
+    ({                                                                  \
+        int DUJ_x, DUJ_rc;                                              \
+        libxl_domain_config DUJ_d_config;                               \
+        bool DUJ_updated = false;                                       \
+                                                                        \
+        libxl_domain_config_init(&DUJ_d_config);                        \
+                                                                        \
+        DUJ_rc = libxl__get_domain_configuration(gc, domid,             \
+                                                 &DUJ_d_config);        \
+        if (DUJ_rc)                                                     \
+            goto DUJ_out;                                               \
+                                                                        \
+        for (DUJ_x = 0; DUJ_x < DUJ_d_config.cnt; DUJ_x++) {            \
+            if (compare(&DUJ_d_config.ptr[DUJ_x], (dev))) {             \
+                libxl_device_##type##_dispose(                          \
+                    &DUJ_d_config.ptr[DUJ_x]);                          \
+                libxl_device_##type##_copy(CTX,                         \
+                                           &DUJ_d_config.ptr[DUJ_x],    \
+                                           (dev));                      \
+                DUJ_updated = true;                                     \
+            }                                                           \
+        }                                                               \
+        if (DUJ_updated)                                                \
+            DUJ_rc = libxl__set_domain_configuration(gc, domid,         \
+                                                     &DUJ_d_config);    \
+                                                                        \
+    DUJ_out:                                                            \
+        libxl_domain_config_dispose(&DUJ_d_config);                     \
+        DUJ_rc;                                                         \
+    })
+
 #endif
 
 /*
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index e05f047..357e274 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1044,6 +1044,10 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, 
libxl_device_pci *pcide
     int num_assigned, i, rc;
     libxl__device device;
     int stubdomid = 0;
+    libxl_device_pci pcidev_saved;
+    libxl__carefd *lock = NULL;
+
+    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));
@@ -1057,6 +1061,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;
 
@@ -1125,6 +1131,18 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, 
libxl_device_pci *pcide
         pfunc_mask = (1 << pcidev->func);
     }
 
+    if (!starting) {
+        lock = libxl__lock_domain_data(gc, domid);
+        if (!lock) {
+            rc = ERROR_LOCK_FAIL;
+            goto out;
+        }
+
+        rc = DEVICE_ADD_JSON(pci, pcidevs, num_pcidevs, domid, pcidev,
+                             COMPARE_PCI);
+        if (rc) goto out;
+    }
+
     for(rc = 0, i = 7; i >= 0; --i) {
         if ( (1 << i) & pfunc_mask ) {
             if ( pcidev->vfunc_mask == pfunc_mask ) {
@@ -1143,6 +1161,8 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, 
libxl_device_pci *pcide
     }
 
 out:
+    if (lock) libxl__unlock_domain_data(lock);
+    libxl_device_pci_dispose(&pcidev_saved);
     return rc;
 }
 
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.