Xen project Mailing List

> * Handling non-existent nodes in the backend xenstore entries more
> gracefully

I wonder if it should error out here rather than assume a default -- the
nodes in question should have been written by libxl itself (I think, I
don't think these are backend provided keys) so if they are missing
something has gone pretty wrong in the library, I think it's worth at
least a log message and more than likely an error return.

I would have returned an error, but the fuction returns void. :-) That leaves us the options of:
1. Erroring and calling exit() in the middle of the function
2. Just doing a warning
3. Modifying the function to return success / failure

#3 seems like the it shouldn't be too very difficult. If that sounds good to you I'll re-post this patch with only the bits you've acked, then post another patch with #3.

-George

> 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
> @@ -2201,14 +2201,14 @@ static void libxl__device_disk_from_xs_b
> disk->removable = 0;
>
> tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/mode", be_path));
> - if (!strcmp(tmp, "w"))
> + if (tmp && !strcmp(tmp, "w"))
> disk->readwrite = 1;
> else
> disk->readwrite = 0;
>
> tmp = libxl__xs_read(gc, XBT_NULL,
> libxl__sprintf(gc, "%s/device-type", be_path));
> - disk->is_cdrom = !strcmp(tmp, "cdrom");
> + disk->is_cdrom = tmp && !strcmp(tmp, "cdrom");
>
> disk->format = LIBXL_DISK_FORMAT_UNKNOWN;
> }
> @@ -2353,6 +2353,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
>
> libxl__device device;
> const char * path;
> + char * tmp;
>
> flexarray_t *insert = NULL;
>
> @@ -2383,8 +2384,11 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
> disks = libxl_device_disk_list(ctx, domid, &num);
> for (i = 0; i < num; i++) {
> if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
> - /* found */
> + {
> + /* Found. Set backend type appropriately. */
> + disk->backend=disks[i].backend;
> break;
> + }
> }
> if (i == num) {
> LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual device not found");
> @@ -2410,6 +2414,17 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
>
> path = libxl__device_backend_path(gc, &device);
>
> + /* Sanity check: make sure the backend exists before writing here */
> + tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/frontend", path));
> + if (!tmp)
> + {
> + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Internal error: %s does not exist",
> + libxl__sprintf(gc, "%s/frontend", path));
> + rc = ERROR_FAIL;
> + goto out;
> + }
> +
> +
> insert = flexarray_make(gc, 4, 1);
>
> flexarray_append_pair(insert, "type",
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] libxl: Fix bug in libxl_cdrom_insert, make more robust against bad xenstore data