diff options
author | George Dunlap <george.dunlap@eu.citrix.com> | 2012-12-06 10:19:08 +0000 |
---|---|---|
committer | George Dunlap <george.dunlap@eu.citrix.com> | 2012-12-06 10:19:08 +0000 |
commit | d7942d1dbe5d98cef71c00c51f2826eefb9273d0 (patch) | |
tree | 3a34cc02873e6c66c59a5f4830ce0dacdc9893f9 /tools/libxl/libxl.c | |
parent | f60d158a02165aac0eb824106f0655589b4f683e (diff) | |
download | xen-d7942d1dbe5d98cef71c00c51f2826eefb9273d0.tar.gz xen-d7942d1dbe5d98cef71c00c51f2826eefb9273d0.tar.bz2 xen-d7942d1dbe5d98cef71c00c51f2826eefb9273d0.zip |
libxl: Make an internal function explicitly check existence of expected paths
libxl__device_disk_from_xs_be() was failing without error for some
missing xenstore nodes in a backend, while assuming (without checking)
that other nodes were valid, causing a crash when another internal
error wrote these nodes in the wrong place.
Make this function consistent by:
* Checking the existence of all nodes before using
* Choosing a default only when the node is not written in device_disk_add()
* Failing with log msg if any node written by device_disk_add() is not present
* Returning an error on failure
* Disposing of the structure before returning using libxl_device_disk_displose()
Also make the callers of the function pay attention to the error and
behave appropriately. In the case of libxl__append_disk_list_of_type(),
this means only incrementing *ndisks as the disk structures are
successfully initialized.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
Diffstat (limited to 'tools/libxl/libxl.c')
-rw-r--r-- | tools/libxl/libxl.c | 67 |
1 files changed, 47 insertions, 20 deletions
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index b411dd1a6f..8d921bceaa 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -2169,9 +2169,9 @@ void libxl__device_disk_add(libxl__egc *egc, uint32_t domid, device_disk_add(egc, domid, disk, aodev, NULL, NULL); } -static void libxl__device_disk_from_xs_be(libxl__gc *gc, - const char *be_path, - libxl_device_disk *disk) +static int libxl__device_disk_from_xs_be(libxl__gc *gc, + const char *be_path, + libxl_device_disk *disk) { libxl_ctx *ctx = libxl__gc_owner(gc); unsigned int len; @@ -2179,6 +2179,7 @@ static void libxl__device_disk_from_xs_be(libxl__gc *gc, libxl_device_disk_init(disk); + /* "params" may not be present; but everything else must be. */ tmp = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(gc, "%s/params", be_path), &len); if (tmp && strchr(tmp, ':')) { @@ -2187,21 +2188,36 @@ static void libxl__device_disk_from_xs_be(libxl__gc *gc, } else { disk->pdev_path = tmp; } - libxl_string_to_backend(ctx, - libxl__xs_read(gc, XBT_NULL, - libxl__sprintf(gc, "%s/type", be_path)), - &(disk->backend)); + + + tmp = libxl__xs_read(gc, XBT_NULL, + libxl__sprintf(gc, "%s/type", be_path)); + if (!tmp) { + LOG(ERROR, "Missing xenstore node %s/type", be_path); + goto cleanup; + } + libxl_string_to_backend(ctx, tmp, &(disk->backend)); + disk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(gc, "%s/dev", be_path), &len); + if (!disk->vdev) { + LOG(ERROR, "Missing xenstore node %s/dev", be_path); + goto cleanup; + } + tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf (gc, "%s/removable", be_path)); - - if (tmp) - disk->removable = atoi(tmp); - else - disk->removable = 0; + if (!tmp) { + LOG(ERROR, "Missing xenstore node %s/removable", be_path); + goto cleanup; + } + disk->removable = atoi(tmp); tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/mode", be_path)); + if (!tmp) { + LOG(ERROR, "Missing xenstore node %s/mode", be_path); + goto cleanup; + } if (!strcmp(tmp, "w")) disk->readwrite = 1; else @@ -2209,9 +2225,18 @@ static void libxl__device_disk_from_xs_be(libxl__gc *gc, tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/device-type", be_path)); + if (!tmp) { + LOG(ERROR, "Missing xenstore node %s/device-type", be_path); + goto cleanup; + } disk->is_cdrom = !strcmp(tmp, "cdrom"); disk->format = LIBXL_DISK_FORMAT_UNKNOWN; + + return 0; +cleanup: + libxl_device_disk_dispose(disk); + return ERROR_FAIL; } int libxl_vdev_to_device_disk(libxl_ctx *ctx, uint32_t domid, @@ -2237,9 +2262,7 @@ int libxl_vdev_to_device_disk(libxl_ctx *ctx, uint32_t domid, if (!path) goto out; - libxl__device_disk_from_xs_be(gc, path, disk); - - rc = 0; + rc = libxl__device_disk_from_xs_be(gc, path, disk); out: GC_FREE; return rc; @@ -2256,6 +2279,8 @@ static int libxl__append_disk_list_of_type(libxl__gc *gc, char **dir = NULL; unsigned int n = 0; libxl_device_disk *pdisk = NULL, *pdisk_end = NULL; + int rc=0; + int initial_disks = *ndisks; be_path = libxl__sprintf(gc, "%s/backend/%s/%d", libxl__xs_get_dompath(gc, 0), type, domid); @@ -2266,17 +2291,19 @@ static int libxl__append_disk_list_of_type(libxl__gc *gc, if (tmp == NULL) return ERROR_NOMEM; *disks = tmp; - pdisk = *disks + *ndisks; - *ndisks += n; - pdisk_end = *disks + *ndisks; + pdisk = *disks + initial_disks; + pdisk_end = *disks + initial_disks + n; for (; pdisk < pdisk_end; pdisk++, dir++) { const char *p; p = libxl__sprintf(gc, "%s/%s", be_path, *dir); - libxl__device_disk_from_xs_be(gc, p, pdisk); + if ((rc=libxl__device_disk_from_xs_be(gc, p, pdisk))) + goto out; pdisk->backend_domid = 0; + *ndisks += 1; } } - return 0; +out: + return rc; } libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int *num) |