Re: [libvirt] [PATCH] libxl: fix domxml-to-native wrong output for qcow2 format

>>Jim Fehlig wrote: Bamvor Jian Zhang&lt;bjzhang(a)suse.com&gt; wrote: > e.g. for these following disk configuration in libvirt. > <disk type='file' device='disk'> For Xen, using <driver name='qemu' .../> (which means qdisk) is only supported by the libxl driver. The old xm/xend stack does not support qdisk. > <driver name='qemu' type='qcow2'/> > <source file='/var/lib/xen/images/001/disk0.qcow2'/> > <target dev='hdc'/> > > without this patch, it will be > "qemu:/var/lib/xen/images/001/disk0.qcow2,hdc,w" Yeah, that won't work with xend or libxl :) . > but it should be(if with this patch) > "qcow2:/var/lib/xen/images/001/disk0.qcow2,hdc,w" > > Signed-off-by: Bamvor Jian Zhang <bjzhang(a)suse.com&gt; > --- > src/xenxs/xen_xm.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c > index b2db97d..29835b4 100644 > --- a/src/xenxs/xen_xm.c > +++ b/src/xenxs/xen_xm.c > @@ -1209,7 +1209,10 @@ xenFormatXMDisk(virConfValuePtr list, > type = "aio"; > else > type = virStorageFileFormatTypeToString(format); > - virBufferAsprintf(&buf, "%s:", driver); > + if (!STREQ(type, "qcow2")) > + virBufferAsprintf(&buf, "%s:", driver); > + else > + virBufferAsprintf(&buf, "%s:", type); But this only handles the specific case of <driver name='qemu' type='qcow2'/>. With <driver name='qemu' type='raw'/>, I get "qemu:/path/to/disk,hdc,w" Unfortunately, this is a case where we are trying to use the xm config parser to parse something that is xl-specific. With <driver name='qemu'.../>, the corresponding xl disk config would look something like disk = ['backendtype=qdisk,/var/lib/xen/images/001/disk0.qcow2,hdc,w'] With the type also specified, e.g. <driver name='qemu' type='qcow2'/>, the xl disk config would be disk = ['backendtype=qdisk,format=qcow2,/var/lib/xen/images/001/disk0.qcow2,hdc,w'] As I see it, the options are 1. start working on a xen-xl parser 2. map 'qemu' to 'tap' We'll eventually need 1 anyhow. I haven't looked to see how much work that would be. Certainly a lot of the xm parsing code could be used since xm config is a subset of xl config.

Option 2 is really a no-op in the libxl stack, where qdisk is used in place of tap anyhow. One could argue that mapping qemu to tap isn't too insane in the xm/xend stack either, since much of the blktap userspace code was originally based on qemu. The below patch works for me. Can you give it a try? I'd like to hear wha others think about taking the easy way out with option 2.

Regards, Jim From 92c476cbe47009c43ebb4a3076a17347c8eea238 Mon Sep 17 00:00:00 2001 From: Jim Fehlig <jfehlig(a)suse.com&gt; Date: Thu, 12 Jun 2014 15:28:48 -0600 Subject: [PATCH] libxl: fix domxml-to-native with qemu disk driver Disk config containing <driver name='qemu'.../> is converted to the native config containg "qemu:/path/to/disk", which is not understood by xm or xl. This patch maps 'qemu' to 'tap'. In xl, tap is mapped to the qemu backend (aka qdisk), making this change essentially a no-op. In the xm stack, one could argue that mapping qemu to tap isn't too insane, since much of the blktap userspace code was originally based on qemu. While at it, noticed that 'driver' wasn't being honored in xenFormatXMDisk, so change the logic a bit to honor a user-specified driver. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- src/xenxs/xen_xm.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index b2db97d..ebd525b 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1201,17 +1201,28 @@ xenFormatXMDisk(virConfValuePtr list, int format = virDomainDiskGetFormat(disk); const char *driver = virDomainDiskGetDriver(disk); - if (src) { - if (format) { - const char *type; + /* + * In pre-libxl Xen, 'tap' provided the qemu driver functionality. + * In libxl Xen, qemu (aka qdisk) is in fact used. For backwards + * compatibility, tap == qdisk in libxl Xen, so it is safe to use + * 'tap' in libxl tool. + */ + if (STREQ(driver, "qemu")) + driver = "tap"; - if (format == VIR_STORAGE_FILE_RAW) - type = "aio"; - else - type = virStorageFileFormatTypeToString(format); + if (src) { + if (driver) { virBufferAsprintf(&buf, "%s:", driver); - if (STREQ(driver, "tap")) - virBufferAsprintf(&buf, "%s:", type); + if (format) { + const char *type; + + if (format == VIR_STORAGE_FILE_RAW) + type = "aio"; + else + type = virStorageFileFormatTypeToString(format); + if (STREQ(driver, "tap")) + virBufferAsprintf(&buf, "%s:", type); + } } else { switch (virDomainDiskGetType(disk)) { case VIR_STORAGE_TYPE_FILE: -- 1.8.4.5