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