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

[Jim Fehlig]

Bamvor Jian Zhang<bjzhang at suse.com> 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 at suse.com>
> ---
>  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 at suse.com>
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 at suse.com>
---
 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