[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v3] libxl: Make an internal function explicitly check existence of expected paths
# HG changeset patch # User George Dunlap <george.dunlap@xxxxxxxxxxxxx> # Date 1354732124 0 # Node ID 21504ec56304ada2f093c30b290ac33c28381ae1 # Parent 670b07e8d7382229639af0d1df30071e6c1ebb19 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. v3: * Make a failure path in libxl__device_disk_from_be() to free allocations from half-initialized disks * Modify libxl__append_disk_list_of_type to only update *ndisks as they are completed. This will allow the only caller (libxl_device_disk_list()) to clean up the completed disks properly on an error. v2: * Remove "Internal error", as the failure will most likely look internal * Use LOG(ERROR...) macros for incrased prettiness Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -2169,9 +2169,9 @@ void libxl__device_disk_add(libxl__egc * 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_b 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_b } 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_b 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 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_ty 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_ty 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) _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |