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

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Wed, 26 Jan 2011, Kamala Narasimhan wrote:
> 
> Current disk validation code will fail when the disk file path is prefixed with tap:aio:vhd in the disk configuration file option.  This patch special cases tap:aio validation.
> 
> Note:  It appears qcow/qcow2 file format does not work with the current tapdisk.  So, I am checking only for vhd file format.  If there are other formats to check also, let me know.
> 

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.
A block device must be specified because blkback cannot handle files by
itself.
The format must be raw.

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

- tap:tapdisk:
exactly as for aio:, tapdisk: should just be ignored.

- tap:ioemu:
exactly as for aio: and tapdisk:, ioemu: should just be ignored.





Following these notes, we need:

- a document describing this in full details.

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

- A fix for validate_virtual_disk.

- A fix for the usage of aio in libxl: aio is currently considered a
type and should be renamed PHYSTYPE_RAW.

- A fix for libxl_device_disk_add so that QCOW and QCOW2 are
handled by qemu.

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:

---


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));