[PATCH] libxl: prefer qdisk for <driver name='file'>

[PATCH] libxl: prefer qdisk for <driver name='file'>

[Jim Fehlig]

The libxl driver currently sets the disk backend to
LIBXL_DISK_BACKEND_TAP when <driver name='file'> is specified
in the <disk> config.  qdisk should be prefered with this
configuration, otherwise existing configuration such as the
following, which worked with the old Xen driver, will not work
with the libxl driver

  <disk type='file' device='cdrom'>
    <driver name='file'/>
    <source file='/path/to/some/iso'/>
    <target dev='hdc' bus='ide'/>
    <readonly/>
  </disk>

In addition, tap performs poorly compared to qdisk.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index fdbb522..4b6b5c0 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -796,7 +796,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
                 return -1;
             }
             x_disk->format = LIBXL_DISK_FORMAT_RAW;
-            x_disk->backend = LIBXL_DISK_BACKEND_TAP;
+            x_disk->backend = LIBXL_DISK_BACKEND_QDISK;
         } else if (STREQ(driver, "phy")) {
             if (format != VIR_STORAGE_FILE_NONE &&
                 format != VIR_STORAGE_FILE_RAW) {
-- 
1.8.4.5

Re: [PATCH] libxl: prefer qdisk for <driver name='file'>

[Stefano Stabellini]

On Fri, 20 Jun 2014, Jim Fehlig wrote:
> The libxl driver currently sets the disk backend to
> LIBXL_DISK_BACKEND_TAP when <driver name='file'> is specified
> in the <disk> config.  qdisk should be prefered with this
> configuration, otherwise existing configuration such as the
> following, which worked with the old Xen driver, will not work
> with the libxl driver
> 
>   <disk type='file' device='cdrom'>
>     <driver name='file'/>
>     <source file='/path/to/some/iso'/>
>     <target dev='hdc' bus='ide'/>
>     <readonly/>
>   </disk>
> 
> In addition, tap performs poorly compared to qdisk.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


>  src/libxl/libxl_conf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index fdbb522..4b6b5c0 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -796,7 +796,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
>                  return -1;
>              }
>              x_disk->format = LIBXL_DISK_FORMAT_RAW;
> -            x_disk->backend = LIBXL_DISK_BACKEND_TAP;
> +            x_disk->backend = LIBXL_DISK_BACKEND_QDISK;
>          } else if (STREQ(driver, "phy")) {
>              if (format != VIR_STORAGE_FILE_NONE &&
>                  format != VIR_STORAGE_FILE_RAW) {
> -- 
> 1.8.4.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

Re: [PATCH] libxl: prefer qdisk for <driver name='file'>

[Jim Fehlig]

Stefano Stabellini wrote:
> On Fri, 20 Jun 2014, Jim Fehlig wrote:
>   
>> The libxl driver currently sets the disk backend to
>> LIBXL_DISK_BACKEND_TAP when <driver name='file'> is specified
>> in the <disk> config.  qdisk should be prefered with this
>> configuration, otherwise existing configuration such as the
>> following, which worked with the old Xen driver, will not work
>> with the libxl driver
>>
>>   <disk type='file' device='cdrom'>
>>     <driver name='file'/>
>>     <source file='/path/to/some/iso'/>
>>     <target dev='hdc' bus='ide'/>
>>     <readonly/>
>>   </disk>
>>
>> In addition, tap performs poorly compared to qdisk.
>>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>>     
>
> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>   

Thanks Stefano.  This is a change that needed blessing from one of the
Xen tools devs IMO.  I've pushed the patch.

Regards,
Jim

Re: [PATCH] libxl: prefer qdisk for <driver name='file'>

[Ian Campbell]

On Fri, 2014-06-20 at 15:07 -0600, Jim Fehlig wrote:
> The libxl driver currently sets the disk backend to
> LIBXL_DISK_BACKEND_TAP when <driver name='file'> is specified
> in the <disk> config.  qdisk should be prefered with this
> configuration, otherwise existing configuration such as the
> following, which worked with the old Xen driver, will not work
> with the libxl driver

OOI why not let libxl pick the most appropriate backend for (most of)
these alternatives?

> 
>   <disk type='file' device='cdrom'>
>     <driver name='file'/>
>     <source file='/path/to/some/iso'/>
>     <target dev='hdc' bus='ide'/>
>     <readonly/>
>   </disk>
> 
> In addition, tap performs poorly compared to qdisk.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>  src/libxl/libxl_conf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index fdbb522..4b6b5c0 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -796,7 +796,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
>                  return -1;
>              }
>              x_disk->format = LIBXL_DISK_FORMAT_RAW;
> -            x_disk->backend = LIBXL_DISK_BACKEND_TAP;
> +            x_disk->backend = LIBXL_DISK_BACKEND_QDISK;
>          } else if (STREQ(driver, "phy")) {
>              if (format != VIR_STORAGE_FILE_NONE &&
>                  format != VIR_STORAGE_FILE_RAW) {

Re: [PATCH] libxl: prefer qdisk for <driver name='file'>

[Jim Fehlig]

Ian Campbell wrote:
> On Fri, 2014-06-20 at 15:07 -0600, Jim Fehlig wrote:
>   
>> The libxl driver currently sets the disk backend to
>> LIBXL_DISK_BACKEND_TAP when <driver name='file'> is specified
>> in the <disk> config.  qdisk should be prefered with this
>> configuration, otherwise existing configuration such as the
>> following, which worked with the old Xen driver, will not work
>> with the libxl driver
>>     
>
> OOI why not let libxl pick the most appropriate backend for (most of)
> these alternatives?
>   

I think the libvirt approach is to be a bit more explicit, instead of
relying on hypervisor defaults that might change over time.  I'm quite
sure this is the approach danpb has always advocated.

Regards,
Jim

Re: [PATCH] libxl: prefer qdisk for <driver name='file'>

[Ian Campbell]

On Tue, 2014-06-24 at 16:24 -0600, Jim Fehlig wrote:
> Ian Campbell wrote:
> > On Fri, 2014-06-20 at 15:07 -0600, Jim Fehlig wrote:
> >   
> >> The libxl driver currently sets the disk backend to
> >> LIBXL_DISK_BACKEND_TAP when <driver name='file'> is specified
> >> in the <disk> config.  qdisk should be prefered with this
> >> configuration, otherwise existing configuration such as the
> >> following, which worked with the old Xen driver, will not work
> >> with the libxl driver
> >>     
> >
> > OOI why not let libxl pick the most appropriate backend for (most of)
> > these alternatives?
> >   
> 
> I think the libvirt approach is to be a bit more explicit, instead of
> relying on hypervisor defaults that might change over time.  I'm quite
> sure this is the approach danpb has always advocated.

I thought that policy was more for guest visible things whereas the disk
backend is transparent to the guest.

But either way it's up to libvirt.

Thanks,
Ian.