From: Wei Liu <wei.liu2@citrix.com>
To: xen-devel@lists.xen.org
Cc: Wei Liu <wei.liu2@citrix.com>,
ian.jackson@eu.citrix.com, ian.campbell@citrix.com
Subject: [PATCH v3 07/15] libxl: disallow attaching the same device more than once
Date: Thu, 4 Sep 2014 23:43:13 +0100 [thread overview]
Message-ID: <1409870601-7538-8-git-send-email-wei.liu2@citrix.com> (raw)
In-Reply-To: <1409870601-7538-1-git-send-email-wei.liu2@citrix.com>
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>
---
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
next prev parent reply other threads:[~2014-09-04 22:43 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-04 22:43 [PATCH v3 00/15] libxl: synchronise domain configuration Wei Liu
2014-09-04 22:43 ` [PATCH v3 01/15] libxl: make userdata_path libxl internal function Wei Liu
2014-09-04 22:43 ` [PATCH v3 02/15] libxl: functions to lock / unlock libxl userdata store Wei Liu
2014-09-04 22:43 ` [PATCH v3 03/15] libxl: properly lock " Wei Liu
2014-09-09 10:52 ` Ian Campbell
2014-09-04 22:43 ` [PATCH v3 04/15] libxl: libxl-json format and internal functions to get / set it Wei Liu
2014-09-04 22:43 ` [PATCH v3 05/15] libxl: store a copy of configuration when creating domain Wei Liu
2014-09-04 22:43 ` [PATCH v3 06/15] libxl: introduce libxl__device_from_pcidev Wei Liu
2014-09-04 22:43 ` Wei Liu [this message]
2014-09-09 10:56 ` [PATCH v3 07/15] libxl: disallow attaching the same device more than once Ian Campbell
2014-09-04 22:43 ` [PATCH v3 08/15] libxl: introduce helper to initialise Dom0 Wei Liu
2014-09-05 13:22 ` Wei Liu
2014-09-09 11:03 ` Wei Liu
2014-09-09 11:16 ` Ian Campbell
2014-09-09 12:16 ` Ian Campbell
2014-09-04 22:43 ` [PATCH v3 09/15] libxl: synchronise configuration when we hotplug a device Wei Liu
2014-09-09 11:11 ` Ian Campbell
2014-09-09 11:23 ` Ian Campbell
2014-09-09 13:37 ` Wei Liu
2014-09-09 13:41 ` Ian Campbell
2014-09-04 22:43 ` [PATCH v3 10/15] libxl: make libxl_cd_insert "eject" + "insert" Wei Liu
2014-09-09 11:30 ` Ian Campbell
2014-09-09 13:38 ` Wei Liu
2014-09-15 14:38 ` Wei Liu
2014-09-04 22:43 ` [PATCH v3 11/15] libxl: refactor libxl_get_memory_target Wei Liu
2014-09-09 11:36 ` Ian Campbell
2014-09-09 11:39 ` Ian Campbell
2014-09-09 13:39 ` Wei Liu
2014-09-04 22:43 ` [PATCH v3 12/15] libxl: introduce libxl_retrieve_domain_configuration Wei Liu
2014-09-09 11:41 ` Ian Campbell
2014-09-04 22:43 ` [PATCH v3 13/15] libxl: introduce libxl_userdata_unlink Wei Liu
2014-09-09 11:42 ` Ian Campbell
2014-09-04 22:43 ` [PATCH v3 14/15] xl: use libxl_retrieve_domain_configuration and JSON format Wei Liu
2014-09-09 11:44 ` Ian Campbell
2014-09-04 22:43 ` [PATCH v3 15/15] xl: long output of "list" command now contains Dom0 information Wei Liu
-- strict thread matches above, loose matches on Subject: below --
2014-11-17 7:53 [PATCH v3 07/15] libxl: disallow attaching the same device more than once Li, Liang Z
2014-11-17 9:37 ` Wei Liu
2014-11-17 10:55 ` Li, Liang Z
2014-11-17 11:03 ` Wei Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1409870601-7538-8-git-send-email-wei.liu2@citrix.com \
--to=wei.liu2@citrix.com \
--cc=ian.campbell@citrix.com \
--cc=ian.jackson@eu.citrix.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).