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

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

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@xxxxxxxxxxxxx>
>> ---
>> CC: Ian Campbell <ian.campbell@xxxxxxxxxx>
>> CC: Ian Jackson <ian.jackson@xxxxxxxxxx>
>> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
>> CC: Konrad Wilk <konrad.wilk@xxxxxxxxxx>
>>
> 
> Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> 
>> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel