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

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.

Signed-off-by: Kamala Narasimhan <kamala.narasimhan@citrix.com>

Kamala

diff -r 67d5b8004947 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c       Wed Jan 26 11:58:45 2011 +0000
+++ b/tools/libxl/libxl.c       Wed Jan 26 14:26:57 2011 -0500
@@ -836,22 +836,40 @@ static int validate_virtual_disk(libxl_c
 static int validate_virtual_disk(libxl_ctx *ctx, char *file_name,
libxl_disk_phystype disk_type)
 {
     struct stat stat_buf;
+    const char *fname;

     if ( (file_name[0] == ''\0'') && (disk_type ==
PHYSTYPE_EMPTY) )
         return 0;

-    if ( stat(file_name, &stat_buf) != 0 ) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s",
file_name);
+    if ( disk_type == PHYSTYPE_AIO ) {
+        fname = strchr(file_name, '':'');
+        if ( fname == NULL ) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "missing file format
prefix!"
+                   " tap:aio disk option must be followed by file format
type");
+            return ERROR_INVAL;
+        }
+        fname++;
+        if ( strncmp(file_name, "vhd:", sizeof("vhd:")-1)
!= 0 ) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Only vhd file format
supported"
+                   " with tapdisk");
+            return ERROR_INVAL;
+        }
+    }
+    else
+        fname = file_name;
+
+    if ( stat(fname, &stat_buf) != 0 ) {
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s",
fname);
         return ERROR_INVAL;
     }
     if ( disk_type == PHYSTYPE_PHY ) {
         if ( !(S_ISBLK(stat_buf.st_mode)) ) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s is not a
block device!\n",
-                file_name);
+                fname);
             return ERROR_INVAL;
         }
     } else if ( S_ISREG(stat_buf.st_mode) && stat_buf.st_size == 0 ) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s size is
0!\n", file_name);
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s size is
0!\n", fname);
         return ERROR_INVAL;
     }



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

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

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

Stefano Stabellini writes ("Re: [Xen-devel] [PATCH] xl: Special case
tap/aio for disk validation"):
> - 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.
You can recognise the filename by the fact that it starts with a "/",
since most of our stuff doesn''t work with relative pathnames for block
devices and we don''t want it to try to make it work now.

So the specification string should always be either
   someprefix:[stuff]
in which case you strip off someprefix: (perhaps making a note of its
implications) or
   /somepath
in which case you''ve got to the end and know what to do.

Ian.

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

On Thu, 2011-01-27 at 15:22 +0000, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH] xl: Special case
tap/aio for disk validation"):
> > - 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.
> 
> You can recognise the filename by the fact that it starts with a
"/",
> since most of our stuff doesn''t work with relative pathnames for
block
> devices and we don''t want it to try to make it work now.
FWIW xend notices a relative path and wacks a "/dev/" on the front.
> So the specification string should always be either
>    someprefix:[stuff]
> in which case you strip off someprefix: (perhaps making a note of its
> implications) or /somepath
> in which case you''ve got to the end and know what to do.
Shall we pre-deprecate the someprefix: along with all the
tap:{aio,ioemu,FOO}: stuff while we are at this? IOW the syntax we want
to actually support going forward would be:
	(raw|vhd|qcow2|qcow):/path/to/something
with the toolstack being at liberty to best decide how to achieve the
necessary format support? (optionally raw: as the default). All the rest
of the format is really semantically meaningless and supported only for
compatibility with existing configuration files.

Ian.



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

Hello,

Am Donnerstag 27 Januar 2011 16:17:01 schrieb Stefano
Stabellini:
> - tap:vhd:/path/to/file
> tapdisk2 is used as block backend, if tapdisk2 is missing qemu is used
> instead.
As far as I know "vhd" is not supported by qemu and only is
implemented in
tapdisk2. You might recheck that.

Sincerely
Philipp
-- 
Philipp Hahn           Open Source Software Engineer      hahn@univention.de   
Univention GmbH        Linux for Your Business        fon: +49 421 22 232- 0
Mary-Somerville-Str.1  28359 Bremen                   fax: +49 421 22 232-99
                                                    http://www.univention.de/


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

Ian Campbell writes ("Re: [Xen-devel] [PATCH] xl: Special case tap/aio for
disk validation"):
> FWIW xend notices a relative path and wacks a "/dev/" on the
front.
Oh dear.  So is the syntax
   some:pile:of:useless:prefixes:sponge
allowed to mean /dev/sponge ?  What if you want to refer to
/dev/sponge:1 ?  I guess that probably doesn''t work right now.

So the right algorithm is:
   - rhs starts with ''/'': it''s a pathname
   - rhs has a colon: it''s a prefix
   - rhs has neither: hope it''s a pathname in /dev
?
> Shall we pre-deprecate the someprefix: along with all the
> tap:{aio,ioemu,FOO}: stuff while we are at this? IOW the syntax we want
> to actually support going forward would be:
> 	(raw|vhd|qcow2|qcow):/path/to/something
> with the toolstack being at liberty to best decide how to achieve the
> necessary format support? (optionally raw: as the default). All the rest
> of the format is really semantically meaningless and supported only for
> compatibility with existing configuration files.
Yes.

Ian.

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

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@lists.xensource.com
> http://lists.xensource.com/xen-devel

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

> Shall we pre-deprecate the someprefix: along with all the
> tap:{aio,ioemu,FOO}: stuff while we are at this? IOW the syntax we want
> to actually support going forward would be:
> 	(raw|vhd|qcow2|qcow):/path/to/something
> with the toolstack being at liberty to best decide how to achieve the
> necessary format support? (optionally raw: as the default). All the rest
> of the format is really semantically meaningless and supported only for
> compatibility with existing configuration files.
>
That sure does simply things!  When we have multiple backends that work with
different file formats for users to choose from, we can revisit this.

Kamala
 
> Ian.
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

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

>> Shall we pre-deprecate the someprefix: along with all the
>> tap:{aio,ioemu,FOO}: stuff while we are at this? IOW the syntax we want
>> to actually support going forward would be:
>> 	(raw|vhd|qcow2|qcow):/path/to/something
>> with the toolstack being at liberty to best decide how to achieve the
>> necessary format support? (optionally raw: as the default). All the
rest
>> of the format is really semantically meaningless and supported only for
>> compatibility with existing configuration files.
> 
> Yes.
> 
That brings the question on what we do to make it obvious to the user that we
are deprecating these options.  That is other than explicitly mentioning it in
the documentation.  Do we display a warning in our validation code?

Kamala


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

On Thu, 2011-01-27 at 16:23 +0000, Ian Jackson wrote: 
> Ian Campbell writes ("Re: [Xen-devel] [PATCH] xl: Special case tap/aio
for disk validation"):
> > FWIW xend notices a relative path and wacks a "/dev/" on the
front.
> 
> Oh dear.  So is the syntax
>    some:pile:of:useless:prefixes:sponge
> allowed to mean /dev/sponge ?  What if you want to refer to
> /dev/sponge:1 ?  I guess that probably doesn''t work right now.
> 
> So the right algorithm is:
>    - rhs starts with ''/'': it''s a pathname
>    - rhs has a colon: it''s a prefix
>    - rhs has neither: hope it''s a pathname in /dev
> ?
Sounds about right.

FWIW from xend:

def _parse_uname(uname):
    fn = taptype = None
    if uname.find(":") != -1:
        (typ, fn) = uname.split(":", 1)

        if typ in ("phy", "drbd") and not
fn.startswith("/"):
            fn = "/dev/%s" %(fn,)
               
        if typ in ("tap", "tap2"):
            (taptype, fn) = fn.split(":", 1)
    return (fn, taptype)

/me cries.



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

On Thu, 27 Jan 2011, Kamala Narasimhan wrote:
> 
> 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?
> 
Yeah, you can check for the presence of:

/sys/modules/{blkbk,xen_blkback}

the modules are called differently in pvops compared to traditional
xenolinux kernels.

> > 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.
Right.
> > - 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.
> 
I think there is nothing wrong in using the qemu disk backend,
especially when the new qemu will be available with much better aio
support.
I also think that we shouldn''t bother the users with implementation
details on the disk backend we use, the toolstack should just set it up
behind the scene.
So in the qcow and qcow2 cases I wouldn''t print anything at all.
However in the vhd case, considering that I have never tested qemu''s
vhd
support, I don''t know how that would actually work. So for the vhd
format it might be OK to just print an error an exit if tapdisk2 is not
present.

> > 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?
>  
I think this fix can be considered critical so we''ll accept it for 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.
> 
Right.

> > 
> > 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)?
Yep. A text file under docs will be perfect.


 
> > - 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.
We''ll take it because it is a critical fix. Xl has to be compatibile
with xend.
> 
> > - 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?
>  
I think the patch below should be sufficient, once the disk parsing is
fixed (in particular "aio:" is removed).
> > 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?
> 
the target is 4.1

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

On Thu, 27 Jan 2011, Kamala Narasimhan wrote:
> >> Shall we pre-deprecate the someprefix: along with all the
> >> tap:{aio,ioemu,FOO}: stuff while we are at this? IOW the syntax we
want
> >> to actually support going forward would be:
> >> 	(raw|vhd|qcow2|qcow):/path/to/something
> >> with the toolstack being at liberty to best decide how to achieve
the
> >> necessary format support? (optionally raw: as the default). All
the rest
> >> of the format is really semantically meaningless and supported
only for
> >> compatibility with existing configuration files.
> > 
> > Yes.
> > 
> 
> That brings the question on what we do to make it obvious to the user that
we are deprecating these options.  That is other than explicitly mentioning it
in the documentation.  Do we display a warning in our validation code?
> 
While fixing the disk parsing is critical for 4.1, I am not sure if we
can deprecate the current syntax for 4.1 as well.
We should keep in mind that xend is still using the other syntax and it
is still present in-tree, so if we really want to deprecate it we need
to fix xend too. 

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

Stefano Stabellini writes ("Re: [Xen-devel] [PATCH] xl: Special case
tap/aio for disk validation"):
> Yeah, you can check for the presence of:
> 
> /sys/modules/{blkbk,xen_blkback}
> 
> the modules are called differently in pvops compared to traditional
> xenolinux kernels.
No, that doesn''t work because it may not be a module.  And we need an
answer that works with 2.6.18 as well as 2.6.27 and 2.6.32 and 2.6.37
and 2.6.38 ...

Ian.

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

On Thu, 2011-01-27 at 18:35 +0000, Ian Jackson wrote: 
> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH] xl: Special case
tap/aio for disk validation"):
> > Yeah, you can check for the presence of:
> > 
> > /sys/modules/{blkbk,xen_blkback}
> > 
> > the modules are called differently in pvops compared to traditional
> > xenolinux kernels.
> 
> No, that doesn''t work because it may not be a module.
These entries appear for builtin things too (badly named directory? why
yes) if the "module" has any parameters. blkback does appear, I
checked
on both types of dom0 with static blkback.

It looks like /sys/bus/xen-backend/drivers/vbd/
and /sys/bus/xen/drivers/vbd/ are also both present on systems with
blkback available and have consistent names on both classic-Xen and
pvops systems AFAICT.

One of these last two is likely to be the right answer, since the
"vbd"
part of the string is the xenstore directory name too and the two are
linked.
> And we need an answer that works with 2.6.18 as well as 2.6.27 and 2.6.32
and 2.6.37
> and 2.6.38 ...
2.6.18 would be the unknown here. Should be easy enough for someone to
confirm I guess.

I''m not sure that failing to detect blkback on 2.6.18 and therefore
falling back to tapdisk* and/or qemu is a huge problem but I suppose
it''s best avoided.

Ian.



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

On Thu, 27 Jan 2011, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH] xl: Special case
tap/aio for disk validation"):
> > Yeah, you can check for the presence of:
> > 
> > /sys/modules/{blkbk,xen_blkback}
> > 
> > the modules are called differently in pvops compared to traditional
> > xenolinux kernels.
> 
> No, that doesn''t work because it may not be a module.  And we need
an
> answer that works with 2.6.18 as well as 2.6.27 and 2.6.32 and 2.6.37
> and 2.6.38 ...
> 
I think that they should be present even if they are built-in.
All the pvops kernels should have:

/sys/modules/xen_blkback

while all the xenlinux kernels should have:

/sys/modules/blkbk

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

> While fixing the disk parsing is critical for 4.1, I am not sure if we
> can deprecate the current syntax for 4.1 as well.
> We should keep in mind that xend is still using the other syntax and it
> is still present in-tree, so if we really want to deprecate it we need
> to fix xend too. 
By deprecating I didn''t mean we would remove the functionality and
introduce inconsistency between xend and xl in terms of usage.  I
wasn''t sure if we should stop with documenting the fact that it is
deprecated or also display a warning message (in libxl) to that effect.  I would
go with a warning also unless someone disapproves.

Kamala


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

> - phy:/path/to/device
> - file:/path/to/file
> - tap:/path/to/file
Along with phy, file and tap for block device types there appear to be a
''drdb'' option too.  Do we actively support it?  If so do we
treat it same as ''phy'' and use blkback for backend?

Regarding VDEV (appears to be referred to as drive designation in some
specificatons) -

Other than hd[x] and xvd[x], are there other supported VDEV formats?

Kamala



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

On Thu, 27 Jan 2011, Stefano Stabellini wrote:
> - 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.
If you want backwards compatibility then phy:devname works in xm as well 
as phy:/dev/devname . Many of the example files in /etc/xen still use the 
earlier format.

 	Michael Young

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

>>> 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)?
> 
> Yep. A text file under docs will be perfect.
> 
> 
>  
Attached is a first stab at this for your perusal.  It is mostly based on the
discussion we had in this thread.  Please let me know any/all changes you would
recommend.

I am sending this in advance as anything I have captured wrong here would mean
it is likely that the code changes I am making right now could also be wrong! 
So please do respond back if I have incorrectly captured any  conclusion we
arrived at in terms of the supported/deprecated attributes for the disk
configuration option.  Thanks!

Kamala



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

On Thu, 2011-01-27 at 20:14 +0000, Kamala Narasimhan
wrote:
> > While fixing the disk parsing is critical for 4.1, I am not sure if we
> > can deprecate the current syntax for 4.1 as well.
> > We should keep in mind that xend is still using the other syntax and
it
> > is still present in-tree, so if we really want to deprecate it we need
> > to fix xend too. 
> 
> By deprecating I didn''t mean we would remove the functionality and
> introduce inconsistency between xend and xl in terms of usage.  I
> wasn''t sure if we should stop with documenting the fact that it is
> deprecated or also display a warning message (in libxl) to that
> effect.  I would go with a warning also unless someone disapproves.
When I originally said "deprecate" I really meant "plan to
deprecate",
i.e. accept the new and old syntax now and intend to deprecate the old
syntax e.g. in 4.2.

WRT xend compatibility, I think the goal that a xend/xm configuration
file can be fed to xl without modification is a worthy one and obviously
aids in the transition etc.

The opposite goal (that an xl configuration file can be fed directly to
xm) is not so obviously worthwhile.

I don''t think we should be hobbling the evolution and ongoing use of
the
xl configuration file syntax (or indeed the (lib)xl functionality)
simply to keep xend happy, in just the same way as we currently don''t
insist that every feature added to (lib)xl also gets added to xend.

I guess what I mean is that if people use xl specific syntax/options in
their configuration files then they should not expect things to Just
Work if they go back to xend.

Ian.


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

On Fri, 2011-01-28 at 01:56 +0000, Kamala Narasimhan
wrote:
> >>> 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)?
> > 
> > Yep. A text file under docs will be perfect.
> > 
> > 
> >  
> Attached is a first stab at this for your perusal.  It is mostly based
> on the discussion we had in this thread.  Please let me know any/all
> changes you would recommend.
> 
> I am sending this in advance as anything I have captured wrong here
> would mean it is likely that the code changes I am making right now
> could also be wrong!  So please do respond back if I have incorrectly
> captured any  conclusion we arrived at in terms of the
> supported/deprecated attributes for the disk configuration option.
> Thanks!
Nice one. I have a handful of minor comments.

