[PATCH for 4.5] libxl: Tell qemu to use raw format when using a tapdisk

[PATCH for 4.5] libxl: Tell qemu to use raw format when using a tapdisk

[George Dunlap]

At the moment libxl unconditinally passes the underlying file format
to qemu in the device string.  However, when tapdisk is in use,
tapdisk handles the underlying format and presents qemu with
effectively a raw disk.  When qemu looks at the tapdisk block device
and doesn't find the image format it was looking for, it will fail.

This effectively means that tapdisk cannot be used with HVM domains at
the moment except for raw files.

Instead, if we're using a tapdisk backend, tell qemu to use a raw file
format.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Konrad Wilk <konrad.wilk@oracle.com>

Release exception justification: This fixes a bug in functionality, in
that at the moment HVM guests cannot boot with tapdisk and vhd format.

This is not a regression in xl functionality per se, since (AFAICT)
this has never worked.  However, given that 4.5 is the first release
without xend, this *does* represent a regression in functionality for
Xen as a whole (since before people using hvm guest with vhd on blktap
could use xend).

The fix is very simple and should only affect codepaths that already
don't work, so the risk of regressions should be very low.

While preparing this patch, I also noticed that cdroms will ignore the
backend parameter and treat everything as a file.  This is a bug but I
think it's a much less important one to address this late in the
release cycle.
---
 tools/libxl/libxl_dm.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index b25b574..10f3090 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -797,11 +797,14 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
                     continue;
                 }
 
