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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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