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

[Bamvor Jian Zhang]

 >>>Jim Fehlig wrote: 
> 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. 
understand. the main difference between xl and xm format is disk and network.
and it worth to do it.
>  
> 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. 
the following patch works for me.

regards

bamvor

>  
> 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