-                if (disks[i].backend == LIBXL_DISK_BACKEND_TAP)
+                if (disks[i].backend == LIBXL_DISK_BACKEND_TAP) {
+                    format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
                     pdev_path = libxl__blktap_devpath(gc, disks[i].pdev_path,
                                                       disks[i].format);
-                else
+                } else {
                     pdev_path = disks[i].pdev_path;
+                }
+
 
                 /*
                  * Explicit sd disks are passed through as is.
-- 
1.9.1

Re: [PATCH for 4.5] libxl: Tell qemu to use raw format when using a tapdisk

[Wei Liu]

On Tue, Dec 09, 2014 at 02:04:19PM +0000, George Dunlap wrote:
> At the moment libxl unconditinally passes the underlying file format
> to qemu in the device string.  However, when tapdisk is in use,
> tapdisk handles the underlying format and presents qemu with
> effectively a raw disk.  When qemu looks at the tapdisk block device
> and doesn't find the image format it was looking for, it will fail.
> 
> This effectively means that tapdisk cannot be used with HVM domains at
> the moment except for raw files.
> 
> Instead, if we're using a tapdisk backend, tell qemu to use a raw file
> format.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Konrad Wilk <konrad.wilk@oracle.com>
> 

Acked-by: Wei Liu <wei.liu2@citrix.com>

> Release exception justification: This fixes a bug in functionality, in
> that at the moment HVM guests cannot boot with tapdisk and vhd format.
> 
> This is not a regression in xl functionality per se, since (AFAICT)
> this has never worked.  However, given that 4.5 is the first release
> without xend, this *does* represent a regression in functionality for
> Xen as a whole (since before people using hvm guest with vhd on blktap
> could use xend).
> 
> The fix is very simple and should only affect codepaths that already
> don't work, so the risk of regressions should be very low.
> 
> While preparing this patch, I also noticed that cdroms will ignore the
> backend parameter and treat everything as a file.  This is a bug but I
> think it's a much less important one to address this late in the
> release cycle.

We should create a bug tracker entry for this.

> ---
>  tools/libxl/libxl_dm.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index b25b574..10f3090 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -797,11 +797,14 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>                      continue;
>                  }
>  
> -                if (disks[i].backend == LIBXL_DISK_BACKEND_TAP)
> +                if (disks[i].backend == LIBXL_DISK_BACKEND_TAP) {
> +                    format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
>                      pdev_path = libxl__blktap_devpath(gc, disks[i].pdev_path,
>                                                        disks[i].format);
> -                else
> +                } else {
>                      pdev_path = disks[i].pdev_path;
> +                }
> +

Minor nit, extra blank line, but this alone doesn't warrant a resend.

>  
>                  /*
>                   * Explicit sd disks are passed through as is.
> -- 
> 1.9.1

Re: [PATCH for 4.5] libxl: Tell qemu to use raw format when using a tapdisk

[George Dunlap]

On 12/09/2014 02:32 PM, Wei Liu wrote:
> On Tue, Dec 09, 2014 at 02:04:19PM +0000, George Dunlap wrote:
>> At the moment libxl unconditinally passes the underlying file format
>> to qemu in the device string.  However, when tapdisk is in use,
>> tapdisk handles the underlying format and presents qemu with
>> effectively a raw disk.  When qemu looks at the tapdisk block device
>> and doesn't find the image format it was looking for, it will fail.
>>
>> This effectively means that tapdisk cannot be used with HVM domains at
>> the moment except for raw files.
>>
>> Instead, if we're using a tapdisk backend, tell qemu to use a raw file
>> format.
>>
>> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>> ---
>> CC: Ian Campbell <ian.campbell@citrix.com>
>> CC: Ian Jackson <ian.jackson@citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Konrad Wilk <konrad.wilk@oracle.com>
>>
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
>> Release exception justification: This fixes a bug in functionality, in
>> that at the moment HVM guests cannot boot with tapdisk and vhd format.
>>
>> This is not a regression in xl functionality per se, since (AFAICT)
>> this has never worked.  However, given that 4.5 is the first release
>> without xend, this *does* represent a regression in functionality for
>> Xen as a whole (since before people using hvm guest with vhd on blktap
>> could use xend).
>>
>> The fix is very simple and should only affect codepaths that already
>> don't work, so the risk of regressions should be very low.
>>
>> While preparing this patch, I also noticed that cdroms will ignore the
>> backend parameter and treat everything as a file.  This is a bug but I
>> think it's a much less important one to address this late in the
>> release cycle.
> 
> We should create a bug tracker entry for this.
> 
>> ---
>>  tools/libxl/libxl_dm.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>> index b25b574..10f3090 100644
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -797,11 +797,14 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>>                      continue;
>>                  }
>>  
>> -                if (disks[i].backend == LIBXL_DISK_BACKEND_TAP)
>> +                if (disks[i].backend == LIBXL_DISK_BACKEND_TAP) {
>> +                    format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
>>                      pdev_path = libxl__blktap_devpath(gc, disks[i].pdev_path,
>>                                                        disks[i].format);
>> -                else
>> +                } else {
>>                      pdev_path = disks[i].pdev_path;
>> +                }
>> +
> 
> Minor nit, extra blank line, but this alone doesn't warrant a resend.

Yes, I noticed this as soon as I sent it. :-/

 -George

Re: [PATCH for 4.5] libxl: Tell qemu to use raw format when using a tapdisk

[Ian Campbell]

On Tue, 2014-12-09 at 14:04 +0000, George Dunlap wrote:
> At the moment libxl unconditinally passes the underlying file format
> to qemu in the device string.  However, when tapdisk is in use,
> tapdisk handles the underlying format and presents qemu with
> effectively a raw disk.  When qemu looks at the tapdisk block device
> and doesn't find the image format it was looking for, it will fail.
> 
> This effectively means that tapdisk cannot be used with HVM domains at
> the moment except for raw files.
> 
> Instead, if we're using a tapdisk backend, tell qemu to use a raw file
> format.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Konrad Wilk <konrad.wilk@oracle.com>
> 
> Release exception justification:

I agree with your reasoning.

>  This fixes a bug in functionality, in
> that at the moment HVM guests cannot boot with tapdisk and vhd format.
> 
> This is not a regression in xl functionality per se, since (AFAICT)
> this has never worked.  However, given that 4.5 is the first release
> without xend, this *does* represent a regression in functionality for
> Xen as a whole (since before people using hvm guest with vhd on blktap
> could use xend).
> 
> The fix is very simple and should only affect codepaths that already
> don't work, so the risk of regressions should be very low.
> 
> While preparing this patch, I also noticed that cdroms will ignore the
> backend parameter and treat everything as a file.  This is a bug but I
> think it's a much less important one to address this late in the
> release cycle.
> ---
>  tools/libxl/libxl_dm.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index b25b574..10f3090 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -797,11 +797,14 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>                      continue;
>                  }
>  
> -                if (disks[i].backend == LIBXL_DISK_BACKEND_TAP)
> +                if (disks[i].backend == LIBXL_DISK_BACKEND_TAP) {
> +                    format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
>                      pdev_path = libxl__blktap_devpath(gc, disks[i].pdev_path,
>                                                        disks[i].format);
> -                else
> +                } else {
>                      pdev_path = disks[i].pdev_path;
> +                }
> +
>  
>                  /*
>                   * Explicit sd disks are passed through as is.

Re: [PATCH for 4.5] libxl: Tell qemu to use raw format when using a tapdisk

[Konrad Rzeszutek Wilk]

On Tue, Dec 09, 2014 at 02:48:21PM +0000, Ian Campbell wrote:
> On Tue, 2014-12-09 at 14:04 +0000, George Dunlap wrote:
> > At the moment libxl unconditinally passes the underlying file format
> > to qemu in the device string.  However, when tapdisk is in use,
> > tapdisk handles the underlying format and presents qemu with
> > effectively a raw disk.  When qemu looks at the tapdisk block device
> > and doesn't find the image format it was looking for, it will fail.
> > 
> > This effectively means that tapdisk cannot be used with HVM domains at
> > the moment except for raw files.
> > 
> > Instead, if we're using a tapdisk backend, tell qemu to use a raw file
> > format.
> > 
> > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> > ---
> > CC: Ian Campbell <ian.campbell@citrix.com>
> > CC: Ian Jackson <ian.jackson@citrix.com>
> > CC: Wei Liu <wei.liu2@citrix.com>
> > CC: Konrad Wilk <konrad.wilk@oracle.com>
> > 
> > Release exception justification:
> 
> I agree with your reasoning.
> 
> >  This fixes a bug in functionality, in
> > that at the moment HVM guests cannot boot with tapdisk and vhd format.
> > 
> > This is not a regression in xl functionality per se, since (AFAICT)
> > this has never worked.  However, given that 4.5 is the first release
> > without xend, this *does* represent a regression in functionality for
> > Xen as a whole (since before people using hvm guest with vhd on blktap
> > could use xend).
> > 
> > The fix is very simple and should only affect codepaths that already
> > don't work, so the risk of regressions should be very low.
> > 
> > While preparing this patch, I also noticed that cdroms will ignore the
> > backend parameter and treat everything as a file.  This is a bug but I
> > think it's a much less important one to address this late in the
> > release cycle.
> > ---
> >  tools/libxl/libxl_dm.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index b25b574..10f3090 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -797,11 +797,14 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> >                      continue;
> >                  }
> >  
> > -                if (disks[i].backend == LIBXL_DISK_BACKEND_TAP)
> > +                if (disks[i].backend == LIBXL_DISK_BACKEND_TAP) {
> > +                    format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
> >                      pdev_path = libxl__blktap_devpath(gc, disks[i].pdev_path,
> >                                                        disks[i].format);
> > -                else
> > +                } else {
> >                      pdev_path = disks[i].pdev_path;
> > +                }
> > +
> >  
> >                  /*
> >                   * Explicit sd disks are passed through as is.
> 
> 

Re: [PATCH for 4.5] libxl: Tell qemu to use raw format when using a tapdisk

[George Dunlap]

On Wed, Dec 10, 2014 at 4:30 PM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Tue, Dec 09, 2014 at 02:48:21PM +0000, Ian Campbell wrote:
>> On Tue, 2014-12-09 at 14:04 +0000, George Dunlap wrote:
>> > At the moment libxl unconditinally passes the underlying file format
>> > to qemu in the device string.  However, when tapdisk is in use,
>> > tapdisk handles the underlying format and presents qemu with
>> > effectively a raw disk.  When qemu looks at the tapdisk block device
>> > and doesn't find the image format it was looking for, it will fail.
>> >
>> > This effectively means that tapdisk cannot be used with HVM domains at
>> > the moment except for raw files.
>> >
>> > Instead, if we're using a tapdisk backend, tell qemu to use a raw file
>> > format.
>> >
>> > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>>
>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

This doesn't seem to have been applied yet.

 -George

Re: [PATCH for 4.5] libxl: Tell qemu to use raw format when using a tapdisk

[Ian Campbell]

On Mon, 2014-12-15 at 16:47 +0000, George Dunlap wrote:
> On Wed, Dec 10, 2014 at 4:30 PM, Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com> wrote:
> > On Tue, Dec 09, 2014 at 02:48:21PM +0000, Ian Campbell wrote:
> >> On Tue, 2014-12-09 at 14:04 +0000, George Dunlap wrote:
> >> > At the moment libxl unconditinally passes the underlying file format
> >> > to qemu in the device string.  However, when tapdisk is in use,
> >> > tapdisk handles the underlying format and presents qemu with
> >> > effectively a raw disk.  When qemu looks at the tapdisk block device
> >> > and doesn't find the image format it was looking for, it will fail.
> >> >
> >> > This effectively means that tapdisk cannot be used with HVM domains at
> >> > the moment except for raw files.
> >> >
> >> > Instead, if we're using a tapdisk backend, tell qemu to use a raw file
> >> > format.
> >> >
> >> > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> >>
> >> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >
> > Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> This doesn't seem to have been applied yet.

Now applied, nuking the extra blank line which way noticed on the way.

Ian.

Re: [PATCH for 4.5] libxl: Tell qemu to use raw format when using a tapdisk

[Wei Liu]

On Tue, Dec 09, 2014 at 02:04:19PM +0000, George Dunlap wrote:
[...]
> While preparing this patch, I also noticed that cdroms will ignore the
> backend parameter and treat everything as a file.  This is a bug

create !
title it CDROM's backend ignored and always treated as file
thanks

Processed: Re: [PATCH for 4.5] libxl: Tell qemu to use raw format when using a tapdisk

[xen]

Processing commands for xen@bugs.xenproject.org:

> create !
Created new bug #47 rooted at `<20141215154710.GC8049@zion.uk.xensource.com>'
Title: `Re: [PATCH for 4.5] libxl: Tell qemu to use raw format when using a tapdisk'
> title it CDROM backend ignored and always treated as file
Set title for #47 to `CDROM backend ignored and always treated as file'
> thanks
Finished processing.

Modified/created Bugs:
 - 47: http://bugs.xenproject.org/xen/bug/47 (new)

---
Xen Hypervisor Bug Tracker
See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs
Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues