Re: [PATCH v2] introduce a cache options for PV disks

[Ian Campbell <Ian.Campbell@citrix.com>]

On Thu, 2013-06-27 at 17:30 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH v2] introduce a cache options for PV disks"):
> > On Thu, 2013-06-27 at 17:09 +0100, Ian Jackson wrote:
> > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > > index d218a2d..892ab01 100644
> > > --- a/tools/libxl/libxl_types.idl
> > > +++ b/tools/libxl/libxl_types.idl
> > > @@ -375,6 +375,7 @@ libxl_device_disk = Struct("device_disk", [
> > >      ("removable", integer),
> > >      ("readwrite", integer),
> > >      ("is_cdrom", integer),
> > > +    ("direct_io_safe", bool),
> > 
> > You'll want a #define LIBXL_HAVE_DEVICE_DISK_DIRECT_IO_SAFE (or
> > something more wieldy) in libxl.h to enable people to write code which
> > works against different versions of libxl.
> 
> Ah yes.
> 
> > Is this deliberately not a defbool?
> 
> Yes.

OK.

> > > + *
> > > + * direct-io-safe
> > > + *      Values:         1
> > 
> > 0 (==direct-io is unsafe) is explicitly not a valid value?
> 
> Yes.  I could make it permitted.  The obvious parsing code would
> understand it, obviously.

The reason it occurred to me (other than most other similar keys
supporting it) is that you were missing the "Default Value"
documentation, which was only really relevant because I couldn't easily
parse the description in terms of what happens when this option is
absent (or actually present, but there's another subthread about
that ;-)).

> Do I take it you're happy with the general approach ?

Yes.