There''s a bunch of examples of this stuff in tools/examples, much of
which would benefit from the addition of a reference to this document in
its eventual resting place and sanity checking for adherence to what the
document says. It''s hard to believe (ok, not really) that we
don''t have
any other in tree documentation of any of the other configuration syntax
other than those examples (and some vagueish lists in
tools/python/README.*) but it does seem to be the case :-(. Perhaps
there is something in the wiki or else maybe this document can be the
seed around which the rest grows...

Several elements of the ''format:path,vdev:type,attrib'' string
are
optional. Can we delineate those in some non-confusing way? Often man
pages and the like use [], which would end up as (I think):
	[format:]path,vdev[:type],attrib

I don''t know if that meets the non-confusing criterion though. I
didn''t
initially notice the "Mandatory" tag (see comments on line-wrapping
below). The thing the tag doesn''t cover is how the punctuation binds to
the mandatory vs. optional elements. Perhaps insert
        The "format:" and ":type" elements are optional
alongside the comment about not all attributes being required?

IanJ wrote a existing spec for vdev at one point. I thought it had been
committed somewhere but I can''t see it. I think the most recent version
was
http://lists.xensource.com/archives/html/xen-devel/2010-09/msg01329.html
but perhaps IanJ can confirm. We should incorporate it, either by
reference or as a separate chapter in the same document or something
similar.

The discussion of which backend corresponds to each option is useful
from an implementation detail point of view but I''m not sure it is
necessary as part of the documentation of the configuration syntax.
It''s
liable get out of date IMHO. Perhaps those bits would be better as a
comment alongside the implementation?

It''s not clear where the deprecated block-dev-type and access-type
attributes are acceptable. I think we can just say that multiple
deprecated attributes are accepted before the first valid "format"
type
is discovered, with the last one taking precedence, or something along
those lines.

Lastly, I think the doc should be line-wrapped to 80 columns, otherwise
the autowrapping of the table like elements ends up quite hard to
follow. e.g. it currently comes out looking like:
        
        Description:         Specifies the format of image file.
        Supported values:    raw, qcow, qcow2, vhd 
        Deprecated values:   None
        Mandatory:           No. When not specified raw format is
        assumed.  For a physical block device the format must be raw and
        need not be explicitly specified.  For an image file the format
        could be one of the supported values and when not specified
        assumed to be raw.
        Backend used:        For qcow/qcow2 qemu backend is u[...]
        
should be (subject to my guessing where 80 columns actually is in this
case):

        Description:         Specifies the format of image file.
        Supported values:    raw, qcow, qcow2, vhd 
        Deprecated values:   None
        Mandatory:           No. When not specified raw format is assumed.  For
a physical block device the format
                             must be raw and need not be explicitly specified. 
For an image file the format could
                             be one of the supported values and when not
specified assumed to be raw.
        Backend used:        For qcow/qcow2 qemu backend is u[...]
                             [...] etc 


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

On Fri, 2011-01-28 at 10:06 +0000, Ian Campbell wrote:
> It''s hard to believe (ok, not really) that we don''t have
> any other in tree documentation of any of the other configuration
> syntax other than those examples (and some vagueish lists in
> tools/python/README.*) but it does seem to be the case :-(. Perhaps
> there is something in the wiki or else maybe this document can be the
> seed around which the rest grows... 
I found http://wiki.xen.org/xenwiki/XenConfigurationFileOptions (which
also refers to
http://www.xen.org/files/Support/XenConfigurationDetails.pdf).

I''m not sure if the canonical place for this sort of thing should be
the
wiki (where it is simple for anyone to update, but where it might get
out of date or suffer from version skew etc) or in-tree (where we can
patch as part of the implementation of new features).

I''d be inclined to suggest we take that wiki page, text-ify it, sanity
check for updatedness, stick it in the tree and update the wiki to
reference it. Opinions?

Ian.


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

Ian Campbell writes ("Re: [Xen-devel] [PATCH] xl: Special case tap/aio for
disk validation"):
> I''m not sure if the canonical place for this sort of thing should
be the
> wiki (where it is simple for anyone to update, but where it might get
> out of date or suffer from version skew etc) or in-tree (where we can
> patch as part of the implementation of new features).
There should be primary reference documentation, of every syntax,
public interface, and feature, in-tree.  This should be in plain text
files or perhaps manpages or something equally easy to edit and view.

That way we can require people who add new features to update the
documentation, and users will have the documentation which corresponds
to the code that they have.
> I''d be inclined to suggest we take that wiki page, text-ify it,
sanity
> check for updatedness, stick it in the tree and update the wiki to
> reference it. Opinions?
That would be a good start.  NB that there''s also the xm-config.5
manpage.  All of this needs to be unified.

Ian.

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

On Fri, 28 Jan 2011, Ian Campbell wrote:
> On Thu, 2011-01-27 at 20:14 +0000, Kamala Narasimhan wrote:
> > > While fixing the disk parsing is critical for 4.1, I am not sure
if we
> > > can deprecate the current syntax for 4.1 as well.
> > > We should keep in mind that xend is still using the other syntax
and it
> > > is still present in-tree, so if we really want to deprecate it we
need
> > > to fix xend too. 
> > 
> > By deprecating I didn''t mean we would remove the
functionality and
> > introduce inconsistency between xend and xl in terms of usage.  I
> > wasn''t sure if we should stop with documenting the fact that
it is
> > deprecated or also display a warning message (in libxl) to that
> > effect.  I would go with a warning also unless someone disapproves.
> 
> When I originally said "deprecate" I really meant "plan to
deprecate",
> i.e. accept the new and old syntax now and intend to deprecate the old
> syntax e.g. in 4.2.
> 
Yes, this plan is much more feasible.
> WRT xend compatibility, I think the goal that a xend/xm configuration
> file can be fed to xl without modification is a worthy one and obviously
> aids in the transition etc.
> 
> The opposite goal (that an xl configuration file can be fed directly to
> xm) is not so obviously worthwhile.
> 
> I don''t think we should be hobbling the evolution and ongoing use
of the
> xl configuration file syntax (or indeed the (lib)xl functionality)
> simply to keep xend happy, in just the same way as we currently
don''t
> insist that every feature added to (lib)xl also gets added to xend.
> 
> I guess what I mean is that if people use xl specific syntax/options in
> their configuration files then they should not expect things to Just
> Work if they go back to xend.
 
+1

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

On Thu, 27 Jan 2011, Kamala Narasimhan wrote:
> > - phy:/path/to/device
> > - file:/path/to/file
> > - tap:/path/to/file
> 
> Along with phy, file and tap for block device types there appear to be a
''drdb'' option too.  Do we actively support it?  If so do we
treat it same as ''phy'' and use blkback for backend?
Yes, I think so. Given his recent work on drdb support in qemu, maybe
James can comment on this.
> 
> Regarding VDEV (appears to be referred to as drive designation in some
specificatons) -
> 
> Other than hd[x] and xvd[x], are there other supported VDEV formats?
> 
No.

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

On Fri, 28 Jan 2011, Kamala Narasimhan wrote:
> >>> 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)?
> > 
> > Yep. A text file under docs will be perfect.
> > 
> > 
> >  
> Attached is a first stab at this for your perusal.  It is mostly based on
the discussion we had in this thread.  Please let me know any/all changes you
would recommend.
> 
> I am sending this in advance as anything I have captured wrong here would
mean it is likely that the code changes I am making right now could also be
wrong!  So please do respond back if I have incorrectly captured any  conclusion
we arrived at in terms of the supported/deprecated attributes for the disk
configuration option.  Thanks!
> 
I think it is pretty good.
You need to add tap2 among the block-dev-types.


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

On Fri, 28 Jan 2011, Ian Campbell wrote:
> The discussion of which backend corresponds to each option is useful
> from an implementation detail point of view but I''m not sure it is
> necessary as part of the documentation of the configuration syntax.
It''s
> liable get out of date IMHO. Perhaps those bits would be better as a
> comment alongside the implementation?
> 
I think it is really important that we write it down somewhere in a
document, after all users have to know what to compile in order to get
things to work.
However considering that this is an interface description, there is an
argument for moving this information elsewhere, but for simplicity''s
sake I would just leave it there.
> Lastly, I think the doc should be line-wrapped to 80 columns, otherwise
if you are using thunderbird you can use this extension to word wrap
your email:

https://addons.mozilla.org/en-US/thunderbird/addon/toggle-word-wrap/


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

On Fri, 2011-01-28 at 13:19 +0000, Stefano Stabellini
wrote:
> 
> 
> > Lastly, I think the doc should be line-wrapped to 80 columns,
> otherwise
> 
> if you are using thunderbird you can use this extension to word wrap
> your email:
> 
> https://addons.mozilla.org/en-US/thunderbird/addon/toggle-word-wrap/
I''m not worried about the email, I''m worried about the format
of the
document checked into xen-unstable which will be read with all sorts of
different tools.

Ian.


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

On Fri, 28 Jan 2011, Ian Campbell wrote:
> On Fri, 2011-01-28 at 13:19 +0000, Stefano Stabellini wrote:
> > 
> > 
> > > Lastly, I think the doc should be line-wrapped to 80 columns,
> > otherwise
> > 
> > if you are using thunderbird you can use this extension to word wrap
> > your email:
> > 
> > https://addons.mozilla.org/en-US/thunderbird/addon/toggle-word-wrap/
> 
> I''m not worried about the email, I''m worried about the
format of the
> document checked into xen-unstable which will be read with all sorts of
> different tools.
the implicit next step is to write the document inline in the email with
that extension:)

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

On Fri, 2011-01-28 at 13:28 +0000, Stefano Stabellini
wrote:
> On Fri, 28 Jan 2011, Ian Campbell wrote:
> > On Fri, 2011-01-28 at 13:19 +0000, Stefano Stabellini wrote:
> > > 
> > > 
> > > > Lastly, I think the doc should be line-wrapped to 80
columns,
> > > otherwise
> > > 
> > > if you are using thunderbird you can use this extension to word
wrap
> > > your email:
> > > 
> > >
https://addons.mozilla.org/en-US/thunderbird/addon/toggle-word-wrap/
> > 
> > I''m not worried about the email, I''m worried about
the format of the
> > document checked into xen-unstable which will be read with all sorts
of
> > different tools.
> 
> the implicit next step is to write the document inline in the email with
> that extension:)
Oh right, sure ;-)

I think it probably needs a bit more manual formatting than just
automatic wordwrap to make it readable though, e.g. to indent the table
in a readable way.

Ian.


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

> Several elements of the ''format:path,vdev:type,attrib''
string are
> optional. Can we delineate those in some non-confusing way? Often man
> pages and the like use [], which would end up as (I think):
> 	[format:]path,vdev[:type],attrib
>
Believe it or not I tried that!  It ended up looking very confusing which is why
I switched to explicit verbal description of whether or not a particular
attribute is mandatory.
> I don''t know if that meets the non-confusing criterion though. I
didn''t
> initially notice the "Mandatory" tag (see comments on
line-wrapping
> below). The thing the tag doesn''t cover is how the punctuation
binds to
> the mandatory vs. optional elements. Perhaps insert
>         The "format:" and ":type" elements are optional
> alongside the comment about not all attributes being required?
> 
Sure, if that helps.
> IanJ wrote a existing spec for vdev at one point. I thought it had been
> committed somewhere but I can''t see it. I think the most recent
version
> was
> http://lists.xensource.com/archives/html/xen-devel/2010-09/msg01329.html
> but perhaps IanJ can confirm. We should incorporate it, either by
> reference or as a separate chapter in the same document or something
> similar.
> 
Sure.
> The discussion of which backend corresponds to each option is useful
> from an implementation detail point of view but I''m not sure it is
> necessary as part of the documentation of the configuration syntax.
It''s
> liable get out of date IMHO. Perhaps those bits would be better as a
> comment alongside the implementation?
> 
I do agree this is an information a pure end user won''t care about. 
The
question then gets to who is the target audience for this document.  If it is
mostly developers, it might help for them to have this information.
> It''s not clear where the deprecated block-dev-type and access-type
> attributes are acceptable. I think we can just say that multiple
> deprecated attributes are accepted before the first valid
"format" type
> is discovered, with the last one taking precedence, or something along
> those lines.
>
Sure.
> Lastly, I think the doc should be line-wrapped to 80 columns, otherwise
> the autowrapping of the table like elements ends up quite hard to
> follow. e.g. it currently comes out looking like:
>         
>         Description:         Specifies the format of image file.
>         Supported values:    raw, qcow, qcow2, vhd 
>         Deprecated values:   None
>         Mandatory:           No. When not specified raw format is
>         assumed.  For a physical block device the format must be raw and
>         need not be explicitly specified.  For an image file the format
>         could be one of the supported values and when not specified
>         assumed to be raw.
>         Backend used:        For qcow/qcow2 qemu backend is u[...]
>
> should be (subject to my guessing where 80 columns actually is in this
> case):
> 
>         Description:         Specifies the format of image file.
>         Supported values:    raw, qcow, qcow2, vhd 
>         Deprecated values:   None
>         Mandatory:           No. When not specified raw format is assumed. 
For a physical block device the format
>                              must be raw and need not be explicitly
specified.  For an image file the format could
>                              be one of the supported values and when not
specified assumed to be raw.
>         Backend used:        For qcow/qcow2 qemu backend is u[...]
>                              [...] etc 
> 
Yep, I will do that and send out a revised version.

Kamala

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

Stefano Stabellini wrote:
> On Fri, 28 Jan 2011, Ian Campbell wrote:
>> On Fri, 2011-01-28 at 13:19 +0000, Stefano Stabellini wrote:
>>>
>>>> Lastly, I think the doc should be line-wrapped to 80 columns,
>>> otherwise
>>>
>>> if you are using thunderbird you can use this extension to word
wrap
>>> your email:
>>>
>>>
https://addons.mozilla.org/en-US/thunderbird/addon/toggle-word-wrap/
>> I''m not worried about the email, I''m worried about
the format of the
>> document checked into xen-unstable which will be read with all sorts of
>> different tools.
> 
> the implicit next step is to write the document inline in the email with
> that extension:)
Yes, I have that addon installed.  But it doesn''t allow me to
selectively word
wrap parts of the message which I would need while submitting patches (as I
wouldn''t want to word wrap code) which is why I turned off word
wrapping
completely.  I included the text file as an attachment and wasn''t
expecting it
to inline it to the email (which I then realized would be the case with some
email clients)!  Anyway, as suggested I should be fixing the format in the txt
file which I will.

Kamala





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

Kamala Narasimhan wrote:
>> - phy:/path/to/device
>> - file:/path/to/file
>> - tap:/path/to/file
>>     
>
> Along with phy, file and tap for block device types there appear to be a
''drdb'' option too.  Do we actively support it?
The xend toolstack supports arbitrary external block device types.  We
have block-iscsi and block-npiv,  and the drbd project provides
block-drbd.  I''m quite sure I''ve seen other custom block types
as well.
Will these arbitrary block device types still be supported in the new
toolstack?

Thanks,
Jim


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

On Fri, 28 Jan 2011, Jim Fehlig wrote:
> Kamala Narasimhan wrote:
> >> - phy:/path/to/device
> >> - file:/path/to/file
> >> - tap:/path/to/file
> >>     
> >
> > Along with phy, file and tap for block device types there appear to be
a ''drdb'' option too.  Do we actively support it?
> 
> The xend toolstack supports arbitrary external block device types.  We
> have block-iscsi and block-npiv,  and the drbd project provides
> block-drbd.  I''m quite sure I''ve seen other custom block
types as well.
> Will these arbitrary block device types still be supported in the new
> toolstack?
Is it just a matter of forking and executing an external script to
setup a block device and threat it as phy: afterwards?
It shouldn''t be too difficult but still non-trivial at this point of
the
release cycle.
Maybe users could work around the lack of support in 4.1 calling the
script themselves and using the name of the block device in the VM
config file directly?

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

its drbd :)

On Fri, Jan 28, 2011 at 4:57 AM, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Thu, 27 Jan 2011, Kamala Narasimhan wrote:
>> > - phy:/path/to/device
>> > - file:/path/to/file
>> > - tap:/path/to/file
>>
>> Along with phy, file and tap for block device types there appear to be
a ''drdb'' option too.  Do we actively support it?  If so do we
treat it same as ''phy'' and use blkback for backend?
>
> Yes, I think so. Given his recent work on drdb support in qemu, maybe
> James can comment on this.
>
>>
>> Regarding VDEV (appears to be referred to as drive designation in some
specificatons) -
>>
>> Other than hd[x] and xvd[x], are there other supported VDEV formats?
>>
>
> No.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>


-- 
perception is but an offspring of its own self

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

Stefano Stabellini wrote:
> On Fri, 28 Jan 2011, Jim Fehlig wrote:
>   
>> Kamala Narasimhan wrote:
>>     
>>>> - phy:/path/to/device
>>>> - file:/path/to/file
>>>> - tap:/path/to/file
>>>>     
>>>>         
>>> Along with phy, file and tap for block device types there appear to
be a ''drdb'' option too.  Do we actively support it?
>>>       
>> The xend toolstack supports arbitrary external block device types.  We
>> have block-iscsi and block-npiv,  and the drbd project provides
>> block-drbd.  I''m quite sure I''ve seen other custom
block types as well.
>> Will these arbitrary block device types still be supported in the new
>> toolstack?
>>     
>
> Is it just a matter of forking and executing an external script to
> setup a block device and threat it as phy: afterwards?
>   
Currently, IIUC, the external script is invoked by udev

SUBSYSTEM=="xen-backend", KERNEL=="tap*",
RUN+="/etc/xen/scripts/blktap
$env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vbd*",
RUN+="/etc/xen/scripts/block
$env{ACTION}"

The block script will delegate to a helper if needed, which configures
the block device and writes some info (like resulting dev node) to
xenstore.  xend and/or qemu then read the info and continue configuring
the device from backend perspective.
> It shouldn''t be too difficult but still non-trivial at this point
of the
> release cycle.
> Maybe users could work around the lack of support in 4.1 calling the
> script themselves and using the name of the block device in the VM
> config file directly?
>   
Yes, this is certainly a workaround.  One could argue this is the proper
solution and remove support for external block scripts.  But this
functionality has been around for years.

Jim

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

> IanJ wrote a existing spec for vdev at one point. I thought it had been
> committed somewhere but I can''t see it. I think the most recent
version
> was
> http://lists.xensource.com/archives/html/xen-devel/2010-09/msg01329.html
> but perhaps IanJ can confirm. We should incorporate it, either by
> reference or as a separate chapter in the same document or something
> similar.
> 
IanJ - I am also unable to find the txt doc mentioned in the link above.  Please
could you confirm whether or not this txt file is likely to be added, if not why
and whether there are other relevant docs for reference?  I will add then all to
a "Reference" or "Related documents" section.

Kamala


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

On Fri, 28 Jan 2011, Jim Fehlig wrote:
> Currently, IIUC, the external script is invoked by udev
> 
> SUBSYSTEM=="xen-backend", KERNEL=="tap*",
RUN+="/etc/xen/scripts/blktap
> $env{ACTION}"
> SUBSYSTEM=="xen-backend", KERNEL=="vbd*",
RUN+="/etc/xen/scripts/block
> $env{ACTION}"
> 
> The block script will delegate to a helper if needed, which configures
> the block device and writes some info (like resulting dev node) to
> xenstore.  xend and/or qemu then read the info and continue configuring
> the device from backend perspective.
> 
When we add support for this to xl we are probably going to execute
these scripts directly. Having udev doing it for us doesn''t simplify
things.

> > It shouldn''t be too difficult but still non-trivial at this
point of the
> > release cycle.
> > Maybe users could work around the lack of support in 4.1 calling the
> > script themselves and using the name of the block device in the VM
> > config file directly?
> >   
> 
> Yes, this is certainly a workaround.  One could argue this is the proper
> solution and remove support for external block scripts.  But this
> functionality has been around for years.
I think it is the best we can do for 4.1, given the timeframe available.

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

Jim Fehlig writes ("Re: [Xen-devel] [PATCH] xl: Special case tap/aio for
disk validation"):
> The xend toolstack supports arbitrary external block device types.  We
> have block-iscsi and block-npiv,  and the drbd project provides
> block-drbd.  I''m quite sure I''ve seen other custom block
types as well.
> Will these arbitrary block device types still be supported in the new
> toolstack?
Yes, but I think we will have to change the interface to the block
scripts.  The current arrangements are baroque to say the least.  So
probably not in 4.1''s xl, sorry.

Ian.

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

Kamala Narasimhan writes ("Re: [Xen-devel] [PATCH] xl: Special case tap/aio
for disk validation"):
> Attached is a first stab at this for your perusal.  It is mostly
> based on the discussion we had in this thread.  Please let me know
> any/all changes you would recommend.
Thanks.  I concur with Ian Campbell''s comments, but I have some of my
own:
> At a higher level, xl disk configuration option takes the following format:
> 
>     disk = [ ''format:path,vdev:type,attrib'',
''format:path,vdev:type,attrib'', ... ]
I think Ian''s comments about using [ ] for optional elements are very
apposite.  If it''s too confusing because of the surrounding
pythonesque [ ] you could try something like
   disk = [ ''disk-spec'', ''disk-spec'' ]
   where disk-spec is
   [format:]path,vdev[:type][,attribute]
(or whatever is actually true).

Also as well as the doc needing to stay within 70-75 columns, and
certainly within 79, it would be sensible for your emails to do so too
as it makes them easier to read in "traditional" clients.

> Description:         Block device or image file path.  For a physical block
device a /dev will be prepended when not specified and when the path
doesn''t start with a ''/''.
> Supported values:    N/A
> Deprecated values:   N/A
> Mandatory:           Yes
I think in fact it''s not mandatory because for removeable devices it
can be specified as empty to mean "removed".

> Description:         Virtual device as seen by the guest (also referred to
as guest drive designation in some specifications).
It would be sensible refer here to my virtual disk specification doc.


On which subject you wrote:
> IanJ - I am also unable to find the txt doc mentioned in the link
> above.  Please could you confirm whether or not this txt file is
> likely to be added, if not why and whether there are other relevant
> docs for reference?  I will add then all to a "Reference" or
> "Related documents" section.
The document in that url which Ian refers to
  http://lists.xensource.com/archives/html/xen-devel/2010-09/msg01329.html
was never checked into the Xen tree.

There was a bit of discussion about it but I never got a clear ack.
If someone would like to give me an ack now I will commit it :-).
> Supported values:    hd[x], xvd[x]
As you''ll see from the doc, it''s a bit more complicated than
that.


Thanks,
Ian.

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

Stefano Stabellini writes ("[Xen-devel] xl: drdb
support"):
> On Thu, 27 Jan 2011, Kamala Narasimhan wrote:
> > Other than hd[x] and xvd[x], are there other supported VDEV formats?
This is not the case.  There are numeric formats of various kinds,
too, and also sd[x].

Ian.

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

On Fri, 28 Jan 2011, Ian Jackson wrote:
> Stefano Stabellini writes ("[Xen-devel] xl: drdb support"):
> > On Thu, 27 Jan 2011, Kamala Narasimhan wrote:
> > > Other than hd[x] and xvd[x], are there other supported VDEV
formats?
> 
> This is not the case.  There are numeric formats of various kinds,
> too, and also sd[x].
 
sd[x] doesn''t work at the moment, so I wouldn''t add it to the
document.
When (and if) it gets fixed then we can add it.

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

Stefano Stabellini writes ("Re: [Xen-devel] xl: drdb
support"):
> sd[x] doesn''t work at the moment, so I wouldn''t add it to
the document.
> When (and if) it gets fixed then we can add it.
What you mean is that it doesn''t work for HVM guests with current
versions of qemu.

However it _does_ work with at least some PV guests, for example
Linux 2.6.18.  Therefore it should be listed.

Ian.

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

On Fri, 28 Jan 2011, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] xl: drdb support"):
> > sd[x] doesn''t work at the moment, so I wouldn''t add
it to the document.
> > When (and if) it gets fixed then we can add it.
> 
> What you mean is that it doesn''t work for HVM guests with current
> versions of qemu.
> 
> However it _does_ work with at least some PV guests, for example
> Linux 2.6.18.  Therefore it should be listed.
this is a quote from your document:

So for Linux PV guests, users are recommended to supply xvd* devices
only.  Modern PV drivers will map these to identically-named devices
in the guest.


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

Stefano Stabellini writes ("Re: [Xen-devel] xl: drdb
support"):
> this is a quote from your document:
> 
> So for Linux PV guests, users are recommended to supply xvd* devices
> only.  Modern PV drivers will map these to identically-named devices
> in the guest.
Yes.  But existing setups which specify sd* must still work.  Because
we must continue to support the configuration inside those old guests
which expects sd*.

So using sd* with Linux is deprecated but (a) it should still be in
the document and (b) this is a fact which is decided on by those who
develop Linux, not by Xen.

Using sd* with other PV guests is entirely supported and reasonable.
I don''t have a complete inventory of which guests support what.

Ian.

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

I am attaching a newer version of the txt file that incorporates all the review
comments.  Here are the only changes I am yet to make -

1) Refer to IanJ''s virtual disk specification once it is checked-in.
2) Include information pertaining to drbd and other arbitrary block device
types.  This information can be included post 4.1 as I gather from the
discussion on this topic.

Please let me know if I neglected any of your comments or if you have further
comments.

Kamala


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

> 
> On Fri, 28 Jan 2011, Ian Jackson wrote:
> > Stefano Stabellini writes ("[Xen-devel] xl: drdb support"):
> > > On Thu, 27 Jan 2011, Kamala Narasimhan wrote:
> > > > Other than hd[x] and xvd[x], are there other supported VDEV
formats?
> >
> > This is not the case.  There are numeric formats of various kinds,
> > too, and also sd[x].
> 
> sd[x] doesn''t work at the moment, so I wouldn''t add it to
the
document.
> When (and if) it gets fixed then we can add it.
Not even as an "sd[x] doesn''t work at the moment" footnote?

James

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

> 
> On Thu, 27 Jan 2011, Kamala Narasimhan wrote:
> > > - phy:/path/to/device
> > > - file:/path/to/file
> > > - tap:/path/to/file
> >
> > Along with phy, file and tap for block device types there appear to
be a
> ''drdb'' option too.  Do we actively support it?  If so do
we treat it
same as
> ''phy'' and use blkback for backend?
> 
> Yes, I think so. Given his recent work on drdb support in qemu, maybe
> James can comment on this.
> 
Aside from the qemu problems I posted a patch for, drbd works great
under Windows. I haven''t tested live migration (don''t have a
pair of
servers with compatible CPU''s) so I don''t know if that works.
I also
don''t allow multiple-primary as single-primary is the only locking
mechanism I have at this time to make sure the same DomU isn''t started
on two machines concurrently.

James


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

On Sat, Jan 29, 2011 at 12:12:50PM +1100, James Harper
wrote:
> > 
> > On Thu, 27 Jan 2011, Kamala Narasimhan wrote:
> > > > - phy:/path/to/device
> > > > - file:/path/to/file
> > > > - tap:/path/to/file
> > >
> > > Along with phy, file and tap for block device types there appear
to
> be a
> > ''drdb'' option too.  Do we actively support it?  If
so do we treat it
> same as
> > ''phy'' and use blkback for backend?
> > 
> > Yes, I think so. Given his recent work on drdb support in qemu, maybe
> > James can comment on this.
> > 
> 
> Aside from the qemu problems I posted a patch for, drbd works great
> under Windows. I haven''t tested live migration (don''t
have a pair of
> servers with compatible CPU''s) so I don''t know if that
works. I also
> don''t allow multiple-primary as single-primary is the only locking
> mechanism I have at this time to make sure the same DomU isn''t
started
> on two machines concurrently.
> 
IIRC Novell has some additional xend locking patch included
in SLES11 Xen .. I guess it was this one:
http://lists.xensource.com/archives/html/xen-devel/2008-08/msg00196.html
http://serverfault.com/questions/21699/how-to-manage-xen-virtual-machines-on-shared-san-storage

Also Oracle has some locking patch:
http://lists.xensource.com/archives/html/xen-devel/2009-08/msg00101.html

-- Pasi


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

On Fri, 28 Jan 2011, Kamala Narasimhan wrote:
> I am attaching a newer version of the txt file that incorporates all the
review
> comments.  Here are the only changes I am yet to make -
> 
> 1) Refer to IanJ''s virtual disk specification once it is
checked-in.
> 2) Include information pertaining to drbd and other arbitrary block device
> types.  This information can be included post 4.1 as I gather from the
> discussion on this topic.
> 
> Please let me know if I neglected any of your comments or if you have
further
> comments.
 

I think it looks good: I would just unify the two paragraphs under
"Backend Details".

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