[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH V10 1/3] libxl: Add support for Virtio disk configuration
On Mon, Jun 13, 2022 at 09:05:20PM +0300, Oleksandr Tyshchenko wrote: > diff --git a/tools/libs/light/libxl_disk.c b/tools/libs/light/libxl_disk.c > index a5ca778..673b0d6 100644 > --- a/tools/libs/light/libxl_disk.c > +++ b/tools/libs/light/libxl_disk.c > @@ -575,6 +660,41 @@ cleanup: > return rc; > } > > +static int libxl__device_disk_get_path(libxl__gc *gc, uint32_t domid, > + char **path) > +{ > + const char *xen_dir, *virtio_dir; > + char *xen_path, *virtio_path; > + int rc; > + > + /* default path */ > + xen_path = GCSPRINTF("%s/device/%s", > + libxl__xs_libxl_path(gc, domid), > + > libxl__device_kind_to_string(LIBXL__DEVICE_KIND_VBD)); > + > + rc = libxl__xs_read_checked(gc, XBT_NULL, xen_path, &xen_dir); > + if (rc) > + return rc; > + > + virtio_path = GCSPRINTF("%s/device/%s", > + libxl__xs_libxl_path(gc, domid), > + > libxl__device_kind_to_string(LIBXL__DEVICE_KIND_VIRTIO_DISK)); > + > + rc = libxl__xs_read_checked(gc, XBT_NULL, virtio_path, &virtio_dir); > + if (rc) > + return rc; > + > + if (xen_dir && virtio_dir) { > + LOGD(ERROR, domid, "Invalid configuration, both xen and virtio paths > are present"); > + return ERROR_INVAL; > + } else if (virtio_dir) > + *path = virtio_path; > + else > + *path = xen_path; Small coding style issue, could you use blocks {} on all part of the if...else, since you are using them on one of the block? This is described in tools/libs/light/CODING_STYLE (5. Block structure). > diff --git a/tools/xl/xl_block.c b/tools/xl/xl_block.c > index 70eed43..f2b0ff5 100644 > --- a/tools/xl/xl_block.c > +++ b/tools/xl/xl_block.c > @@ -50,6 +50,11 @@ int main_blockattach(int argc, char **argv) > return 0; > } > > + if (disk.specification != LIBXL_DISK_SPECIFICATION_XEN) { > + fprintf(stderr, "block-attach is only supported for specification > xen\n"); This check prevents a previously working `block-attach` command line from working. # xl -Tvvv block-attach 0 /dev/vg/guest_disk,raw,hda block-attach is only supported for specification xen At least, that works by adding ",specification=xen", but it should work without it as "xen" is the default (from the man page). Maybe the check is done too soon, or maybe a better place to do it would be in libxl. libxl__device_disk_setdefault() is called much later, while executing libxl_device_disk_add(), so `xl` can't use the default been done there to "disk.specification". `xl block-attach` calls libxl_device_disk_add() which I think is only called for hotplug of disk. If I recall correctly, libxl__add_disks() is called instead at guest creation. So maybe it is possible to do something in libxl_device_disk_add(), but that a function defined by a macro, and the macro is using the same libxl__device_disk_add() that libxl_device_disk_add(). On the other hand, there is a "hotplug" parameter to libxl__device_disk_setdefault(), maybe that could be use? Cheers, -- Anthony PERARD
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |