Kamala Narasimhan
2011-Feb-07 21:22 UTC
[xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts
Attached are the changes made to xl disk related interface per earlier discussion. Please let me know if there are further comments/issues to fix. Signed-off-by: Kamala Narasimhan <kamala.narasimhan@citrix.com> Kamala _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Feb-08 14:38 UTC
Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts
On Mon, 2011-02-07 at 21:22 +0000, Kamala Narasimhan wrote:> Attached are the changes made to xl disk related interface per earlier discussion. Please let me know if there are further comments/issues to fix.Please can you include a full/standalone commit message describing the change and the reasons for it etc, The reference to "earlier discussion" is likely to be lost with time. Did we decide to leave DISK_BACKEND_DEFAULT (i.e. libxl chooses based on the image type, host''s backend support etc) for 4.2? I don''t mind if we did but does that make "block-dev-type" as described in patch 1/5 non-optional? (and therefore not really deprecated) Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Feb-08 14:41 UTC
Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts
On Tue, 2011-02-08 at 14:38 +0000, Ian Campbell wrote:> On Mon, 2011-02-07 at 21:22 +0000, Kamala Narasimhan wrote: > > Attached are the changes made to xl disk related interface per earlier discussion. Please let me know if there are further comments/issues to fix. > > Please can you include a full/standalone commit message describing the > change and the reasons for it etc, The reference to "earlier discussion" > is likely to be lost with time. > > Did we decide to leave DISK_BACKEND_DEFAULT (i.e. libxl chooses based on > the image type, host''s backend support etc) for 4.2? I don''t mind if we > did but does that make "block-dev-type" as described in patch 1/5 > non-optional? (and therefore not really deprecated)Maybe this is handled in xl by patch 3/5? (I should really apply the patch and read the result instead of trying to decode the meaning from the patch form) Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Feb-08 15:42 UTC
Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts
Kamala Narasimhan writes ("[xen-devel][PATCH 2/5] Xl interface change plus
changes to code it impacts"):> Attached are the changes made to xl disk related interface per earlier
discussion. Please let me know if there are further comments/issues to fix.
Thanks. I have some comments of my own:
> +char *libxl__device_disk_string_of_backend(libxl_disk_backend backend)
...> + switch (backend) {
> + case DISK_BACKEND_QEMU: return "qdisk";
> + case DISK_BACKEND_TAPDISK2: return "tap";
> + case DISK_BACKEND_BLKBACK: return "phy";
Perhaps the backend type number constants should be _QDISK, _TAP,
_PHY ? I think a function like _string_of should be a simple mapping
to return the string version of the same name, not also change the
name.
> - if (libxl__blktap_enabled(&gc))
> + if ( libxl__blktap_enabled(&gc) &&
> + disk->format != DISK_BACKEND_QEMU )
Don''t add whitespace inside the if''s ( ). (You have done this
in
several places. I know that libxl isn''t entirely consistent but
we have a defined coding style shouldn''t be making the code less
consistent.)
> diff -r e4406b9fb064 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c Mon Feb 07 15:04:32 2011 +0000
> +++ b/tools/libxl/xl_cmdimpl.c Mon Feb 07 11:28:10 2011 -0500
> @@ -361,9 +361,9 @@ static void printf_info(int domid,
> printf("\t\t(tap\n");
> printf("\t\t\t(backend_domid %d)\n",
d_config->disks[i].backend_domid);
> printf("\t\t\t(domid %d)\n",
d_config->disks[i].domid);
> - printf("\t\t\t(physpath %s)\n",
d_config->disks[i].physpath);
> - printf("\t\t\t(phystype %d)\n",
d_config->disks[i].phystype);
> - printf("\t\t\t(virtpath %s)\n",
d_config->disks[i].virtpath);
> + printf("\t\t\t(pdev_path %s)\n",
d_config->disks[i].pdev_path);
> + printf("\t\t\t(backend %d)\n",
d_config->disks[i].backend);
> + printf("\t\t\t(vdev %s)\n", d_config->disks[i].vdev);
This part of the code is providing information which is intended to be
parsed by callers which were written to cope with the output from xm.
For backward compatibility, the previously used names and values
should be output with the previously used semantics; it is OK to add
new ones too with more sane semantics.
I think it''s also acceptable to be a bit approximate with the
emulation, but simply removing the old names is not correct.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Feb-08 16:42 UTC
Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts
> diff -r e4406b9fb064 tools/libxl/libxl.c > --- a/tools/libxl/libxl.c Mon Feb 07 15:04:32 2011 +0000 > +++ b/tools/libxl/libxl.c Mon Feb 07 11:28:10 2011 -0500 > @@ -588,7 +588,7 @@ int libxl_wait_for_disk_ejects(libxl_ctx > for (i = 0; i < num_disks; i++) { > if (asprintf(&(waiter[i].path), "%s/device/vbd/%d/eject", > libxl__xs_get_dompath(&gc, domid), > - libxl__device_disk_dev_number(disks[i].virtpath)) < 0) > + libxl__device_disk_dev_number(disks[i].vdev)) < 0) > goto out; > if (asprintf(&(waiter[i].token), "%d", LIBXL_EVENT_DISK_EJECT) < 0) > goto out; > @@ -668,10 +668,10 @@ int libxl_event_get_disk_eject_info(libx > > disk->backend_domid = 0; > disk->domid = domid; > - disk->physpath = strdup(""); > - disk->phystype = PHYSTYPE_EMPTY; > + disk->pdev_path = strdup(""); > + disk->format = DISK_FORMAT_EMPTY; > /* this value is returned to the user: do not free right away */ > - disk->virtpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", backend)); > + disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", backend)); > disk->unpluggable = 1; > disk->readwrite = 0; > disk->is_cdrom = 1; > @@ -863,18 +863,19 @@ int libxl_vncviewer_exec(libxl_ctx *ctx, > > /******************************************************************************/ > > -static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, libxl_disk_phystype disk_type) > +static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, > + libxl_disk_backend backend_type, libxl_disk_format format) > { > struct stat stat_buf; > > - if ( (file_name[0] == ''\0'') && (disk_type == PHYSTYPE_EMPTY) ) > + if ( (file_name[0] == ''\0'') && (format == DISK_FORMAT_EMPTY) ) > return 0; > > if ( stat(file_name, &stat_buf) != 0 ) { > LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s", file_name); > return ERROR_INVAL; > } > - if ( disk_type == PHYSTYPE_PHY ) { > + if ( backend_type == DISK_BACKEND_BLKBACK ) { > if ( !(S_ISBLK(stat_buf.st_mode)) ) { > LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s is not a block device!\n", > file_name); > @@ -898,7 +899,8 @@ int libxl_device_disk_add(libxl_ctx *ctx > libxl__device device; > int major, minor, rc; > > - rc = validate_virtual_disk(ctx, disk->physpath, disk->phystype); > + rc = validate_virtual_disk(ctx, disk->pdev_path, disk->backend, > + disk->format); > if (rc) > return rc; > > @@ -913,11 +915,11 @@ int libxl_device_disk_add(libxl_ctx *ctx > goto out_free; > } > > - backend_type = libxl__device_disk_backend_type_of_phystype(disk->phystype); > - devid = libxl__device_disk_dev_number(disk->virtpath); > + backend_type = libxl__device_disk_string_of_backend(disk->backend); > + devid = libxl__device_disk_dev_number(disk->vdev); > if (devid==-1) { > LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported" > - " virtual disk identifier %s", disk->virtpath); > + " virtual disk identifier %s", disk->vdev); > rc = ERROR_INVAL; > goto out_free; > } > @@ -928,37 +930,33 @@ int libxl_device_disk_add(libxl_ctx *ctx > device.domid = disk->domid; > device.kind = DEVICE_VBD; > > - switch (disk->phystype) { > - case PHYSTYPE_PHY: { > - > - libxl__device_physdisk_major_minor(disk->physpath, &major, &minor); > + switch ( disk->backend ) { > + case DISK_BACKEND_BLKBACK: { > + libxl__device_physdisk_major_minor(disk->pdev_path, &major, &minor); > flexarray_append(back, "physical-device"); > flexarray_append(back, libxl__sprintf(&gc, "%x:%x", major, minor)); > > flexarray_append(back, "params"); > - flexarray_append(back, disk->physpath); > + flexarray_append(back, disk->pdev_path); > > device.backend_kind = DEVICE_VBD; > break; > } > - case PHYSTYPE_EMPTY: > - break; > - case PHYSTYPE_FILE: > - /* let''s pretend is tap:aio for the moment */ > - disk->phystype = PHYSTYPE_AIO; > - case PHYSTYPE_AIO: > - case PHYSTYPE_QCOW: > - case PHYSTYPE_QCOW2: > - case PHYSTYPE_VHD: > + case DISK_BACKEND_TAPDISK2: > + case DISK_BACKEND_QEMU: { > + if ( disk->format == DISK_FORMAT_EMPTY ) > + break; > if (libxl__blktap_enabled(&gc)) { > const char *dev = libxl__blktap_devpath(&gc, > - disk->physpath, disk->phystype); > + disk->pdev_path, disk->backend); > if (!dev) { > rc = ERROR_FAIL; > goto out_free; > } > flexarray_append(back, "tapdisk-params"); > - flexarray_append(back, libxl__sprintf(&gc, "%s:%s", libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath)); > + flexarray_append(back, libxl__sprintf(&gc, "%s:%s", > + libxl__device_disk_string_of_format(disk->format), > + disk->pdev_path)); > flexarray_append(back, "params"); > flexarray_append(back, libxl__strdup(&gc, dev)); > backend_type = "phy"; > @@ -971,16 +969,17 @@ int libxl_device_disk_add(libxl_ctx *ctx > } > flexarray_append(back, "params"); > flexarray_append(back, libxl__sprintf(&gc, "%s:%s", > - libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath)); > + libxl__device_disk_string_of_format(disk->format), disk->pdev_path)); > > - if (libxl__blktap_enabled(&gc)) > + if ( libxl__blktap_enabled(&gc) && > + disk->format != DISK_BACKEND_QEMU ) > device.backend_kind = DEVICE_TAP; > else > device.backend_kind = DEVICE_QDISK; > break; > - > + } > default: > - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", disk->phystype); > + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend); > rc = ERROR_INVAL; > goto out_free; > } > @@ -996,7 +995,7 @@ int libxl_device_disk_add(libxl_ctx *ctx > flexarray_append(back, "state"); > flexarray_append(back, libxl__sprintf(&gc, "%d", 1)); > flexarray_append(back, "dev"); > - flexarray_append(back, disk->virtpath); > + flexarray_append(back, disk->vdev); > flexarray_append(back, "type"); > flexarray_append(back, backend_type); > flexarray_append(back, "mode"); > @@ -1036,11 +1035,11 @@ int libxl_device_disk_del(libxl_ctx *ctx > libxl__device device; > int devid; > > - devid = libxl__device_disk_dev_number(disk->virtpath); > + devid = libxl__device_disk_dev_number(disk->vdev); > device.backend_domid = disk->backend_domid; > device.backend_devid = devid; > device.backend_kind = > - (disk->phystype == PHYSTYPE_PHY) ? DEVICE_VBD : DEVICE_TAP; > + (disk->backend == DISK_BACKEND_BLKBACK) ? DEVICE_VBD : DEVICE_TAP; > device.domid = disk->domid; > device.devid = devid; > device.kind = DEVICE_VBD; > @@ -1052,36 +1051,44 @@ char * libxl_device_disk_local_attach(li > libxl__gc gc = LIBXL_INIT_GC(ctx); > const char *dev = NULL; > char *ret = NULL; > - int phystype = disk->phystype; > - switch (phystype) { > - case PHYSTYPE_PHY: { > - fprintf(stderr, "attaching PHY disk %s to domain 0\n", disk->physpath); > - dev = disk->physpath; > + > + switch (disk->backend) { > + case DISK_BACKEND_BLKBACK: { > + fprintf(stderr, "attaching PHY disk %s to domain 0\n", disk->pdev_path); > + dev = disk->pdev_path; > break; > } > - case PHYSTYPE_FILE: > - /* let''s pretend is tap:aio for the moment */ > - phystype = PHYSTYPE_AIO; > - case PHYSTYPE_AIO: > - if (!libxl__blktap_enabled(&gc)) { > - dev = disk->physpath; > + case DISK_BACKEND_TAPDISK2: { > + if ( disk->format == DISK_FORMAT_VHD ) > + { > + if (libxl__blktap_enabled(&gc)) > + dev = libxl__blktap_devpath(&gc, disk->pdev_path, disk->backend); > + else > + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required to open a vhd disk\n"); > + break; > + } else if ( disk->format == DISK_FORMAT_QCOW || > + disk->format == DISK_FORMAT_QCOW2 ) { > + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image\n"); > + break; > + } else { > + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend " > + "type: %d\n", disk->backend); > break; > } > - case PHYSTYPE_VHD: > - if (libxl__blktap_enabled(&gc)) > - dev = libxl__blktap_devpath(&gc, disk->physpath, phystype); > - else > - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required to open a vhd disk\n"); > + } > + case DISK_BACKEND_QEMU: { > + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qdisk " > + "image\n"); > break; > - case PHYSTYPE_QCOW: > - case PHYSTYPE_QCOW2: > - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image\n"); > + } > + case DISK_BACKEND_UNKNOWN: > + default: { > + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend " > + "type: %d\n", disk->backend); > break; > + } > + } > > - default: > - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", phystype); > - break; > - } > if (dev != NULL) > ret = strdup(dev); > libxl__free_all(&gc); > @@ -1677,13 +1684,15 @@ static unsigned int libxl_append_disk_li > pdisk->domid = domid; > physpath_tmp = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/params", be_path, *dir), &len); > if (physpath_tmp && strchr(physpath_tmp, '':'')) { > - pdisk->physpath = strdup(strchr(physpath_tmp, '':'') + 1); > + pdisk->pdev_path = strdup(strchr(physpath_tmp, '':'') + 1); > free(physpath_tmp); > } else { > - pdisk->physpath = physpath_tmp; > + pdisk->pdev_path = physpath_tmp; > } > - libxl_string_to_phystype(ctx, libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), &(pdisk->phystype)); > - pdisk->virtpath = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len); > + libxl_string_to_backend(ctx, libxl__xs_read(&gc, XBT_NULL, > + libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), > + &(pdisk->backend)); > + pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len); > pdisk->unpluggable = atoi(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/removable", be_path, *dir))); > if (!strcmp(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/mode", be_path, *dir)), "w")) > pdisk->readwrite = 1; > @@ -1718,7 +1727,7 @@ int libxl_device_disk_getinfo(libxl_ctx > char *val; > > dompath = libxl__xs_get_dompath(&gc, domid); > - diskinfo->devid = libxl__device_disk_dev_number(disk->virtpath); > + diskinfo->devid = libxl__device_disk_dev_number(disk->vdev); > > /* tap devices entries in xenstore are written as vbd devices. */ > diskpath = libxl__sprintf(&gc, "%s/device/vbd/%d", dompath, diskinfo->devid); > @@ -1752,13 +1761,13 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u > libxl_device_disk *disks; > int ret = ERROR_FAIL; > > - if (!disk->physpath) { > - disk->physpath = strdup(""); > - disk->phystype = PHYSTYPE_EMPTY; > + if (!disk->pdev_path) { > + disk->pdev_path = strdup(""); > + disk->format = DISK_FORMAT_EMPTY; > } > disks = libxl_device_disk_list(ctx, domid, &num); > for (i = 0; i < num; i++) { > - if (disks[i].is_cdrom && !strcmp(disk->virtpath, disks[i].virtpath)) > + if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev)) > /* found */ > break; > } > diff -r e4406b9fb064 tools/libxl/libxl.h > --- a/tools/libxl/libxl.h Mon Feb 07 15:04:32 2011 +0000 > +++ b/tools/libxl/libxl.h Mon Feb 07 11:28:10 2011 -0500 > @@ -172,14 +172,20 @@ typedef enum { > } libxl_console_consback; > > typedef enum { > - PHYSTYPE_QCOW = 1, > - PHYSTYPE_QCOW2, > - PHYSTYPE_VHD, > - PHYSTYPE_AIO, > - PHYSTYPE_FILE, > - PHYSTYPE_PHY, > - PHYSTYPE_EMPTY, > -} libxl_disk_phystype; > + DISK_FORMAT_UNKNOWN = 0, > + DISK_FORMAT_QCOW, > + DISK_FORMAT_QCOW2, > + DISK_FORMAT_VHD, > + DISK_FORMAT_RAW, > + DISK_FORMAT_EMPTY, > +} libxl_disk_format; > + > +typedef enum { > + DISK_BACKEND_UNKNOWN = 0, > + DISK_BACKEND_BLKBACK, > + DISK_BACKEND_TAPDISK2, > + DISK_BACKEND_QEMU, > +} libxl_disk_backend; > > typedef enum { > NICTYPE_IOEMU = 1, > diff -r e4406b9fb064 tools/libxl/libxl.idl > --- a/tools/libxl/libxl.idl Mon Feb 07 15:04:32 2011 +0000 > +++ b/tools/libxl/libxl.idl Mon Feb 07 11:28:10 2011 -0500 > @@ -11,7 +11,8 @@ libxl_qemu_machine_type = Number("qemu_m > libxl_qemu_machine_type = Number("qemu_machine_type", namespace="libxl_") > libxl_console_consback = Number("console_consback", namespace="libxl_") > libxl_console_constype = Number("console_constype", namespace="libxl_") > -libxl_disk_phystype = Number("disk_phystype", namespace="libxl_") > +libxl_disk_format = Number("disk_format", namespace="libxl_") > +libxl_disk_backend = Number("disk_backend", namespace="libxl_") > libxl_nic_type = Number("nic_type", namespace="libxl_") > libxl_cpuid_policy_list = Builtin("cpuid_policy_list", destructor_fn="libxl_cpuid_destroy", passby=PASS_BY_REFERENCE) > > @@ -203,9 +204,10 @@ libxl_device_disk = Struct("device_disk" > libxl_device_disk = Struct("device_disk", [ > ("backend_domid", uint32), > ("domid", domid), > - ("physpath", string), > - ("phystype", libxl_disk_phystype), > - ("virtpath", string), > + ("pdev_path", string), > + ("vdev", string), > + ("backend", libxl_disk_backend), > + ("format", libxl_disk_format), > ("unpluggable", integer), > ("readwrite", integer), > ("is_cdrom", integer), > diff -r e4406b9fb064 tools/libxl/libxl_blktap2.c > --- a/tools/libxl/libxl_blktap2.c Mon Feb 07 15:04:32 2011 +0000 > +++ b/tools/libxl/libxl_blktap2.c Mon Feb 07 11:28:10 2011 -0500 > @@ -26,13 +26,13 @@ int libxl__blktap_enabled(libxl__gc *gc) > > const char *libxl__blktap_devpath(libxl__gc *gc, > const char *disk, > - libxl_disk_phystype phystype) > + libxl_disk_backend backend) > { > const char *type; > char *params, *devname = NULL; > int minor, err; > > - type = libxl__device_disk_string_of_phystype(phystype); > + type = libxl__device_disk_string_of_backend(backend);This should be libxl__device_disk_string_of_format and a libxl_disk_format should be passed as an argument to libxl__blktap_devpath instead of libxl_disk_backend.> minor = tap_ctl_find_minor(type, disk); > if (minor >= 0) { > devname = libxl__sprintf(gc, "/dev/xen/blktap-2/tapdev%d", minor); > diff -r e4406b9fb064 tools/libxl/libxl_device.c > --- a/tools/libxl/libxl_device.c Mon Feb 07 15:04:32 2011 +0000 > +++ b/tools/libxl/libxl_device.c Mon Feb 07 11:28:10 2011 -0500 > @@ -121,31 +121,24 @@ out: > return rc; > } > > -char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype) > +char *libxl__device_disk_string_of_format(libxl_disk_format format) > { > - switch (phystype) { > - case PHYSTYPE_QCOW: return "qcow"; > - case PHYSTYPE_QCOW2: return "qcow2"; > - case PHYSTYPE_VHD: return "vhd"; > - case PHYSTYPE_AIO: return "aio"; > - case PHYSTYPE_FILE: return "file"; > - case PHYSTYPE_PHY: return "phy"; > - case PHYSTYPE_EMPTY: return "file"; > - default: return NULL; > + switch (format) { > + case DISK_FORMAT_QCOW: return "qcow"; > + case DISK_FORMAT_QCOW2: return "qcow2"; > + case DISK_FORMAT_VHD: return "vhd"; > + case DISK_FORMAT_RAW: > + case DISK_FORMAT_EMPTY: return "file";This should be return "aio".> + default: return NULL; > } > } > > -char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype) > +char *libxl__device_disk_string_of_backend(libxl_disk_backend backend) > { > - switch (phystype) { > - case PHYSTYPE_QCOW: return "tap"; > - case PHYSTYPE_QCOW2: return "tap"; > - case PHYSTYPE_VHD: return "tap"; > - case PHYSTYPE_AIO: return "tap"; > - /* let''s pretend file is tap:aio */ > - case PHYSTYPE_FILE: return "tap"; > - case PHYSTYPE_EMPTY: return "tap"; > - case PHYSTYPE_PHY: return "phy"; > + switch (backend) { > + case DISK_BACKEND_QEMU: return "qdisk"; > + case DISK_BACKEND_TAPDISK2: return "tap"; > + case DISK_BACKEND_BLKBACK: return "phy"; > default: return NULL; > } > } > diff -r e4406b9fb064 tools/libxl/libxl_dm.c > --- a/tools/libxl/libxl_dm.c Mon Feb 07 15:04:32 2011 +0000 > +++ b/tools/libxl/libxl_dm.c Mon Feb 07 11:28:10 2011 -0500 > @@ -316,10 +316,10 @@ static char ** libxl_build_device_model_ > for (i; i < nb; i++) { > if (disks[i].is_cdrom) { > flexarray_append(dm_args, "-cdrom"); > - flexarray_append(dm_args, libxl__strdup(gc, disks[i].physpath)); > + flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path)); > } else { > - flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].virtpath)); > - flexarray_append(dm_args, libxl__strdup(gc, disks[i].physpath)); > + flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].vdev)); > + flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path)); > } > libxl_device_disk_destroy(&disks[i]); > } > diff -r e4406b9fb064 tools/libxl/libxl_internal.h > --- a/tools/libxl/libxl_internal.h Mon Feb 07 15:04:32 2011 +0000 > +++ b/tools/libxl/libxl_internal.h Mon Feb 07 11:28:10 2011 -0500 > @@ -178,8 +178,8 @@ _hidden void libxl__userdata_destroyall( > _hidden void libxl__userdata_destroyall(libxl_ctx *ctx, uint32_t domid); > > /* from xl_device */ > -_hidden char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype); > -_hidden char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype); > +_hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend); > +_hidden char *libxl__device_disk_string_of_format(libxl_disk_format format); > > _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor); > _hidden int libxl__device_disk_dev_number(char *virtpath); > @@ -306,7 +306,7 @@ _hidden int libxl__blktap_enabled(libxl_ > */ > _hidden const char *libxl__blktap_devpath(libxl__gc *gc, > const char *disk, > - libxl_disk_phystype phystype); > + libxl_disk_backend backend); > > _hidden char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid); > > diff -r e4406b9fb064 tools/libxl/libxl_utils.c > --- a/tools/libxl/libxl_utils.c Mon Feb 07 15:04:32 2011 +0000 > +++ b/tools/libxl/libxl_utils.c Mon Feb 07 11:28:10 2011 -0500 > @@ -275,15 +275,15 @@ out: > return rc; > } > > -int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype) > +int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend) > { > char *p; > int rc = 0; > > if (!strcmp(s, "phy")) { > - *phystype = PHYSTYPE_PHY; > + *backend = DISK_BACKEND_BLKBACK; > } else if (!strcmp(s, "file")) { > - *phystype = PHYSTYPE_FILE; > + *backend = DISK_BACKEND_TAPDISK2; > } else if (!strcmp(s, "tap")) { > p = strchr(s, '':''); > if (!p) { > @@ -291,14 +291,12 @@ int libxl_string_to_phystype(libxl_ctx * > goto out; > } > p++; > - if (!strcmp(p, "aio")) { > - *phystype = PHYSTYPE_AIO; > - } else if (!strcmp(p, "vhd")) { > - *phystype = PHYSTYPE_VHD; > + if (!strcmp(p, "vhd")) { > + *backend = DISK_BACKEND_TAPDISK2; > } else if (!strcmp(p, "qcow")) { > - *phystype = PHYSTYPE_QCOW; > + *backend = DISK_BACKEND_QEMU; > } else if (!strcmp(p, "qcow2")) { > - *phystype = PHYSTYPE_QCOW2; > + *backend = DISK_BACKEND_QEMU; > } > } > out: > @@ -553,10 +551,10 @@ int libxl_devid_to_device_disk(libxl_ctx > disk->backend_domid = strtoul(val, NULL, 10); > disk->domid = domid; > be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend", diskpath)); > - disk->physpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path)); > + disk->pdev_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path)); > val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/type", be_path)); > - libxl_string_to_phystype(ctx, val, &(disk->phystype)); > - disk->virtpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path)); > + libxl_string_to_backend(ctx, val, &(disk->backend)); > + disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path)); > val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/removable", be_path)); > disk->unpluggable = !strcmp(val, "1"); > val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mode", be_path)); > diff -r e4406b9fb064 tools/libxl/libxl_utils.h > --- a/tools/libxl/libxl_utils.h Mon Feb 07 15:04:32 2011 +0000 > +++ b/tools/libxl/libxl_utils.h Mon Feb 07 11:28:10 2011 -0500 > @@ -29,7 +29,7 @@ int libxl_get_stubdom_id(libxl_ctx *ctx, > int libxl_get_stubdom_id(libxl_ctx *ctx, int guest_domid); > int libxl_is_stubdom(libxl_ctx *ctx, uint32_t domid, uint32_t *target_domid); > int libxl_create_logfile(libxl_ctx *ctx, char *name, char **full_name); > -int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype); > +int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend); > > int libxl_read_file_contents(libxl_ctx *ctx, const char *filename, > void **data_r, int *datalen_r); > diff -r e4406b9fb064 tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Mon Feb 07 15:04:32 2011 +0000 > +++ b/tools/libxl/xl_cmdimpl.c Mon Feb 07 11:28:10 2011 -0500 > @@ -361,9 +361,9 @@ static void printf_info(int domid, > printf("\t\t(tap\n"); > printf("\t\t\t(backend_domid %d)\n", d_config->disks[i].backend_domid); > printf("\t\t\t(domid %d)\n", d_config->disks[i].domid); > - printf("\t\t\t(physpath %s)\n", d_config->disks[i].physpath); > - printf("\t\t\t(phystype %d)\n", d_config->disks[i].phystype); > - printf("\t\t\t(virtpath %s)\n", d_config->disks[i].virtpath); > + printf("\t\t\t(pdev_path %s)\n", d_config->disks[i].pdev_path); > + printf("\t\t\t(backend %d)\n", d_config->disks[i].backend); > + printf("\t\t\t(vdev %s)\n", d_config->disks[i].vdev); > printf("\t\t\t(unpluggable %d)\n", d_config->disks[i].unpluggable); > printf("\t\t\t(readwrite %d)\n", d_config->disks[i].readwrite); > printf("\t\t\t(is_cdrom %d)\n", d_config->disks[i].is_cdrom); > @@ -460,10 +460,12 @@ static int parse_disk_config(libxl_devic > *p = ''\0''; > if ( !strcmp(tok, "phy") ) { > state = DSTATE_PHYSPATH; > - disk->phystype = PHYSTYPE_PHY; > + disk->format = DISK_FORMAT_RAW; > + disk->backend = DISK_BACKEND_BLKBACK; > }else if ( !strcmp(tok, "file") ) { > state = DSTATE_PHYSPATH; > - disk->phystype = PHYSTYPE_FILE; > + disk->format = DISK_FORMAT_RAW; > + disk->backend = DISK_BACKEND_TAPDISK2; > }else if ( !strcmp(tok, "tap") ) { > state = DSTATE_TAP; > }else{ > @@ -473,8 +475,8 @@ static int parse_disk_config(libxl_devic > tok = p + 1; > } else if (*p == '','') { > state = DSTATE_VIRTPATH; > - disk->phystype = PHYSTYPE_EMPTY; > - disk->physpath = strdup(""); > + disk->backend = DISK_FORMAT_EMPTY; > + disk->pdev_path = strdup(""); > tok = p + 1; > } > break; > @@ -482,13 +484,17 @@ static int parse_disk_config(libxl_devic > if ( *p == '':'' ) { > *p = ''\0''; > if ( !strcmp(tok, "aio") ) { > - disk->phystype = PHYSTYPE_AIO; > + disk->format = DISK_FORMAT_RAW; > + disk->backend = DISK_BACKEND_TAPDISK2; > }else if ( !strcmp(tok, "vhd") ) { > - disk->phystype = PHYSTYPE_VHD; > + disk->format = DISK_FORMAT_VHD; > + disk->backend = DISK_BACKEND_TAPDISK2; > }else if ( !strcmp(tok, "qcow") ) { > - disk->phystype = PHYSTYPE_QCOW; > + disk->format = DISK_FORMAT_QCOW; > + disk->backend = DISK_BACKEND_QEMU; > }else if ( !strcmp(tok, "qcow2") ) { > - disk->phystype = PHYSTYPE_QCOW2; > + disk->format = DISK_FORMAT_QCOW2; > + disk->backend = DISK_BACKEND_QEMU; > }else { > fprintf(stderr, "Unknown tapdisk type: %s\n", tok); > return 0; > @@ -503,7 +509,7 @@ static int parse_disk_config(libxl_devic > int ioemu_len; > > *p = ''\0''; > - disk->physpath = (*tok) ? strdup(tok) : NULL; > + disk->pdev_path = (*tok) ? strdup(tok) : NULL; > tok = p + 1; > > /* hack for ioemu disk spec */ > @@ -532,7 +538,7 @@ static int parse_disk_config(libxl_devic > if ( tok == p ) > goto out; > *p = ''\0''; > - disk->virtpath = (*tok) ? strdup(tok) : NULL; > + disk->vdev = (*tok) ? strdup(tok) : NULL; > tok = p + 1; > } > break; > @@ -1838,25 +1844,25 @@ static void cd_insert(const char *dom, c > p = strchr(phys, '':''); > if (!p) { > fprintf(stderr, "No type specified, "); > - disk.physpath = phys; > + disk.pdev_path = phys; > if (!strncmp(phys, "/dev", 4)) { > fprintf(stderr, "assuming phy:\n"); > - disk.phystype = PHYSTYPE_PHY; > + disk.backend = DISK_BACKEND_BLKBACK; > } else { > fprintf(stderr, "assuming file:\n"); > - disk.phystype = PHYSTYPE_FILE; > + disk.backend = DISK_BACKEND_TAPDISK2; > } > } else { > *p = ''\0''; > p++; > - disk.physpath = p; > - libxl_string_to_phystype(&ctx, phys, &disk.phystype); > - } > - } else { > - disk.physpath = strdup(""); > - disk.phystype = PHYSTYPE_EMPTY; > - } > - disk.virtpath = (char*)virtdev; > + disk.pdev_path = p; > + libxl_string_to_backend(&ctx, phys, &disk.backend); > + } > + } else { > + disk.pdev_path = strdup(""); > + disk.format = DISK_FORMAT_EMPTY; > + } > + disk.vdev = (char*)virtdev; > disk.unpluggable = 1; > disk.readwrite = 0; > disk.is_cdrom = 1; > @@ -4383,19 +4389,22 @@ int main_blockattach(int argc, char **ar > > tok = strtok(argv[optind+1], ":"); > if (!strcmp(tok, "phy")) { > - disk.phystype = PHYSTYPE_PHY; > + disk.backend = DISK_BACKEND_BLKBACK; > } else if (!strcmp(tok, "file")) { > - disk.phystype = PHYSTYPE_FILE; > + disk.backend = DISK_BACKEND_TAPDISK2; > } else if (!strcmp(tok, "tap")) { > tok = strtok(NULL, ":"); > if (!strcmp(tok, "aio")) { > - disk.phystype = PHYSTYPE_AIO; > + disk.backend = DISK_BACKEND_TAPDISK2; > } else if (!strcmp(tok, "vhd")) { > - disk.phystype = PHYSTYPE_VHD; > + disk.format = DISK_FORMAT_VHD; > + disk.backend = DISK_BACKEND_TAPDISK2; > } else if (!strcmp(tok, "qcow")) { > - disk.phystype = PHYSTYPE_QCOW; > + disk.format = DISK_FORMAT_QCOW; > + disk.backend = DISK_BACKEND_QEMU; > } else if (!strcmp(tok, "qcow2")) { > - disk.phystype = PHYSTYPE_QCOW2; > + disk.format = DISK_FORMAT_QCOW2; > + disk.backend = DISK_BACKEND_QEMU; > } else { > fprintf(stderr, "Error: `%s'' is not a valid disk image.\n", tok); > return 1; > @@ -4404,12 +4413,12 @@ int main_blockattach(int argc, char **ar > fprintf(stderr, "Error: `%s'' is not a valid block device.\n", tok); > return 1; > } > - disk.physpath = strtok(NULL, "\0"); > - if (!disk.physpath) { > + disk.pdev_path = strtok(NULL, "\0"); > + if (!disk.pdev_path) { > fprintf(stderr, "Error: missing path to disk image.\n"); > return 1; > } > - disk.virtpath = argv[optind+2]; > + disk.vdev = argv[optind+2]; > disk.unpluggable = 1; > disk.readwrite = ((argc-optind <= 3) || (argv[optind+3][0] == ''w'')); > > diff -r e4406b9fb064 tools/python/xen/lowlevel/xl/xl.c > --- a/tools/python/xen/lowlevel/xl/xl.c Mon Feb 07 15:04:32 2011 +0000 > +++ b/tools/python/xen/lowlevel/xl/xl.c Mon Feb 07 11:28:10 2011 -0500 > @@ -779,12 +779,17 @@ PyMODINIT_FUNC initxl(void) > _INT_CONST_LIBXL(m, CONSBACK_XENCONSOLED); > _INT_CONST_LIBXL(m, CONSBACK_IOEMU); > > - _INT_CONST(m, PHYSTYPE_QCOW); > - _INT_CONST(m, PHYSTYPE_QCOW2); > - _INT_CONST(m, PHYSTYPE_VHD); > - _INT_CONST(m, PHYSTYPE_AIO); > - _INT_CONST(m, PHYSTYPE_FILE); > - _INT_CONST(m, PHYSTYPE_PHY); > + _INT_CONST(m, DISK_FORMAT_UNKNOWN); > + _INT_CONST(m, DISK_FORMAT_QCOW); > + _INT_CONST(m, DISK_FORMAT_QCOW2); > + _INT_CONST(m, DISK_FORMAT_VHD); > + _INT_CONST(m, DISK_FORMAT_RAW); > + _INT_CONST(m, DISK_FORMAT_EMPTY); > + > + _INT_CONST(m, DISK_BACKEND_UNKNOWN); > + _INT_CONST(m, DISK_BACKEND_BLKBACK); > + _INT_CONST(m, DISK_BACKEND_TAPDISK2); > + _INT_CONST(m, DISK_BACKEND_QEMU); > > _INT_CONST(m, NICTYPE_IOEMU); > _INT_CONST(m, NICTYPE_VIF);_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kamala Narasimhan
2011-Feb-08 18:37 UTC
Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts
Ian Campbell wrote:> On Mon, 2011-02-07 at 21:22 +0000, Kamala Narasimhan wrote: >> Attached are the changes made to xl disk related interface per earlier discussion. Please let me know if there are further comments/issues to fix. > > Please can you include a full/standalone commit message describing the > change and the reasons for it etc, The reference to "earlier discussion" > is likely to be lost with time. >Sure, will do when I send out a revised patch that addresses review comments.> Did we decide to leave DISK_BACKEND_DEFAULT (i.e. libxl chooses based on > the image type, host''s backend support etc) for 4.2? I don''t mind if we > did but does that make "block-dev-type" as described in patch 1/5 > non-optional? (and therefore not really deprecated) >block-dev-type is still optional. Patch 5/5, the validation patch will take care of the case when that optional attribute is not provided but the patches submitted so far should have stop gap code that sets a fallback value. Kamala _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kamala Narasimhan
2011-Feb-08 18:42 UTC
Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts
>> Did we decide to leave DISK_BACKEND_DEFAULT (i.e. libxl chooses based on >> the image type, host''s backend support etc) for 4.2? I don''t mind if we >> did but does that make "block-dev-type" as described in patch 1/5 >> non-optional? (and therefore not really deprecated) > > Maybe this is handled in xl by patch 3/5? > > (I should really apply the patch and read the result instead of trying > to decode the meaning from the patch form) >Yes, a fallback value is set in 3/5 but patch 5/5 will remove that fallback and do appropriate validation to better handle this. Kamala _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kamala Narasimhan
2011-Feb-08 18:56 UTC
Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts
Ian Jackson wrote:> Kamala Narasimhan writes ("[xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts"): >> Attached are the changes made to xl disk related interface per earlier discussion. Please let me know if there are further comments/issues to fix. > > Thanks. I have some comments of my own: > >> +char *libxl__device_disk_string_of_backend(libxl_disk_backend backend) > ... >> + switch (backend) { >> + case DISK_BACKEND_QEMU: return "qdisk"; >> + case DISK_BACKEND_TAPDISK2: return "tap"; >> + case DISK_BACKEND_BLKBACK: return "phy"; > > Perhaps the backend type number constants should be _QDISK, _TAP, > _PHY ? I think a function like _string_of should be a simple mapping > to return the string version of the same name, not also change the > name. >I can make that change.>> - if (libxl__blktap_enabled(&gc)) >> + if ( libxl__blktap_enabled(&gc) && >> + disk->format != DISK_BACKEND_QEMU ) > > Don''t add whitespace inside the if''s ( ). (You have done this in > several places. I know that libxl isn''t entirely consistent but > we have a defined coding style shouldn''t be making the code less > consistent.) >Oh, boy! I have consistently done that all over the patches as I assumed that to be our convention. Will change that too.>> diff -r e4406b9fb064 tools/libxl/xl_cmdimpl.c >> --- a/tools/libxl/xl_cmdimpl.c Mon Feb 07 15:04:32 2011 +0000 >> +++ b/tools/libxl/xl_cmdimpl.c Mon Feb 07 11:28:10 2011 -0500 >> @@ -361,9 +361,9 @@ static void printf_info(int domid, >> printf("\t\t(tap\n"); >> printf("\t\t\t(backend_domid %d)\n", d_config->disks[i].backend_domid); >> printf("\t\t\t(domid %d)\n", d_config->disks[i].domid); >> - printf("\t\t\t(physpath %s)\n", d_config->disks[i].physpath); >> - printf("\t\t\t(phystype %d)\n", d_config->disks[i].phystype); >> - printf("\t\t\t(virtpath %s)\n", d_config->disks[i].virtpath); >> + printf("\t\t\t(pdev_path %s)\n", d_config->disks[i].pdev_path); >> + printf("\t\t\t(backend %d)\n", d_config->disks[i].backend); >> + printf("\t\t\t(vdev %s)\n", d_config->disks[i].vdev); > > This part of the code is providing information which is intended to be > parsed by callers which were written to cope with the output from xm. > For backward compatibility, the previously used names and values > should be output with the previously used semantics; it is OK to add > new ones too with more sane semantics. > > I think it''s also acceptable to be a bit approximate with the > emulation, but simply removing the old names is not correct. >I didn''t realize that. Will keep the old display names then. Kamala _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kamala Narasimhan
2011-Feb-08 19:04 UTC
Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts
>> diff -r e4406b9fb064 tools/libxl/libxl_device.c >> --- a/tools/libxl/libxl_device.c Mon Feb 07 15:04:32 2011 +0000 >> +++ b/tools/libxl/libxl_device.c Mon Feb 07 11:28:10 2011 -0500 >> @@ -121,31 +121,24 @@ out: >> return rc; >> } >> >> -char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype) >> +char *libxl__device_disk_string_of_format(libxl_disk_format format) >> { >> - switch (phystype) { >> - case PHYSTYPE_QCOW: return "qcow"; >> - case PHYSTYPE_QCOW2: return "qcow2"; >> - case PHYSTYPE_VHD: return "vhd"; >> - case PHYSTYPE_AIO: return "aio"; >> - case PHYSTYPE_FILE: return "file"; >> - case PHYSTYPE_PHY: return "phy"; >> - case PHYSTYPE_EMPTY: return "file"; >> - default: return NULL; >> + switch (format) { >> + case DISK_FORMAT_QCOW: return "qcow"; >> + case DISK_FORMAT_QCOW2: return "qcow2"; >> + case DISK_FORMAT_VHD: return "vhd"; >> + case DISK_FORMAT_RAW: >> + case DISK_FORMAT_EMPTY: return "file"; > > This should be return "aio". >When I tested, both "aio" and "file" worked and I went with "file". But if it is better to go with "aio", I will do that. Kamala _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Feb-08 19:09 UTC
Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts
On Tue, 8 Feb 2011, Kamala Narasimhan wrote:> >> diff -r e4406b9fb064 tools/libxl/libxl_device.c > >> --- a/tools/libxl/libxl_device.c Mon Feb 07 15:04:32 2011 +0000 > >> +++ b/tools/libxl/libxl_device.c Mon Feb 07 11:28:10 2011 -0500 > >> @@ -121,31 +121,24 @@ out: > >> return rc; > >> } > >> > >> -char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype) > >> +char *libxl__device_disk_string_of_format(libxl_disk_format format) > >> { > >> - switch (phystype) { > >> - case PHYSTYPE_QCOW: return "qcow"; > >> - case PHYSTYPE_QCOW2: return "qcow2"; > >> - case PHYSTYPE_VHD: return "vhd"; > >> - case PHYSTYPE_AIO: return "aio"; > >> - case PHYSTYPE_FILE: return "file"; > >> - case PHYSTYPE_PHY: return "phy"; > >> - case PHYSTYPE_EMPTY: return "file"; > >> - default: return NULL; > >> + switch (format) { > >> + case DISK_FORMAT_QCOW: return "qcow"; > >> + case DISK_FORMAT_QCOW2: return "qcow2"; > >> + case DISK_FORMAT_VHD: return "vhd"; > >> + case DISK_FORMAT_RAW: > >> + case DISK_FORMAT_EMPTY: return "file"; > > > > This should be return "aio". > > > When I tested, both "aio" and "file" worked and I went with "file". But if it > is better to go with "aio", I will do that.qemu doesn''t work with "file", it works with "aio" or "raw" ("raw" would be better but I don''t think tapdisk2 supports it). _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Feb-10 09:02 UTC
Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts
On Tue, 2011-02-08 at 18:42 +0000, Kamala Narasimhan wrote:> >> Did we decide to leave DISK_BACKEND_DEFAULT (i.e. libxl chooses based on > >> the image type, host''s backend support etc) for 4.2? I don''t mind if we > >> did but does that make "block-dev-type" as described in patch 1/5 > >> non-optional? (and therefore not really deprecated) > > > > Maybe this is handled in xl by patch 3/5? > > > > (I should really apply the patch and read the result instead of trying > > to decode the meaning from the patch form) > > > Yes, a fallback value is set in 3/5 but patch 5/5 will remove that fallback and > do appropriate validation to better handle this.Patch 3/5 is all on the xl side though, right? I was meaning a default "do the right thing" at the libxl interface level, which given time might become the only option... I didn''t see a patch 5/5 what is in it? Ian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kamala Narasimhan
2011-Feb-10 14:37 UTC
Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts
Ian Campbell wrote:> On Tue, 2011-02-08 at 18:42 +0000, Kamala Narasimhan wrote: >>>> Did we decide to leave DISK_BACKEND_DEFAULT (i.e. libxl chooses based on >>>> the image type, host''s backend support etc) for 4.2? I don''t mind if we >>>> did but does that make "block-dev-type" as described in patch 1/5 >>>> non-optional? (and therefore not really deprecated) >>> Maybe this is handled in xl by patch 3/5? >>> >>> (I should really apply the patch and read the result instead of trying >>> to decode the meaning from the patch form) >>> >> Yes, a fallback value is set in 3/5 but patch 5/5 will remove that fallback and >> do appropriate validation to better handle this. > > Patch 3/5 is all on the xl side though, right? I was meaning a default > "do the right thing" at the libxl interface level, which given time > might become the only option... > > I didn''t see a patch 5/5 what is in it? >Based on further discussion we decided to go with patch 1 & 2 and a couple of minor patches on top for 4.1 and apply the rest for 4.2. I will send patch 5 along with 3 & 4 post 4.1. Patch 5 is all validation code which will be pretty much in libxl and it will take care of the fallback, For now we do nothing more than what we already do IMO. Kamala _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel