libxl: disallow attaching the same device more than once

Originally the code allowed users to attach the same device more than
once. It just stupidly overwrites xenstore entries. This is bogus as
frontend will be very confused.

Introduce a helper function to check if the device to be written to
xenstore already exists. A new error code is also introduced.

The check and add are within one xs transaction.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 9bb1a90bbe82ae52609a261b3c58016cad3bcffc..ad3495a8047325e7cc1f5c983d072358fc56bd01 100644 (file)
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1893,6 +1893,7 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
     flexarray_t *back;
     libxl__device *device;
     int rc;
+    xs_transaction_t t = XBT_NULL;
 
     rc = libxl__device_vtpm_setdefault(gc, vtpm);
     if (rc) goto out;
@@ -1932,10 +1933,30 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
     flexarray_append(front, "handle");
     flexarray_append(front, GCSPRINTF("%d", vtpm->devid));
 
-    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);
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto out;
+
+        rc = libxl__device_exists(gc, t, device);
+        if (rc < 0) goto out;
+        if (rc == 1) {              /* already exists in xenstore */
+            LOG(ERROR, "device already exists in xenstore");
+            aodev->action = LIBXL__DEVICE_ACTION_ADD; /* for error message */
+            rc = ERROR_DEVICE_EXISTS;
+            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),
+                                  NULL);
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out;
+    }
 
     aodev->dev = device;
     aodev->action = LIBXL__DEVICE_ACTION_ADD;
@@ -1943,6 +1964,7 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
 
     rc = 0;
 out:
+    libxl__xs_transaction_abort(gc, &t);
     aodev->rc = rc;
     if(rc) aodev->callback(egc, aodev);
     return;
@@ -2209,6 +2231,15 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
             goto out;
         }
 
+        rc = libxl__device_exists(gc, t, device);
+        if (rc < 0) goto out;
+        if (rc == 1) {              /* already exists in xenstore */
+            LOG(ERROR, "device already exists in xenstore");
+            aodev->action = LIBXL__DEVICE_ACTION_ADD; /* for error message */
+            rc = ERROR_DEVICE_EXISTS;
+            goto out;
+        }
+
         switch (disk->backend) {
             case LIBXL_DISK_BACKEND_PHY:
                 dev = disk->pdev_path;
@@ -2998,6 +3029,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
     flexarray_t *back;
     libxl__device *device;
     int rc;
+    xs_transaction_t t = XBT_NULL;
 
     rc = libxl__device_nic_setdefault(gc, nic, domid);
     if (rc) goto out;
@@ -3068,10 +3100,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)));
-    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);
+
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto out;
+
+        rc = libxl__device_exists(gc, t, device);
+        if (rc < 0) goto out;
+        if (rc == 1) {              /* already exists in xenstore */
+            LOG(ERROR, "device already exists in xenstore");
+            aodev->action = LIBXL__DEVICE_ACTION_ADD; /* for error message */
+            rc = ERROR_DEVICE_EXISTS;
+            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),
+                                  NULL);
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out;
+    }
 
     aodev->dev = device;
     aodev->action = LIBXL__DEVICE_ACTION_ADD;
@@ -3079,6 +3132,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
 
     rc = 0;
 out:
+    libxl__xs_transaction_abort(gc, &t);
     aodev->rc = rc;
     if (rc) aodev->callback(egc, aodev);
     return;

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index f8a2e1ba9213013af7a3cb6b20057e458e267db3..045212fa5b62c5212472ae9e29cf5ff4639f8bf8 100644 (file)
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -40,6 +40,25 @@ char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device)
                      device->domid, device->devid);
 }
 
+/* Returns 1 if device exists, 0 if not, ERROR_* (<0) on error. */
+int libxl__device_exists(libxl__gc *gc, xs_transaction_t t,
+                         libxl__device *device)
+{
+    int rc;
+    char *be_path = libxl__device_backend_path(gc, device);
+    const char *dir;
+
+    be_path = libxl__device_backend_path(gc, device);
+    rc = libxl__xs_read_checked(gc, t, be_path, &dir);
+
+    if (rc)
+        return rc;
+
+    if (dir)
+        return 1;
+    return 0;
+}
+
 int libxl__parse_backend_path(libxl__gc *gc,
                               const char *path,
                               libxl__device *dev)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 3b8f74edf6155f0fb13bc63dccbe3137fb27acd0..fafef5a42025cc4f530b4da75019d4d2745a6deb 100644 (file)
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1033,6 +1033,9 @@ _hidden int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
                                       libxl__device_console *console,
                                       libxl__domain_build_state *state);
 
+/* Returns 1 if device exists, 0 if not, ERROR_* (<0) on error. */
+_hidden int libxl__device_exists(libxl__gc *gc, xs_transaction_t t,
+                                 libxl__device *device);
 _hidden int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
         libxl__device *device, char **bents, char **fents, char **ro_fents);
 _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device);

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 0500cf3593e9ae6dc581748b94ce1e0be45cb323..38a9642e61abeaa3fdc98cf79deb934661846ea8 100644 (file)
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -124,6 +124,8 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_d
     char *num_devs, *be_path;
     int num = 0;
     xs_transaction_t t;
+    libxl__device *device;
+    int rc;
 
     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));
@@ -148,15 +150,32 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_d
     if (!starting)
         flexarray_append_pair(back, "state", libxl__sprintf(gc, "%d", 7));
 
-retry_transaction:
-    t = xs_transaction_start(ctx->xsh);
-    libxl__xs_writev(gc, t, be_path,
-                    libxl__xs_kvs_of_flexarray(gc, back, back->count));
-    if (!xs_transaction_end(ctx->xsh, t, 0))
-        if (errno == EAGAIN)
-            goto retry_transaction;
+    GCNEW(device);
+    libxl__device_from_pcidev(gc, domid, pcidev, device);
 
-    return 0;
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto out;
+
+        rc = libxl__device_exists(gc, t, device);
+        if (rc < 0) goto out;
+        if (rc == 1) {
+            LOG(ERROR, "device already exists in xenstore");
+            rc = ERROR_DEVICE_EXISTS;
+            goto out;
+        }
+
+        libxl__xs_writev(gc, t, be_path,
+                         libxl__xs_kvs_of_flexarray(gc, back, back->count));
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out;
+    }
+
+out:
+    libxl__xs_transaction_abort(gc, &t);
+    return rc;
 }
 
 static int libxl__device_pci_remove_xenstore(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev)

diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 1cd635ea8ca11aa4ec191bfeabc0b34979b08b62..f1fcbc312bf596f764f0eb2e7f9c08d5b192182f 100644 (file)
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -60,6 +60,7 @@ libxl_error = Enumeration("error", [
     (-14, "UNKNOWN_CHILD"),
     (-15, "LOCK_FAIL"),
     (-16, "JSON_CONFIG_EMPTY"),
+    (-17, "DEVICE_EXISTS"),
     ], value_namespace = "")
 
 libxl_domain_type = Enumeration("domain_type", [