[PATCH V5 32/32] libxl: synchronize configuration when we plug / unplug device

[Wei Liu <wei.liu2@citrix.com>]

We synchronize domain configuration in the callback, right before we
return to caller. We depends solely on the return value of AO to
determine if AO is successful.

This approach cannot deal with half-baked device state (say, if AO
returns success but for various reasons other parts just fail and we
have some xenstore entries written / resource allocated). However we
cannot make thing worse as the original code cannot handle that either.
The good side of this approach is that in the future we can compare the
stored configuration and xenstore entries to spot any half-baked device
state so it can help fix that issue in the future.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c          |  152 +++++++++++++++++++++++++++++++++++-------
 tools/libxl/libxl_internal.h |    2 +
 2 files changed, 129 insertions(+), 25 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index f93096b..a7bf0a4 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1794,29 +1794,6 @@ out:
 
 /******************************************************************************/
 
-/* generic callback for devices that only need to set ao_complete */
-static void device_addrm_aocomplete(libxl__egc *egc, libxl__ao_device *aodev)
-{
-    STATE_AO_GC(aodev->ao);
-
-    if (aodev->rc) {
-        if (aodev->dev) {
-            LOG(ERROR, "unable to %s %s with id %u",
-                        libxl__device_action_to_string(aodev->action),
-                        libxl__device_kind_to_string(aodev->dev->kind),
-                        aodev->dev->devid);
-        } else {
-            LOG(ERROR, "unable to %s device",
-                       libxl__device_action_to_string(aodev->action));
-        }
-        goto out;
-    }
-
-out:
-    libxl__ao_complete(egc, ao, aodev->rc);
-    return;
-}
-
 /* common function to get next device id */
 static int libxl__device_nextid(libxl__gc *gc, uint32_t domid, char *device)
 {
@@ -3548,6 +3525,126 @@ out:
     }                                                                   \
     libxl_domain_config_dispose(&d_config);                             \
 
+static void device_nic_fixup(libxl_ctx *ctx, libxl_device_nic *dst,
+                             libxl_device_nic *src)
+{
+    dst->devid = src->devid;
+    libxl_mac_copy(ctx, &dst->mac, &src->mac);
+}
+
+static void device_vtpm_fixup(libxl_ctx *ctx, libxl_device_vtpm *dst,
+                              libxl_device_vtpm *src)
+{
+    libxl_uuid_copy(ctx, &dst->uuid, &src->uuid);
+}
+
+static void device_disk_fixup(libxl_ctx *ctx, libxl_device_disk *dst,
+                              libxl_device_disk *src)
+{
+}
+
+#define DEVICE_AO_FAILED_MSG                                            \
+    do {                                                                \
+        if (aodev->dev) {                                               \
+            LOG(ERROR, "unable to %s %s with id %u",                    \
+                libxl__device_action_to_string(aodev->action),          \
+                libxl__device_kind_to_string(aodev->dev->kind),         \
+                aodev->dev->devid);                                     \
+        } else {                                                        \
+            LOG(ERROR, "unable to %s device",                           \
+                libxl__device_action_to_string(aodev->action));         \
+        }                                                               \
+    } while (0)
+
+
+#define DEFINE_DEVICE_ADD_AOCOMPLETE(type,ptr,cnt)                      \
+    static void device_add_##type##_aocomplete(libxl__egc *egc,         \
+                                               libxl__ao_device *aodev) \
+    {                                                                   \
+        STATE_AO_GC(aodev->ao);                                         \
+        int rc;                                                         \
+                                                                        \
+        if (aodev->rc) {                                                \
+            DEVICE_AO_FAILED_MSG;                                       \
+            goto out;                                                   \
+        } else {                                                        \
+            libxl_device_##type *p;                                     \
+                                                                        \
+            LOAD_DOMAIN_CONFIG(aodev->dev->domid);                      \
+                                                                        \
+            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, aodev->pdev_copy);       \
+                                                                        \
+            device_##type##_fixup(CTX, p, aodev->pdev);                 \
+                                                                        \
+            STORE_DOMAIN_CONFIG(aodev->dev->domid);                     \
+        }                                                               \
+                                                                        \
+    out:                                                                \
+        /* Dispose our internal copy, which is allocated at the entry of \
+         * this AO. */                                                  \
+        libxl_device_##type##_dispose(aodev->pdev_copy);                \
+        libxl__ao_complete(egc, ao, aodev->rc);                         \
+        return;                                                         \
+    }
+
+DEFINE_DEVICE_ADD_AOCOMPLETE(nic, nics, num_nics);
+DEFINE_DEVICE_ADD_AOCOMPLETE(vtpm, vtpms, num_vtpms);
+DEFINE_DEVICE_ADD_AOCOMPLETE(disk, disks, num_disks);
+
+#define COMPARE_DEVID(a, b) ((a)->devid == (b)->devid)
+#define COMPARE_DISK(a, b) (!strcmp((a)->vdev, (b)->vdev))
+
+#define DEFINE_DEVICE_RM_AOCOMPLETE(type,ptr,cnt,compare)               \
+    static void device_rm_##type##_aocomplete(libxl__egc *egc,          \
+                                              libxl__ao_device *aodev)  \
+    {                                                                   \
+        STATE_AO_GC(aodev->ao);                                         \
+        int rc;                                                         \
+                                                                        \
+        if (aodev->rc) {                                                \
+            DEVICE_AO_FAILED_MSG;                                       \
+            goto out;                                                   \
+        } else {                                                        \
+            int i, j;                                                   \
+            libxl_device_##type *p = aodev->pdev;                       \
+            LOAD_DOMAIN_CONFIG(aodev->dev->domid);                      \
+                                                                        \
+            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];              \
+                    }                                                   \
+                    j++;                                                \
+                }                                                       \
+            }                                                           \
+                                                                        \
+            d_config.ptr =                                              \
+                libxl__realloc(gc, d_config.ptr,                        \
+                               j * sizeof(libxl_device_##type));        \
+            d_config.cnt = j;                                           \
+                                                                        \
+            STORE_DOMAIN_CONFIG(aodev->dev->domid);                     \
+        }                                                               \
+                                                                        \
+    out:                                                                \
+        libxl__ao_complete(egc, ao, aodev->rc);                         \
+        return;                                                         \
+    }
+
+DEFINE_DEVICE_RM_AOCOMPLETE(nic, nics, num_nics, COMPARE_DEVID);
+DEFINE_DEVICE_RM_AOCOMPLETE(vtpm, vtpms, num_vtpms, COMPARE_DEVID);
+DEFINE_DEVICE_RM_AOCOMPLETE(disk, disks, num_disks, COMPARE_DISK);
+DEFINE_DEVICE_RM_AOCOMPLETE(vkb, vkbs, num_vkbs, COMPARE_DEVID);
+DEFINE_DEVICE_RM_AOCOMPLETE(vfb, vfbs, num_vfbs, COMPARE_DEVID);
+
 /* Macro for defining device remove/destroy functions in a compact way */
 /* The following functions are defined:
  * libxl_device_disk_remove
@@ -3577,9 +3674,11 @@ out:
                                                                         \
         GCNEW(aodev);                                                   \
         libxl__prepare_ao_device(ao, aodev);                            \
+        aodev->pdev = type;                                             \
+        aodev->pdev_copy = NULL; /* unused */                           \
         aodev->action = LIBXL__DEVICE_ACTION_REMOVE;                    \
         aodev->dev = device;                                            \
-        aodev->callback = device_addrm_aocomplete;                      \
+        aodev->callback = device_rm_##type##_aocomplete;                \
         aodev->force = f;                                               \
         libxl__initiate_device_remove(egc, aodev);                      \
                                                                         \
@@ -3631,8 +3730,11 @@ DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)
         libxl__ao_device *aodev;                                        \
                                                                         \
         GCNEW(aodev);                                                   \
+        aodev->pdev = type;                                             \
+        aodev->pdev_copy = libxl__zalloc(gc, sizeof(*type));            \
+        libxl_device_##type##_copy(CTX, aodev->pdev_copy, aodev->pdev); \
         libxl__prepare_ao_device(ao, aodev);                            \
-        aodev->callback = device_addrm_aocomplete;                      \
+        aodev->callback = device_add_##type##_aocomplete;               \
         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 21bb774..06503df 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2106,6 +2106,8 @@ struct libxl__ao_device {
     const char *what;
     int num_exec;
     libxl__ev_child child;
+    void *pdev; /* pointer to libxl_device_nic/vtpm/disk etc. */
+    void *pdev_copy; /* a copy of vanilla structure pointed to by pdev. */
 };
 
 /*
-- 
1.7.10.4