WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] xl: Special case tap/aio for disk validation

To: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xl: Special case tap/aio for disk validation
From: Kamala Narasimhan <kamala.narasimhan@xxxxxxxxx>
Date: Thu, 27 Jan 2011 12:31:22 -0500
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Kamala Narasimhan <Kamala.Narasimhan@xxxxxxxxxx>
Delivery-date: Thu, 27 Jan 2011 09:32:05 -0800
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=3gasC2BlOyE7F0H4v95HipVwj8+jbjnrse+z+6wX/DE=; b=tDP7VGM2Mk0Qf5YwAIQG9QWlE1Dh32wRF2SuRZ9ZkynVAMak4pGizoDTU6pmGP65Wf dUprx4zGxiPm1QupDBtrBRu9Gru9lmxGq3soez3ui47QXNF3iJGqLkgShFoHP4cDVrJ7 VpZn2ezB//Spuhmnmao1NgbivV+GLkzb2XTZE=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; b=CHdL6LA96Fu66I4T6iBwULhCG87cRSPdhG/9Q8rjbszS6Lh3bXQNZR7b9wkrbyeSyH 4gBnTKH1nshWWjmSUZr/JM2Fmt79YtZfJa+8GMUWPn96JVPMnPqnb3QE0j3YV89MKH7F kltUuxMzdBombpi8fgF0hU2oT3IGsxF6AHacs=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <alpine.DEB.2.00.1101271506410.7277@kaball-desktop>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4D407A06.1050902@xxxxxxxxx> <alpine.DEB.2.00.1101271506410.7277@kaball-desktop>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird 2.0.0.24 (X11/20101027)
Stefano - Thanks, this is great information that I couldn't find before!  Rest 
in-line.

> This patch made me realize that we really need a specification for the
> disk format syntax in the config file and a better xl parser to match it.
> The syntax should be clear and at the same time backward compatible as
> much as we can.
> The followings are disk configurations supported in libxl and what they
> mean:
> 
> - phy:/path/to/device
> blkback is used as block backend, if blkback is missing from your dom0
> kernel, this configuration will fail.
So, we add a check in libxl validate disk code to find whether or not blkback 
is present rather than go all the way and fail?

> A block device must be specified because blkback cannot handle files by
> itself.
> The format must be raw.
> 
We probably need to extend the validation to confirm what is passed is a block 
device and not a file and I will add that.

> - file:/path/to/file
> tapdisk2 is used as block backend, if tapdisk2 is missing qemu is used
> instead.
> Both a file or a block device can be specified.
> The format is raw.
> 
> - tap:/path/to/file
> same as file:
> 
> - tap:qcow:/path/to/file
> qemu is used as block backend because tapdisk2 has broken qcow support.
Maybe we should explicitly fail this case and let the user know that tapdisk2 
is broken with qcow so they change the prefix in the config file.  Alternately, 
we could simply turn the error into a warning and keep going with qemu.

> Both a file or a block device can be specified.
> The format is qcow.
> 
> - tap:qcow2:/path/to/file
> qemu is used as block backend because tapdisk2 has broken qcow2 support.
Same as my comment for qcow.

> Both a file or a block device can be specified.
> The format is qcow2.
> 
> - tap:vhd:/path/to/file
> tapdisk2 is used as block backend, if tapdisk2 is missing qemu is used
> instead.
Same as my comment for qcow.

> Both a file or a block device can be specified.
> The format is vhd.
> 
> 
> The previous cases cover all the supported disk formats, but for
> backward compatibility we also support the following disk configurations:
> 
> - tap:aio:, tap:aio:qcow:, tap:aio:qcow2:, tap:aio:vhd:
> considering that aio is just an implementation detail, it should not be
> part of a user exposed interface, in the xl/libxenlight context aio is
> miningless. aio: should just be ignored by the xl config parser.
>
This would require a bit of refactoring in the parsing code to simply skip past 
aio.  Since we are not accepting interface changes for 4.1, this refactoring 
would have to wait till post 4.1?
 
> - tap:tapdisk:
> exactly as for aio:, tapdisk: should just be ignored.
> 
I don't think our disk parsing code even considers this option at the moment.  
So, we are probably failing when this option is specified.  We would need to 
tweak the parsing code to parse and ignore this option for backward 
compatibility.

> - tap:ioemu:
> exactly as for aio: and tapdisk:, ioemu: should just be ignored.
> 
Comment same as that of tap:tapdisk.
> 
> 
Argg... Sorry, some of what I said above might be redundant given that you have 
summarized what is needed.
> 
> 
> Following these notes, we need:
> 
> - a document describing this in full details.
Would a txt file do?  Where would it go - under tools/examples where other 
config options are described (albeit in a config file and not a separate file)?
> 
> - A fix for parse_disk_config.
> The new algorithm should assume format=raw and ignore tap:, tap2:, aio:,
> tapdisk:, ioemu:, until it gets to a real disk format (qcow:, qcow2:,
> vhd:) or the file name.
> 
Yes. Specifics/further questions above.

> - A fix for validate_virtual_disk.
>
Agreed.  Further clarifications above.

> - A fix for the usage of aio in libxl: aio is currently considered a
> type and should be renamed PHYSTYPE_RAW.
> 
Of course.  This is the interface change I mentioned above that IanJ might not 
take before 4.1.

> - A fix for libxl_device_disk_add so that QCOW and QCOW2 are
> handled by qemu.
>
Is you patch below sufficient for that?  Or, is there more that I need done 
that I am missing?
 
> All these fixes don't have to be separate patches, some of them might
> be collapsed in a sigle patch.
> 
> The last fix should be something like this:
>

Last question -  Are we looking to merge these patches for 4.1 or is this for 
post 4.1?

Kamala
 
> ---
> 
> 
> diff -r 7873885ec74d tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c     Thu Jan 27 09:37:19 2011 +0000
> +++ b/tools/libxl/libxl.c     Thu Jan 27 14:56:51 2011 +0000
> @@ -917,8 +919,6 @@ int libxl_device_disk_add(libxl_ctx *ctx
>              /* let's pretend is tap:aio for the moment */
>              disk->phystype = PHYSTYPE_AIO;
>          case PHYSTYPE_AIO:
> -        case PHYSTYPE_QCOW:
> -        case PHYSTYPE_QCOW2:
>          case PHYSTYPE_VHD:
>              if (libxl__blktap_enabled(&gc)) {
>                  const char *dev = libxl__blktap_devpath(&gc,
> @@ -939,6 +939,8 @@ int libxl_device_disk_add(libxl_ctx *ctx
>  
>                  break;
>              }
> +        case PHYSTYPE_QCOW:
> +        case PHYSTYPE_QCOW2:
>              flexarray_append(back, "params");
>              flexarray_append(back, libxl__sprintf(&gc, "%s:%s",
>                            
> libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath));
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>