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

[Xen-changelog] [xen master] libxl: disallow attaching the same device more than once



commit 0d5d5eca4afa35332891112f5f25c2f2cba790a2
Author:     Wei Liu <wei.liu2@xxxxxxxxxx>
AuthorDate: Thu Sep 4 23:43:13 2014 +0100
Commit:     Ian Campbell <ian.campbell@xxxxxxxxxx>
CommitDate: Tue Sep 9 12:46:23 2014 +0100

    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@xxxxxxxxxx>
    Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
---
 tools/libxl/libxl.c          |   70 +++++++++++++++++++++++++++++++++++++-----
 tools/libxl/libxl_device.c   |   19 +++++++++++
 tools/libxl/libxl_internal.h |    3 ++
 tools/libxl/libxl_pci.c      |   35 ++++++++++++++++-----
 tools/libxl/libxl_types.idl  |    1 +
 5 files changed, 112 insertions(+), 16 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 9bb1a90..ad3495a 100644
--- 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 f8a2e1b..045212f 100644
--- 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 3b8f74e..fafef5a 100644
--- 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 0500cf3..38a9642 100644
--- 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 1cd635e..f1fcbc3 100644
--- 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", [
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog


 


Rackspace

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