From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: [PATCH v3 07/15] libxl: disallow attaching the same device more than once Date: Thu, 4 Sep 2014 23:43:13 +0100 Message-ID: <1409870601-7538-8-git-send-email-wei.liu2@citrix.com> References: <1409870601-7538-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: <1409870601-7538-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 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 --- change in v3: place check and add within one xs transaction --- 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", [ -- 1.7.10.4