From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: [PATCH v2 09/18] libxl: disallow attaching the same device more than once Date: Wed, 30 Jul 2014 19:23:50 +0100 Message-ID: <1406744639-28782-10-git-send-email-wei.liu2@citrix.com> References: <1406744639-28782-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: <1406744639-28782-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. Signed-off-by: Wei Liu --- tools/libxl/libxl.c | 50 ++++++++++++++++++++++++++++++++++++++++++ tools/libxl/libxl_device.c | 19 ++++++++++++++++ tools/libxl/libxl_internal.h | 3 +++ tools/libxl/libxl_pci.c | 12 ++++++++++ tools/libxl/libxl_types.idl | 1 + 5 files changed, 85 insertions(+) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 01dffa0..e6fe238 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -1906,6 +1906,15 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid, rc = libxl__device_from_vtpm(gc, domid, vtpm, device); if ( rc != 0 ) goto out; + rc = libxl__device_exists(gc, XBT_NULL, 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; + } + flexarray_append(back, "frontend-id"); flexarray_append(back, GCSPRINTF("%d", domid)); flexarray_append(back, "online"); @@ -2154,6 +2163,13 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid, * * The passed get_vdev function is also in charge of printing * the corresponding error message when appropiate. + * + * Note: + * + * The presence of get_vdev is also used to distinguish two + * cases: if get_vdev != NULL, then it is called by local attach + * function to run bootloader, otherwise it is called by + * libxl__device_disk_add wrapper. */ static void device_disk_add(libxl__egc *egc, uint32_t domid, libxl_device_disk *disk, @@ -2177,6 +2193,31 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, goto out; } + /* If we're not doing local attach, we're trying to do + * "block-attach". We already have a valid vdev at this point, and + * we need to check if this device already exists in xenstore. + */ + if (!get_vdev) { + /* Construct libxl__device from libxl_device_disk */ + libxl__device_disk_setdefault(gc, disk); + GCNEW(device); + rc = libxl__device_from_disk(gc, domid, disk, device); + if (rc != 0) { + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported" + " virtual disk identifier %s", disk->vdev); + goto out; + } + + rc = libxl__device_exists(gc, XBT_NULL, 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; + } + } + for (;;) { rc = libxl__xs_transaction_start(gc, &t); if (rc) goto out; @@ -3011,6 +3052,15 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid, rc = libxl__device_from_nic(gc, domid, nic, device); if ( rc != 0 ) goto out; + rc = libxl__device_exists(gc, XBT_NULL, 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; + } + flexarray_append(back, "frontend-id"); flexarray_append(back, libxl__sprintf(gc, "%d", domid)); flexarray_append(back, "online"); 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 02708c7..1b44c2c 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -1031,6 +1031,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..e05f047 100644 --- a/tools/libxl/libxl_pci.c +++ b/tools/libxl/libxl_pci.c @@ -1042,6 +1042,7 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcide unsigned int orig_vdev, pfunc_mask; libxl_device_pci *assigned; int num_assigned, i, rc; + libxl__device device; int stubdomid = 0; if (libxl__domain_type(gc, domid) == LIBXL_DOMAIN_TYPE_HVM) { @@ -1059,6 +1060,17 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcide rc = libxl__device_pci_setdefault(gc, pcidev); if (rc) goto out; + if (!starting) { + libxl__device_from_pcidev(gc, domid, pcidev, &device); + rc = libxl__device_exists(gc, XBT_NULL, &device); + if (rc < 0) goto out; + if (rc == 1) { /* already exists in xenstore */ + LOG(ERROR, "device already exists in xenstore"); + rc = ERROR_DEVICE_EXISTS; + goto out; + } + } + if (pcidev->seize && !pciback_dev_is_assigned(gc, pcidev)) { rc = libxl__device_pci_assignable_add(gc, pcidev, 1); if ( rc ) diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index d0a373b..1beb63d 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