All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] introduce a cache options for PV disks
@ 2013-06-27 11:40 Stefano Stabellini
  2013-06-27 14:21 ` Ian Jackson
  2013-06-27 16:20 ` Ian Jackson
  0 siblings, 2 replies; 21+ messages in thread
From: Stefano Stabellini @ 2013-06-27 11:40 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell, Stefano Stabellini

Document a per-disk cache option in the xl config file to allow users to
select the cache mode that the backend should use to open the disk file
or device.

Document a "cache" xenstore backend option, part of the vbd xenstore
interface.

Add a reference to xen/include/public/io/blkif.h in
docs/misc/vbd-interface.txt.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


diff --git a/docs/misc/vbd-interface.txt b/docs/misc/vbd-interface.txt
index 3952e73..f873db0 100644
--- a/docs/misc/vbd-interface.txt
+++ b/docs/misc/vbd-interface.txt
@@ -125,3 +125,9 @@ because they directly map the bottom 8 bits of the xenstore integer
 directly to the Linux guest's device number and throw away the rest;
 they can crash due to minor number clashes.  With these guests, the
 workaround is not to supply problematic combinations of devices.
+
+
+Other frontend and backend options
+----------------------------------
+
+See xen/include/public/io/blkif.h for the full list of options.
diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt
index 5bd456d..51d376f 100644
--- a/docs/misc/xl-disk-configuration.txt
+++ b/docs/misc/xl-disk-configuration.txt
@@ -178,6 +178,25 @@ information to be interpreted by the executable program <script>,
 These scripts are normally called "block-<script>".
 
 
+cache=<cache>
+-------------
+
+Description:           Specifies what type of disk cache to use in the backend
+Supported values:      writeback, writethrough, none
+Mandatory:             No
+Default value:         depends on the backend type
+
+This does not affect the guest's view of the device. 
+Not all backends implement all cache modes: "phy" only supports "none",
+"qdisk" supports all of them and defaults to "writeback".
+It's important to note that if you are storing the VM disk on a network
+filesystem or a network block device (NFS or ISCSI) it might not be safe
+to use "none", see
+http://lists.xen.org/archives/html/xen-devel/2012-12/msg01154.html.
+Otherwise using "none" is safe and gives you better performances.
+
+
+
 
 ============================================
 DEPRECATED PARAMETERS, PREFIXES AND SYNTAXES
diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
index b9b9d98..f4a271c 100644
--- a/xen/include/public/io/blkif.h
+++ b/xen/include/public/io/blkif.h
@@ -97,6 +97,16 @@
  *
  *      The type of the backing device/object.
  *
+ *
+ * cache
+ *      Values:         "none", "writeback", "writethrough"
+ *      Optional
+ *
+ *      The cache mode that the backend should use to open the backing
+ *      device/object. The backend is not required to support all the
+ *      possible cache modes. If an unsupported cache is requested, the
+ *      backend is free to ignore it.
+ *
  *--------------------------------- Features ---------------------------------
  *
  * feature-barrier

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 11:40 [PATCH v2] introduce a cache options for PV disks Stefano Stabellini
@ 2013-06-27 14:21 ` Ian Jackson
  2013-06-27 16:09   ` Ian Jackson
  2013-06-27 16:20 ` Ian Jackson
  1 sibling, 1 reply; 21+ messages in thread
From: Ian Jackson @ 2013-06-27 14:21 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, Ian Campbell

Stefano Stabellini writes ("[PATCH v2] introduce a cache options for PV disks"):
> +cache=<cache>
> +-------------

Following a discussion with Stefano, we're going to rework this
somewhat.  The problem with this "cache=..." option is that it gives
the impression that it should do a lot more than it actually does.
For example, people might try to set cache=none because they know or
suspect their guest is being loose with its barriers, or something.
Or they might want to try to stop their host buffer cache being
swamped by an unfriendly guest's IDE IO.

To avoid these problems we intend to rename this option and change the
documentation accordingly.  Probably, to "direct-io-safe" (both in
Xenstore and the libxl API).  The documentation will say that this
option just disables any workaround where we avoid using direct IO
because of bugs.

Ian.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 14:21 ` Ian Jackson
@ 2013-06-27 16:09   ` Ian Jackson
  2013-06-27 16:24     ` Ian Campbell
  0 siblings, 1 reply; 21+ messages in thread
From: Ian Jackson @ 2013-06-27 16:09 UTC (permalink / raw)
  To: Stefano Stabellini, xen-devel, Ian Campbell

Ian Jackson writes ("Re: [PATCH v2] introduce a cache options for PV disks"):
> To avoid these problems we intend to rename this option and change the
> documentation accordingly.  Probably, to "direct-io-safe" (both in
> Xenstore and the libxl API).  The documentation will say that this
> option just disables any workaround where we avoid using direct IO
> because of bugs.

Here's an RFC version of the libxl part of this patch.  It compiles
but has not been tested.

For clarity the patch below doesn't include the rebuild of
libxlu_disk_l.[ch].

Ian.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff --git a/docs/misc/vbd-interface.txt b/docs/misc/vbd-interface.txt
index 3952e73..f873db0 100644
--- a/docs/misc/vbd-interface.txt
+++ b/docs/misc/vbd-interface.txt
@@ -125,3 +125,9 @@ because they directly map the bottom 8 bits of the xenstore integer
 directly to the Linux guest's device number and throw away the rest;
 they can crash due to minor number clashes.  With these guests, the
 workaround is not to supply problematic combinations of devices.
+
+
+Other frontend and backend options
+----------------------------------
+
+See xen/include/public/io/blkif.h for the full list of options.
diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt
index 5bd456d..9c2650b 100644
--- a/docs/misc/xl-disk-configuration.txt
+++ b/docs/misc/xl-disk-configuration.txt
@@ -178,6 +178,44 @@ information to be interpreted by the executable program <script>,
 These scripts are normally called "block-<script>".
 
 
+direct-io-safe
+--------------
+
+Description:           Disables non-O_DIRECT workaround
+Supported values:      absent, present
+Mandatory:             No
+Default value:         absent (workaround may be enabled)
+
+There is a memory lifetime bug in some driver domain (dom0) kernels
+which can cause crashes when using O_DIRECT.  The bug occurs due to a
+mismatch between the backend-visible lifetime of pages used for the
+Xen PV network protocol and that expected by the backend kernel's
+networking subsystem.  This can cause crashes when using certain
+backends with certain underlying storage.
+
+See:
+ http://lists.xen.org/archives/html/xen-devel/2012-12/msg01154.html
+
+For this reason, (this version of) the Xen libxl toolstack disables
+O_DIRECT when using the qemu-based Xen PV backend ("qdisk").
+
+However, this workaround has performance and scaling implications, and
+it is only necessary if the underlying device is a network filesystem.
+If the underlying device is not, then it is good to disable it; that
+is what this option is for.
+
+This option simply requests that the workaround be disabled.  (However,
+not all backends versions which use the workaround understand this
+option, so this is on a best effort basis.)
+
+It's important to note that if you are storing the VM disk on a
+network filesystem or a network block device (NFS or ISCSI) it might
+not be safe to use this option.  Otherwise specifying it is safe and
+can give better performances.
+
+If in the future the bug is fixed properly this option will then be
+silently ignored.
+
 
 ============================================
 DEPRECATED PARAMETERS, PREFIXES AND SYNTAXES
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 3236aa9..541c258 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2168,6 +2168,10 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
         flexarray_append(back, disk->readwrite ? "w" : "r");
         flexarray_append(back, "device-type");
         flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
+        if (disk->direct_io_safe) {
+            flexarray_append(back, "direct-io-safe");
+            flexarray_append(back, "1");
+        }
 
         flexarray_append(front, "backend-id");
         flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
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),
     ])
 
 libxl_device_nic = Struct("device_nic", [
diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l
index 7c4e7f1..ba8577c 100644
--- a/tools/libxl/libxlu_disk_l.l
+++ b/tools/libxl/libxlu_disk_l.l
@@ -173,6 +173,7 @@ backendtype=[^,]*,? { STRIP(','); setbackendtype(DPC,FROMEQUALS); }
 
 vdev=[^,]*,?	{ STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); }
 script=[^,]*,?	{ STRIP(','); SAVESTRING("script", script, FROMEQUALS); }
+direct-io-safe,? { DPC->disk->direct_io_safe = 1; }
 
  /* the target magic parameter, eats the rest of the string */
 
diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
index b9b9d98..baf0f0a 100644
--- a/xen/include/public/io/blkif.h
+++ b/xen/include/public/io/blkif.h
@@ -97,6 +97,22 @@
  *
  *      The type of the backing device/object.
  *
+ *
+ * direct-io-safe
+ *      Values:         1
+ *      Optional
+ *
+ *      The underlying storage is not affected by the direct IO memory
+ *      lifetime bug.  See:
+ *        http://lists.xen.org/archives/html/xen-devel/2012-12/msg01154.html
+ *
+ *      So use of O_DIRECT is safe, in circumstances
+ *      where we would normally have avoided it as a workaround for
+ *      that bug.  This option is not relevant for all backends, and
+ *      even not necessarily supported for those for which it is
+ *      relevant.  A backend which knows that it is not affected by
+ *      the bug can ignore this option.
+ *
  *--------------------------------- Features ---------------------------------
  *
  * feature-barrier

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 11:40 [PATCH v2] introduce a cache options for PV disks Stefano Stabellini
  2013-06-27 14:21 ` Ian Jackson
@ 2013-06-27 16:20 ` Ian Jackson
  2013-06-27 17:06   ` Stefano Stabellini
  1 sibling, 1 reply; 21+ messages in thread
From: Ian Jackson @ 2013-06-27 16:20 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, Ian Campbell

Stefano Stabellini writes ("[PATCH v2] introduce a cache options for PV disks"):
> Document a per-disk cache option in the xl config file to allow users to
> select the cache mode that the backend should use to open the disk file
> or device.

Here's an RFD revised version of the qemu patch.  I haven't compiled
it yet.

Ian.

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 247f32f..9b42e7f 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -93,6 +93,7 @@ struct XenBlkDev {
     char                *type;
     char                *dev;
     char                *devtype;
+    bool                directiosafe;
     const char          *fileproto;
     const char          *filename;
     int                 ring_ref;
@@ -701,6 +702,7 @@ static int blk_init(struct XenDevice *xendev)
 {
     struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
     int info = 0;
+    char *directiosafe == NULL;
 
     /* read xenstore entries */
     if (blkdev->params == NULL) {
@@ -733,6 +735,8 @@ static int blk_init(struct XenDevice *xendev)
     if (blkdev->devtype == NULL) {
         blkdev->devtype = xenstore_read_be_str(&blkdev->xendev, "device-type");
     }
+    directiosafe = xenstore_read_be_str(&blkdev->xendev, "direct-io-safe");
+    blkdev->directiosafe = (directiosafe && atoi(directiofsafe));
 
     /* do we have all we need? */
     if (blkdev->params == NULL ||
@@ -760,6 +764,8 @@ static int blk_init(struct XenDevice *xendev)
     xenstore_write_be_int(&blkdev->xendev, "feature-flush-cache", 1);
     xenstore_write_be_int(&blkdev->xendev, "feature-persistent", 1);
     xenstore_write_be_int(&blkdev->xendev, "info", info);
+
+    g_free(directiosafe);
     return 0;
 
 out_error:
@@ -773,6 +779,7 @@ out_error:
     blkdev->dev = NULL;
     g_free(blkdev->devtype);
     blkdev->devtype = NULL;
+    g_free(directiosafe);
     return -1;
 }
 
@@ -784,6 +791,9 @@ static int blk_connect(struct XenDevice *xendev)
 
     /* read-only ? */
     qflags = BDRV_O_CACHE_WB | BDRV_O_NATIVE_AIO;
+    if (blkdev->directiosafe) {
+        qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
+    }
     if (strcmp(blkdev->mode, "w") == 0) {
         qflags |= BDRV_O_RDWR;
         readonly = false;

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 16:09   ` Ian Jackson
@ 2013-06-27 16:24     ` Ian Campbell
  2013-06-27 16:30       ` Ian Jackson
  2013-06-27 16:35       ` Ian Jackson
  0 siblings, 2 replies; 21+ messages in thread
From: Ian Campbell @ 2013-06-27 16:24 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Stefano Stabellini

On Thu, 2013-06-27 at 17:09 +0100, Ian Jackson wrote:
> Ian Jackson writes ("Re: [PATCH v2] introduce a cache options for PV disks"):
> > To avoid these problems we intend to rename this option and change the
> > documentation accordingly.  Probably, to "direct-io-safe" (both in
> > Xenstore and the libxl API).  The documentation will say that this
> > option just disables any workaround where we avoid using direct IO
> > because of bugs.
> 
> Here's an RFC version of the libxl part of this patch.  It compiles
> but has not been tested.
> 
> For clarity the patch below doesn't include the rebuild of
> libxlu_disk_l.[ch].
> 
> Ian.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> diff --git a/docs/misc/vbd-interface.txt b/docs/misc/vbd-interface.txt
> index 3952e73..f873db0 100644
> --- a/docs/misc/vbd-interface.txt
> +++ b/docs/misc/vbd-interface.txt
> @@ -125,3 +125,9 @@ because they directly map the bottom 8 bits of the xenstore integer
>  directly to the Linux guest's device number and throw away the rest;
>  they can crash due to minor number clashes.  With these guests, the
>  workaround is not to supply problematic combinations of devices.
> +
> +
> +Other frontend and backend options
> +----------------------------------
> +
> +See xen/include/public/io/blkif.h for the full list of options.
> diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt
> index 5bd456d..9c2650b 100644
> --- a/docs/misc/xl-disk-configuration.txt
> +++ b/docs/misc/xl-disk-configuration.txt
> @@ -178,6 +178,44 @@ information to be interpreted by the executable program <script>,
>  These scripts are normally called "block-<script>".
>  
> 
> +direct-io-safe
> +--------------
> +
> +Description:           Disables non-O_DIRECT workaround
> +Supported values:      absent, present
> +Mandatory:             No
> +Default value:         absent (workaround may be enabled)
> +
> +There is a memory lifetime bug in some driver domain (dom0) kernels
> +which can cause crashes when using O_DIRECT.  The bug occurs due to a
> +mismatch between the backend-visible lifetime of pages used for the
> +Xen PV network protocol and that expected by the backend kernel's
> +networking subsystem.  This can cause crashes when using certain
> +backends with certain underlying storage.
> +
> +See:
> + http://lists.xen.org/archives/html/xen-devel/2012-12/msg01154.html
> +
> +For this reason, (this version of) the Xen libxl toolstack disables
> +O_DIRECT when using the qemu-based Xen PV backend ("qdisk").
> +
> +However, this workaround has performance and scaling implications, and
> +it is only necessary if the underlying device is a network filesystem.
> +If the underlying device is not, then it is good to disable it; that
> +is what this option is for.
> +
> +This option simply requests that the workaround be disabled.  (However,
> +not all backends versions which use the workaround understand this
> +option, so this is on a best effort basis.)
> +
> +It's important to note that if you are storing the VM disk on a
> +network filesystem or a network block device (NFS or ISCSI) it might
> +not be safe to use this option.  Otherwise specifying it is safe and
> +can give better performances.
> +
> +If in the future the bug is fixed properly this option will then be
> +silently ignored.
> +
>  
>  ============================================
>  DEPRECATED PARAMETERS, PREFIXES AND SYNTAXES
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 3236aa9..541c258 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2168,6 +2168,10 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
>          flexarray_append(back, disk->readwrite ? "w" : "r");
>          flexarray_append(back, "device-type");
>          flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
> +        if (disk->direct_io_safe) {
> +            flexarray_append(back, "direct-io-safe");
> +            flexarray_append(back, "1");
> +        }
>  
>          flexarray_append(front, "backend-id");
>          flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
> 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.

Is this deliberately not a defbool?

>      ])
>  
>  libxl_device_nic = Struct("device_nic", [
> diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l
> index 7c4e7f1..ba8577c 100644
> --- a/tools/libxl/libxlu_disk_l.l
> +++ b/tools/libxl/libxlu_disk_l.l
> @@ -173,6 +173,7 @@ backendtype=[^,]*,? { STRIP(','); setbackendtype(DPC,FROMEQUALS); }
>  
>  vdev=[^,]*,?	{ STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); }
>  script=[^,]*,?	{ STRIP(','); SAVESTRING("script", script, FROMEQUALS); }
> +direct-io-safe,? { DPC->disk->direct_io_safe = 1; }
>  
>   /* the target magic parameter, eats the rest of the string */
>  
> diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
> index b9b9d98..baf0f0a 100644
> --- a/xen/include/public/io/blkif.h
> +++ b/xen/include/public/io/blkif.h
> @@ -97,6 +97,22 @@
>   *
>   *      The type of the backing device/object.
>   *
> + *
> + * direct-io-safe
> + *      Values:         1

0 (==direct-io is unsafe) is explicitly not a valid value?

> + *      Optional
> + *
> + *      The underlying storage is not affected by the direct IO memory
> + *      lifetime bug.  See:
> + *        http://lists.xen.org/archives/html/xen-devel/2012-12/msg01154.html
> + *
> + *      So use of O_DIRECT is safe, in circumstances

           ^Some ?

Otherwise I can't parse this sentence.

> + *      where we would normally have avoided it as a workaround for
> + *      that bug.  This option is not relevant for all backends, and
> + *      even not necessarily supported for those for which it is
> + *      relevant.  A backend which knows that it is not affected by
> + *      the bug can ignore this option.
> + *
>   *--------------------------------- Features ---------------------------------
>   *
>   * feature-barrier

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 16:24     ` Ian Campbell
@ 2013-06-27 16:30       ` Ian Jackson
  2013-06-27 16:34         ` Ian Jackson
  2013-06-27 16:41         ` Ian Campbell
  2013-06-27 16:35       ` Ian Jackson
  1 sibling, 2 replies; 21+ messages in thread
From: Ian Jackson @ 2013-06-27 16:30 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Stefano Stabellini

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.

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

> > + *      So use of O_DIRECT is safe, in circumstances
> 
>            ^Some ?
> 
> Otherwise I can't parse this sentence.

Yes.

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

Ian.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 16:30       ` Ian Jackson
@ 2013-06-27 16:34         ` Ian Jackson
  2013-06-27 16:41           ` Ian Campbell
  2013-06-27 16:41         ` Ian Campbell
  1 sibling, 1 reply; 21+ messages in thread
From: Ian Jackson @ 2013-06-27 16:34 UTC (permalink / raw)
  To: Ian Campbell, Stefano Stabellini, xen-devel

Ian Jackson writes ("Re: [PATCH v2] introduce a cache options for PV disks"):
> 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:
> > > + *      So use of O_DIRECT is safe, in circumstances
> > 
> >            ^Some ?
> > 
> > Otherwise I can't parse this sentence.
> 
> Yes.

In, fact, no.  How about:

 *      That is, if this option is enabled, use of O_DIRECT is safe,
 *      in circumstances where we would normally have avoided it as a
 *      workaround for that bug.  This option is not relevant for ...

Ian.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 16:24     ` Ian Campbell
  2013-06-27 16:30       ` Ian Jackson
@ 2013-06-27 16:35       ` Ian Jackson
  1 sibling, 0 replies; 21+ messages in thread
From: Ian Jackson @ 2013-06-27 16:35 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Ian Jackson, Stefano Stabellini

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/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
> > index b9b9d98..baf0f0a 100644
> > --- a/xen/include/public/io/blkif.h
> > +++ b/xen/include/public/io/blkif.h
> > @@ -97,6 +97,22 @@
> >   *
> >   *      The type of the backing device/object.
> >   *
> > + *
> > + * direct-io-safe
> > + *      Values:         1
> 
> 0 (==direct-io is unsafe) is explicitly not a valid value?

Looking around a bit, the other things in xenstore do this differently
and it should be consistent.  So I will change this to 0/1, defaulting
to 0.

Ian.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 16:30       ` Ian Jackson
  2013-06-27 16:34         ` Ian Jackson
@ 2013-06-27 16:41         ` Ian Campbell
  1 sibling, 0 replies; 21+ messages in thread
From: Ian Campbell @ 2013-06-27 16:41 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Stefano Stabellini

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.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 16:34         ` Ian Jackson
@ 2013-06-27 16:41           ` Ian Campbell
  2013-06-27 16:50             ` Ian Jackson
  0 siblings, 1 reply; 21+ messages in thread
From: Ian Campbell @ 2013-06-27 16:41 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Stefano Stabellini

On Thu, 2013-06-27 at 17:34 +0100, Ian Jackson wrote:
> Ian Jackson writes ("Re: [PATCH v2] introduce a cache options for PV disks"):
> > 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:
> > > > + *      So use of O_DIRECT is safe, in circumstances
> > > 
> > >            ^Some ?
> > > 
> > > Otherwise I can't parse this sentence.
> > 
> > Yes.
> 
> In, fact, no.  How about:
> 
>  *      That is, if this option is enabled, use of O_DIRECT is safe,
>  *      in circumstances where we would normally have avoided it as a
>  *      workaround for that bug.  This option is not relevant for ...

I think I can parse it now. It doesn't really say what the option does
though. I'd expect it to say something about "disabling the workaround"
or "the backend may use O_DIRECT".

Ian.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 16:41           ` Ian Campbell
@ 2013-06-27 16:50             ` Ian Jackson
  2013-06-27 16:58               ` Ian Campbell
  0 siblings, 1 reply; 21+ messages in thread
From: Ian Jackson @ 2013-06-27 16:50 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Stefano Stabellini

Ian Campbell writes ("Re: [PATCH v2] introduce a cache options for PV disks"):
> I think I can parse it now. It doesn't really say what the option does
> though. I'd expect it to say something about "disabling the workaround"
> or "the backend may use O_DIRECT".

How about:

 * direct-io-safe
 *      Values:         0/1 (boolean)
 *      Default Value:  0
 *
 *      The underlying storage is not affected by the direct IO memory
 *      lifetime bug.  See:
 *        http://lists.xen.org/archives/html/xen-devel/2012-12/msg01154.html
 *
 *      Therefore this option gives the backend permission to use
 *      O_DIRECT, notwithstanding that bug.
 *
 *      That is, if this option is enabled, use of O_DIRECT is safe,
 *      in circumstances where we would normally have avoided it as a
 *      workaround for that bug.  This option is not relevant for all
 *      backends, and even not necessarily supported for those for
 *      which it is relevant.  A backend which knows that it is not
 *      affected by the bug can ignore this option.
 *
 *      This option doesn't require a backend to use O_DIRECT, so it
 *      should not be used to try to control the caching behaviour.

Ian.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 16:50             ` Ian Jackson
@ 2013-06-27 16:58               ` Ian Campbell
  0 siblings, 0 replies; 21+ messages in thread
From: Ian Campbell @ 2013-06-27 16:58 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Stefano Stabellini

On Thu, 2013-06-27 at 17:50 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH v2] introduce a cache options for PV disks"):
> > I think I can parse it now. It doesn't really say what the option does
> > though. I'd expect it to say something about "disabling the workaround"
> > or "the backend may use O_DIRECT".
> 
> How about:

Sounds good thanks.

> 
>  * direct-io-safe
>  *      Values:         0/1 (boolean)
>  *      Default Value:  0
>  *
>  *      The underlying storage is not affected by the direct IO memory
>  *      lifetime bug.  See:
>  *        http://lists.xen.org/archives/html/xen-devel/2012-12/msg01154.html
>  *
>  *      Therefore this option gives the backend permission to use
>  *      O_DIRECT, notwithstanding that bug.
>  *
>  *      That is, if this option is enabled, use of O_DIRECT is safe,
>  *      in circumstances where we would normally have avoided it as a
>  *      workaround for that bug.  This option is not relevant for all
>  *      backends, and even not necessarily supported for those for
>  *      which it is relevant.  A backend which knows that it is not
>  *      affected by the bug can ignore this option.
>  *
>  *      This option doesn't require a backend to use O_DIRECT, so it
>  *      should not be used to try to control the caching behaviour.
> 
> Ian.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 16:20 ` Ian Jackson
@ 2013-06-27 17:06   ` Stefano Stabellini
  2013-06-27 17:13     ` Stefano Stabellini
  2013-06-27 17:44     ` Ian Jackson
  0 siblings, 2 replies; 21+ messages in thread
From: Stefano Stabellini @ 2013-06-27 17:06 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Ian Campbell, Stefano Stabellini

On Thu, 27 Jun 2013, Ian Jackson wrote:
> Stefano Stabellini writes ("[PATCH v2] introduce a cache options for PV disks"):
> > Document a per-disk cache option in the xl config file to allow users to
> > select the cache mode that the backend should use to open the disk file
> > or device.
> 
> Here's an RFD revised version of the qemu patch.  I haven't compiled
> it yet.
> 

It's mostly OK, just few minor corrections for code readability


> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 247f32f..9b42e7f 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -93,6 +93,7 @@ struct XenBlkDev {
>      char                *type;
>      char                *dev;
>      char                *devtype;
> +    bool                directiosafe;
>      const char          *fileproto;
>      const char          *filename;
>      int                 ring_ref;
> @@ -701,6 +702,7 @@ static int blk_init(struct XenDevice *xendev)
>  {
>      struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
>      int info = 0;
> +    char *directiosafe == NULL;
                           ^ =

>      /* read xenstore entries */
>      if (blkdev->params == NULL) {
> @@ -733,6 +735,8 @@ static int blk_init(struct XenDevice *xendev)
>      if (blkdev->devtype == NULL) {
>          blkdev->devtype = xenstore_read_be_str(&blkdev->xendev, "device-type");
>      }
> +    directiosafe = xenstore_read_be_str(&blkdev->xendev, "direct-io-safe");
> +    blkdev->directiosafe = (directiosafe && atoi(directiofsafe));
>  
>      /* do we have all we need? */
>      if (blkdev->params == NULL ||
> @@ -760,6 +764,8 @@ static int blk_init(struct XenDevice *xendev)
>      xenstore_write_be_int(&blkdev->xendev, "feature-flush-cache", 1);
>      xenstore_write_be_int(&blkdev->xendev, "feature-persistent", 1);
>      xenstore_write_be_int(&blkdev->xendev, "info", info);
> +
> +    g_free(directiosafe);
>      return 0;
>  
>  out_error:
> @@ -773,6 +779,7 @@ out_error:
>      blkdev->dev = NULL;
>      g_free(blkdev->devtype);
>      blkdev->devtype = NULL;
> +    g_free(directiosafe);

maybe add

blkdev->directiosafe = false;


>      return -1;
>  }
>  
> @@ -784,6 +791,9 @@ static int blk_connect(struct XenDevice *xendev)
>  
>      /* read-only ? */
>      qflags = BDRV_O_CACHE_WB | BDRV_O_NATIVE_AIO;
> +    if (blkdev->directiosafe) {
> +        qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
> +    }

Please change this into:

if (blkdev->directiosafe) {
    qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
} else {
    qflags = BDRV_O_CACHE_WB;
}

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 17:06   ` Stefano Stabellini
@ 2013-06-27 17:13     ` Stefano Stabellini
  2013-06-27 17:44     ` Ian Jackson
  1 sibling, 0 replies; 21+ messages in thread
From: Stefano Stabellini @ 2013-06-27 17:13 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, Ian Jackson, Ian Campbell

On Thu, 27 Jun 2013, Stefano Stabellini wrote:
> On Thu, 27 Jun 2013, Ian Jackson wrote:
> > Stefano Stabellini writes ("[PATCH v2] introduce a cache options for PV disks"):
> > > Document a per-disk cache option in the xl config file to allow users to
> > > select the cache mode that the backend should use to open the disk file
> > > or device.
> > 
> > Here's an RFD revised version of the qemu patch.  I haven't compiled
> > it yet.
> > 
> 
> It's mostly OK, just few minor corrections for code readability

BTW are you going to follow up or do you want me to?

> 
> > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> > index 247f32f..9b42e7f 100644
> > --- a/hw/block/xen_disk.c
> > +++ b/hw/block/xen_disk.c
> > @@ -93,6 +93,7 @@ struct XenBlkDev {
> >      char                *type;
> >      char                *dev;
> >      char                *devtype;
> > +    bool                directiosafe;
> >      const char          *fileproto;
> >      const char          *filename;
> >      int                 ring_ref;
> > @@ -701,6 +702,7 @@ static int blk_init(struct XenDevice *xendev)
> >  {
> >      struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
> >      int info = 0;
> > +    char *directiosafe == NULL;
>                            ^ =
> 
> >      /* read xenstore entries */
> >      if (blkdev->params == NULL) {
> > @@ -733,6 +735,8 @@ static int blk_init(struct XenDevice *xendev)
> >      if (blkdev->devtype == NULL) {
> >          blkdev->devtype = xenstore_read_be_str(&blkdev->xendev, "device-type");
> >      }
> > +    directiosafe = xenstore_read_be_str(&blkdev->xendev, "direct-io-safe");
> > +    blkdev->directiosafe = (directiosafe && atoi(directiofsafe));
> >  
> >      /* do we have all we need? */
> >      if (blkdev->params == NULL ||
> > @@ -760,6 +764,8 @@ static int blk_init(struct XenDevice *xendev)
> >      xenstore_write_be_int(&blkdev->xendev, "feature-flush-cache", 1);
> >      xenstore_write_be_int(&blkdev->xendev, "feature-persistent", 1);
> >      xenstore_write_be_int(&blkdev->xendev, "info", info);
> > +
> > +    g_free(directiosafe);
> >      return 0;
> >  
> >  out_error:
> > @@ -773,6 +779,7 @@ out_error:
> >      blkdev->dev = NULL;
> >      g_free(blkdev->devtype);
> >      blkdev->devtype = NULL;
> > +    g_free(directiosafe);
> 
> maybe add
> 
> blkdev->directiosafe = false;
> 
> 
> >      return -1;
> >  }
> >  
> > @@ -784,6 +791,9 @@ static int blk_connect(struct XenDevice *xendev)
> >  
> >      /* read-only ? */
> >      qflags = BDRV_O_CACHE_WB | BDRV_O_NATIVE_AIO;
> > +    if (blkdev->directiosafe) {
> > +        qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
> > +    }
> 
> Please change this into:
> 
> if (blkdev->directiosafe) {
>     qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
> } else {
>     qflags = BDRV_O_CACHE_WB;
> }
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 17:06   ` Stefano Stabellini
  2013-06-27 17:13     ` Stefano Stabellini
@ 2013-06-27 17:44     ` Ian Jackson
  2013-06-27 17:50       ` Stefano Stabellini
  1 sibling, 1 reply; 21+ messages in thread
From: Ian Jackson @ 2013-06-27 17:44 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, Ian Campbell

Stefano Stabellini writes ("Re: [PATCH v2] introduce a cache options for PV disks"):
> It's mostly OK, just few minor corrections for code readability

Thanks.

> >      int info = 0;
> > +    char *directiosafe == NULL;
>                            ^ =

I did warn that I hadn't compiled it :-).

> >  out_error:
> > @@ -773,6 +779,7 @@ out_error:
> >      blkdev->dev = NULL;
> >      g_free(blkdev->devtype);
> >      blkdev->devtype = NULL;
> > +    g_free(directiosafe);
> 
> maybe add
> 
> blkdev->directiosafe = false;

Sure, does no harm.

> > +    if (blkdev->directiosafe) {
> > +        qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
> > +    }
> 
> Please change this into:
> 
> if (blkdev->directiosafe) {
>     qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
> } else {
>     qflags = BDRV_O_CACHE_WB;
> }

I don't think that can be right.  The result would be that the value
non-directiosafe value of qflags is changed.  But I will break it
apart as you suggest.

Ian.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 17:44     ` Ian Jackson
@ 2013-06-27 17:50       ` Stefano Stabellini
  2013-06-27 17:59         ` Ian Jackson
  2013-06-27 18:01         ` Ian Jackson
  0 siblings, 2 replies; 21+ messages in thread
From: Stefano Stabellini @ 2013-06-27 17:50 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Ian Campbell, Stefano Stabellini

On Thu, 27 Jun 2013, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [PATCH v2] introduce a cache options for PV disks"):
> > It's mostly OK, just few minor corrections for code readability
> 
> Thanks.
> 
> > >      int info = 0;
> > > +    char *directiosafe == NULL;
> >                            ^ =
> 
> I did warn that I hadn't compiled it :-).
> 
> > >  out_error:
> > > @@ -773,6 +779,7 @@ out_error:
> > >      blkdev->dev = NULL;
> > >      g_free(blkdev->devtype);
> > >      blkdev->devtype = NULL;
> > > +    g_free(directiosafe);
> > 
> > maybe add
> > 
> > blkdev->directiosafe = false;
> 
> Sure, does no harm.
> 
> > > +    if (blkdev->directiosafe) {
> > > +        qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
> > > +    }
> > 
> > Please change this into:
> > 
> > if (blkdev->directiosafe) {
> >     qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
> > } else {
> >     qflags = BDRV_O_CACHE_WB;
> > }
> 
> I don't think that can be right.  The result would be that the value
> non-directiosafe value of qflags is changed.  But I will break it
> apart as you suggest.

BDRV_O_NATIVE_AIO is ignored if BDRV_O_NOCACHE is not also set, so there
would be not behavioural change. However the code would be clearer.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 17:50       ` Stefano Stabellini
@ 2013-06-27 17:59         ` Ian Jackson
  2013-06-27 18:01         ` Ian Jackson
  1 sibling, 0 replies; 21+ messages in thread
From: Ian Jackson @ 2013-06-27 17:59 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, Ian Campbell

Stefano Stabellini writes ("Re: [PATCH v2] introduce a cache options for PV disks"):
> BDRV_O_NATIVE_AIO is ignored if BDRV_O_NOCACHE is not also set, so there
> would be not behavioural change. However the code would be clearer.

Right, OK.

Ian.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 17:50       ` Stefano Stabellini
  2013-06-27 17:59         ` Ian Jackson
@ 2013-06-27 18:01         ` Ian Jackson
  2013-06-27 18:04           ` Ian Jackson
  2013-06-27 18:12           ` Stefano Stabellini
  1 sibling, 2 replies; 21+ messages in thread
From: Ian Jackson @ 2013-06-27 18:01 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, Ian Campbell

Here's the result.  I took Anthony Ligouri's ack off it.

Ian.

commit 21538601f7d4f2de4d518b92ec9212c99be829e8
Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Date:   Thu Jun 27 14:20:51 2013 +0000

    xen_disk: support "direct-io-safe" backend option
    
    Support backend option "direct-io-safe".  This is documented as
    follows in the Xen backend specification:
    
     * direct-io-safe
     *      Values:         0/1 (boolean)
     *      Default Value:  0
     *
     *      The underlying storage is not affected by the direct IO memory
     *      lifetime bug.  See:
     *        http://lists.xen.org/archives/html/xen-devel/2012-12/msg01154.html
     *
     *      Therefore this option gives the backend permission to use
     *      O_DIRECT, notwithstanding that bug.
     *
     *      That is, if this option is enabled, use of O_DIRECT is safe,
     *      in circumstances where we would normally have avoided it as a
     *      workaround for that bug.  This option is not relevant for all
     *      backends, and even not necessarily supported for those for
     *      which it is relevant.  A backend which knows that it is not
     *      affected by the bug can ignore this option.
     *
     *      This option doesn't require a backend to use O_DIRECT, so it
     *      should not be used to try to control the caching behaviour.
    
    Also, BDRV_O_NATIVE_AIO is ignored if BDRV_O_NOCACHE, so clarify the
    default flags passed to the qemu block layer.
    
    Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 247f32f..091284b 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -93,6 +93,7 @@ struct XenBlkDev {
     char                *type;
     char                *dev;
     char                *devtype;
+    bool                directiosafe;
     const char          *fileproto;
     const char          *filename;
     int                 ring_ref;
@@ -701,6 +702,7 @@ static int blk_init(struct XenDevice *xendev)
 {
     struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
     int info = 0;
+    char *directiosafe = NULL;
 
     /* read xenstore entries */
     if (blkdev->params == NULL) {
@@ -733,6 +735,8 @@ static int blk_init(struct XenDevice *xendev)
     if (blkdev->devtype == NULL) {
         blkdev->devtype = xenstore_read_be_str(&blkdev->xendev, "device-type");
     }
+    directiosafe = xenstore_read_be_str(&blkdev->xendev, "direct-io-safe");
+    blkdev->directiosafe = (directiosafe && atoi(directiofsafe));
 
     /* do we have all we need? */
     if (blkdev->params == NULL ||
@@ -760,6 +764,8 @@ static int blk_init(struct XenDevice *xendev)
     xenstore_write_be_int(&blkdev->xendev, "feature-flush-cache", 1);
     xenstore_write_be_int(&blkdev->xendev, "feature-persistent", 1);
     xenstore_write_be_int(&blkdev->xendev, "info", info);
+
+    g_free(directiosafe);
     return 0;
 
 out_error:
@@ -773,6 +779,8 @@ out_error:
     blkdev->dev = NULL;
     g_free(blkdev->devtype);
     blkdev->devtype = NULL;
+    g_free(directiosafe);
+    blkdev->directiosafe = false;
     return -1;
 }
 
@@ -783,7 +791,11 @@ static int blk_connect(struct XenDevice *xendev)
     bool readonly = true;
 
     /* read-only ? */
-    qflags = BDRV_O_CACHE_WB | BDRV_O_NATIVE_AIO;
+    if (blkdev->directiosafe) {
+        qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
+    } else {
+        qflags = BDRV_O_CACHE_WB;
+    }
     if (strcmp(blkdev->mode, "w") == 0) {
         qflags |= BDRV_O_RDWR;
         readonly = false;

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 18:01         ` Ian Jackson
@ 2013-06-27 18:04           ` Ian Jackson
  2013-06-28  7:39             ` Ian Campbell
  2013-06-27 18:12           ` Stefano Stabellini
  1 sibling, 1 reply; 21+ messages in thread
From: Ian Jackson @ 2013-06-27 18:04 UTC (permalink / raw)
  To: Stefano Stabellini, xen-devel, Ian Campbell

Ian Jackson writes ("Re: [PATCH v2] introduce a cache options for PV disks"):
> Here's the result.  I took Anthony Ligouri's ack off it.

And the libxl half.

commit 98c0ba29e33ae875e8b854fa1ff58862ac09a530
Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Date:   Thu Jun 27 12:40:20 2013 +0100

    libxl: introduce an option for disabling the non-O_DIRECT workaround
    
    Document and implement a new option that permits disk backends which
    would otherwise have to avoid O_DIRECT (because of the network memory
    lifetime bug) to use it anyway.  This is:
     direct-io-safe   in the xl domain disk config specification
     direct_io_safe   in the libxl disk API
     direct-io-safe   in the backend xenstore interface
    
    Add a reference to xen/include/public/io/blkif.h in
    docs/misc/vbd-interface.txt.
    
    Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
    Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff --git a/docs/misc/vbd-interface.txt b/docs/misc/vbd-interface.txt
index 3952e73..f873db0 100644
--- a/docs/misc/vbd-interface.txt
+++ b/docs/misc/vbd-interface.txt
@@ -125,3 +125,9 @@ because they directly map the bottom 8 bits of the xenstore integer
 directly to the Linux guest's device number and throw away the rest;
 they can crash due to minor number clashes.  With these guests, the
 workaround is not to supply problematic combinations of devices.
+
+
+Other frontend and backend options
+----------------------------------
+
+See xen/include/public/io/blkif.h for the full list of options.
diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt
index 5bd456d..9c2650b 100644
--- a/docs/misc/xl-disk-configuration.txt
+++ b/docs/misc/xl-disk-configuration.txt
@@ -178,6 +178,44 @@ information to be interpreted by the executable program <script>,
 These scripts are normally called "block-<script>".
 
 
+direct-io-safe
+--------------
+
+Description:           Disables non-O_DIRECT workaround
+Supported values:      absent, present
+Mandatory:             No
+Default value:         absent (workaround may be enabled)
+
+There is a memory lifetime bug in some driver domain (dom0) kernels
+which can cause crashes when using O_DIRECT.  The bug occurs due to a
+mismatch between the backend-visible lifetime of pages used for the
+Xen PV network protocol and that expected by the backend kernel's
+networking subsystem.  This can cause crashes when using certain
+backends with certain underlying storage.
+
+See:
+ http://lists.xen.org/archives/html/xen-devel/2012-12/msg01154.html
+
+For this reason, (this version of) the Xen libxl toolstack disables
+O_DIRECT when using the qemu-based Xen PV backend ("qdisk").
+
+However, this workaround has performance and scaling implications, and
+it is only necessary if the underlying device is a network filesystem.
+If the underlying device is not, then it is good to disable it; that
+is what this option is for.
+
+This option simply requests that the workaround be disabled.  (However,
+not all backends versions which use the workaround understand this
+option, so this is on a best effort basis.)
+
+It's important to note that if you are storing the VM disk on a
+network filesystem or a network block device (NFS or ISCSI) it might
+not be safe to use this option.  Otherwise specifying it is safe and
+can give better performances.
+
+If in the future the bug is fixed properly this option will then be
+silently ignored.
+
 
 ============================================
 DEPRECATED PARAMETERS, PREFIXES AND SYNTAXES
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 3236aa9..541c258 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2168,6 +2168,10 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
         flexarray_append(back, disk->readwrite ? "w" : "r");
         flexarray_append(back, "device-type");
         flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
+        if (disk->direct_io_safe) {
+            flexarray_append(back, "direct-io-safe");
+            flexarray_append(back, "1");
+        }
 
         flexarray_append(front, "backend-id");
         flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 37e4d82..1daa085 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -82,6 +82,13 @@
 #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1
 
 /*
+ * LIBXL_HAVE_DEVICE_DISK_DIRECT_IO_SAFE indicates that a
+ * 'direct_io_safe' field (of boolean type) is present in
+ * libxl_device_disk.
+ */
+#define LIBXL_HAVE_DEVICE_DISK_DIRECT_IO_SAFE 1
+
+/*
  * libxl ABI compatibility
  *
  * The only guarantee which libxl makes regarding ABI compatibility
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),
     ])
 
 libxl_device_nic = Struct("device_nic", [
diff --git a/tools/libxl/libxlu_disk_l.c b/tools/libxl/libxlu_disk_l.c
index 03adfef..c97f0b2 100644
--- a/tools/libxl/libxlu_disk_l.c
+++ b/tools/libxl/libxlu_disk_l.c
@@ -361,8 +361,8 @@ static void yy_fatal_error (yyconst char msg[] ,yyscan_t yyscanner );
 	*yy_cp = '\0'; \
 	yyg->yy_c_buf_p = yy_cp;
 
-#define YY_NUM_RULES 26
-#define YY_END_OF_BUFFER 27
+#define YY_NUM_RULES 27
+#define YY_END_OF_BUFFER 28
 /* This struct is not used in this scanner,
    but its presence is necessary. */
 struct yy_trans_info
@@ -370,90 +370,92 @@ struct yy_trans_info
 	flex_int32_t yy_verify;
 	flex_int32_t yy_nxt;
 	};
-static yyconst flex_int16_t yy_acclist[454] =
+static yyconst flex_int16_t yy_acclist[470] =
     {   0,
-       25,   25,   27,   23,   24,   26, 8193,   23,   24,   26,
-    16385, 8193,   23,   26,16385,   23,   24,   26,   24,   26,
-       23,   24,   26,   23,   24,   26,   23,   24,   26,   23,
-       24,   26,   23,   24,   26,   23,   24,   26,   23,   24,
-       26,   23,   24,   26,   23,   24,   26,   23,   24,   26,
-       23,   24,   26,   23,   24,   26,   23,   24,   26,   23,
-       24,   26,   23,   24,   26,   25,   26,   26,   23,   23,
-     8193,   23, 8193,   23,16385, 8193,   23, 8193,   23,   23,
-     8214,   23,16406,   23,   23,   23,   23,   23,   23,   23,
-       23,   23,   23,   23,   23,   23,   23,   23,   23,   23,
-
-       23,   23,   25, 8193,   23, 8193,   23, 8193, 8214,   23,
-     8214,   23, 8214,   13,   23,   23,   23,   23,   23,   23,
-       23,   23,   23,   23,   23,   23,   23,   23,   23,   23,
-       23,   23, 8214,   23, 8214,   23, 8214,   13,   23,   18,
-     8214,   23,16406,   23,   23,   23,   23,   23,   23,   23,
-     8207, 8214,   23,16399,16406,   21, 8214,   23,16406,   23,
-     8206, 8214,   23,16398,16406,   23,   23, 8209, 8214,   23,
-    16401,16406,   23,   23,   23,   23,   18, 8214,   23,   18,
-     8214,   23,   18,   23,   18, 8214,   23,    3,   23,   23,
-       20, 8214,   23,16406,   23,   23, 8207, 8214,   23, 8207,
-
-     8214,   23, 8207,   23, 8207, 8214,   21, 8214,   23,   21,
-     8214,   23,   21,   23,   21, 8214, 8206, 8214,   23, 8206,
-     8214,   23, 8206,   23, 8206, 8214,   23, 8209, 8214,   23,
-     8209, 8214,   23, 8209,   23, 8209, 8214,   23,   23,   10,
-       23,   18, 8214,   23,   18, 8214,   23,   18, 8214,   18,
-       23,   18,   23,    3,   23,   23,   20, 8214,   23,   20,
-     8214,   23,   20,   23,   20, 8214,   23,   19, 8214,   23,
-    16406, 8207, 8214,   23, 8207, 8214,   23, 8207, 8214, 8207,
-       23, 8207,   21, 8214,   23,   21, 8214,   23,   21, 8214,
-       21,   23,   21, 8206, 8214,   23, 8206, 8214,   23, 8206,
-
-     8214, 8206,   23, 8206,   23, 8209, 8214,   23, 8209, 8214,
-       23, 8209, 8214, 8209,   23, 8209,   23,   23,   10,   13,
-       10,    7,   23,   23,   20, 8214,   23,   20, 8214,   23,
-       20, 8214,   20,   23,   20,    2,   19, 8214,   23,   19,
-     8214,   23,   19,   23,   19, 8214,   11,   23,   12,   10,
-       10,   13,    7,   13,    7,    8,   23,    6,    2,   13,
-        2,   19, 8214,   23,   19, 8214,   23,   19, 8214,   19,
-       23,   19,   11,   13,   11,   16, 8214,   23,16406,   12,
-       13,   12,    7,    7,   13,    8,   13,    8,   23,    6,
-       13,    6,    6,   13,    6,   13,    2,    2,   13,   11,
-
-       11,   13,   16, 8214,   23,   16, 8214,   23,   16,   23,
-       16, 8214,   12,   13,    8,    8,   13,   23,    6,    6,
-       13,    6,    6,   16, 8214,   23,   16, 8214,   23,   16,
-     8214,   16,   23,   16,   23,    6,    6,    9,    6,    5,
-        6,    9,   13,    9,    4,    6,    5,    6,    9,    9,
-       13,    4,    6
+       26,   26,   28,   24,   25,   27, 8193,   24,   25,   27,
+    16385, 8193,   24,   27,16385,   24,   25,   27,   25,   27,
+       24,   25,   27,   24,   25,   27,   24,   25,   27,   24,
+       25,   27,   24,   25,   27,   24,   25,   27,   24,   25,
+       27,   24,   25,   27,   24,   25,   27,   24,   25,   27,
+       24,   25,   27,   24,   25,   27,   24,   25,   27,   24,
+       25,   27,   24,   25,   27,   26,   27,   27,   24,   24,
+     8193,   24, 8193,   24,16385, 8193,   24, 8193,   24,   24,
+     8215,   24,16407,   24,   24,   24,   24,   24,   24,   24,
+       24,   24,   24,   24,   24,   24,   24,   24,   24,   24,
+
+       24,   24,   24,   26, 8193,   24, 8193,   24, 8193, 8215,
+       24, 8215,   24, 8215,   14,   24,   24,   24,   24,   24,
+       24,   24,   24,   24,   24,   24,   24,   24,   24,   24,
+       24,   24,   24,   24, 8215,   24, 8215,   24, 8215,   14,
+       24,   19, 8215,   24,16407,   24,   24,   24,   24,   24,
+       24,   24,   24, 8208, 8215,   24,16400,16407,   22, 8215,
+       24,16407,   24, 8207, 8215,   24,16399,16407,   24,   24,
+     8210, 8215,   24,16402,16407,   24,   24,   24,   24,   19,
+     8215,   24,   19, 8215,   24,   19,   24,   19, 8215,   24,
+        3,   24,   24,   24,   21, 8215,   24,16407,   24,   24,
+
+     8208, 8215,   24, 8208, 8215,   24, 8208,   24, 8208, 8215,
+       22, 8215,   24,   22, 8215,   24,   22,   24,   22, 8215,
+     8207, 8215,   24, 8207, 8215,   24, 8207,   24, 8207, 8215,
+       24, 8210, 8215,   24, 8210, 8215,   24, 8210,   24, 8210,
+     8215,   24,   24,   10,   24,   19, 8215,   24,   19, 8215,
+       24,   19, 8215,   19,   24,   19,   24,    3,   24,   24,
+       24,   21, 8215,   24,   21, 8215,   24,   21,   24,   21,
+     8215,   24,   20, 8215,   24,16407, 8208, 8215,   24, 8208,
+     8215,   24, 8208, 8215, 8208,   24, 8208,   22, 8215,   24,
+       22, 8215,   24,   22, 8215,   22,   24,   22, 8207, 8215,
+
+       24, 8207, 8215,   24, 8207, 8215, 8207,   24, 8207,   24,
+     8210, 8215,   24, 8210, 8215,   24, 8210, 8215, 8210,   24,
+     8210,   24,   24,   10,   14,   10,    7,   24,   24,   24,
+       21, 8215,   24,   21, 8215,   24,   21, 8215,   21,   24,
+       21,    2,   20, 8215,   24,   20, 8215,   24,   20,   24,
+       20, 8215,   11,   24,   13,   10,   10,   14,    7,   14,
+        7,    8,   24,    6,   24,    2,   14,    2,   20, 8215,
+       24,   20, 8215,   24,   20, 8215,   20,   24,   20,   11,
+       14,   11,   17, 8215,   24,16407,   13,   14,   13,    7,
+        7,   14,    8,   14,    8,   24,    6,   14,    6,    6,
+
+       14,    6,   14,   24,    2,    2,   14,   11,   11,   14,
+       17, 8215,   24,   17, 8215,   24,   17,   24,   17, 8215,
+       13,   14,    8,    8,   14,   24,    6,    6,   14,    6,
+        6,   24,   17, 8215,   24,   17, 8215,   24,   17, 8215,
+       17,   24,   17,   24,    6,    6,   24,    9,    6,    5,
+        6,   24,    9,   14,    9,    4,    6,    5,    6,   24,
+        9,    9,   14,    4,    6,   12,   24,   12,   24
     } ;
 
-static yyconst flex_int16_t yy_accept[257] =
+static yyconst flex_int16_t yy_accept[271] =
     {   0,
         1,    1,    1,    2,    3,    4,    7,   12,   16,   19,
        21,   24,   27,   30,   33,   36,   39,   42,   45,   48,
        51,   54,   57,   60,   63,   66,   68,   69,   70,   71,
        73,   76,   78,   79,   80,   81,   84,   84,   85,   86,
        87,   88,   89,   90,   91,   92,   93,   94,   95,   96,
-       97,   98,   99,  100,  101,  102,  103,  104,  106,  108,
-      109,  111,  113,  114,  115,  116,  117,  118,  119,  120,
+       97,   98,   99,  100,  101,  102,  103,  104,  105,  107,
+      109,  110,  112,  114,  115,  116,  117,  118,  119,  120,
       121,  122,  123,  124,  125,  126,  127,  128,  129,  130,
-      131,  132,  133,  135,  137,  138,  139,  140,  144,  145,
-      146,  147,  148,  149,  150,  151,  156,  160,  161,  166,
-
-      167,  168,  173,  174,  175,  176,  177,  180,  183,  185,
-      187,  188,  190,  191,  195,  196,  197,  200,  203,  205,
-      207,  210,  213,  215,  217,  220,  223,  225,  227,  228,
-      231,  234,  236,  238,  239,  240,  241,  242,  245,  248,
-      250,  252,  253,  254,  256,  257,  260,  263,  265,  267,
-      268,  272,  275,  278,  280,  282,  283,  286,  289,  291,
-      293,  294,  297,  300,  302,  304,  305,  306,  309,  312,
-      314,  316,  317,  318,  319,  321,  322,  323,  324,  325,
-      328,  331,  333,  335,  336,  337,  340,  343,  345,  347,
-      348,  349,  350,  351,  353,  355,  356,  357,  358,  359,
-
-      361,  362,  365,  368,  370,  372,  373,  375,  376,  380,
-      382,  383,  384,  386,  388,  389,  390,  392,  393,  395,
-      397,  398,  400,  401,  403,  406,  409,  411,  413,  415,
-      416,  418,  419,  420,  422,  423,  424,  427,  430,  432,
-      434,  435,  436,  437,  438,  439,  440,  442,  444,  445,
-      447,  449,  450,  452,  454,  454
+      131,  132,  133,  134,  135,  137,  139,  140,  141,  142,
+      146,  147,  148,  149,  150,  151,  152,  153,  154,  159,
+
+      163,  164,  169,  170,  171,  176,  177,  178,  179,  180,
+      183,  186,  188,  190,  191,  193,  194,  195,  199,  200,
+      201,  204,  207,  209,  211,  214,  217,  219,  221,  224,
+      227,  229,  231,  232,  235,  238,  240,  242,  243,  244,
+      245,  246,  249,  252,  254,  256,  257,  258,  260,  261,
+      262,  265,  268,  270,  272,  273,  277,  280,  283,  285,
+      287,  288,  291,  294,  296,  298,  299,  302,  305,  307,
+      309,  310,  311,  314,  317,  319,  321,  322,  323,  324,
+      326,  327,  328,  329,  330,  331,  334,  337,  339,  341,
+      342,  343,  346,  349,  351,  353,  354,  355,  356,  357,
+
+      359,  361,  362,  363,  364,  365,  366,  368,  369,  372,
+      375,  377,  379,  380,  382,  383,  387,  389,  390,  391,
+      393,  395,  396,  397,  399,  400,  402,  404,  405,  406,
+      408,  409,  411,  414,  417,  419,  421,  423,  424,  426,
+      427,  428,  430,  431,  432,  433,  436,  439,  441,  443,
+      444,  445,  446,  447,  448,  449,  450,  452,  453,  455,
+      456,  458,  460,  461,  462,  464,  466,  468,  470,  470
     } ;
 
 static yyconst flex_int32_t yy_ec[256] =
@@ -496,244 +498,252 @@ static yyconst flex_int32_t yy_meta[34] =
         1,    1,    1
     } ;
 
-static yyconst flex_int16_t yy_base[315] =
+static yyconst flex_int16_t yy_base[329] =
     {   0,
-        0,    0,  563,  559,  558,  546,   32,   35,  665,  665,
-       44,   62,   30,   41,   50,   51,  532,   64,   47,   66,
-       67,  524,   68,  516,   72,    0,  665,  514,  665,   87,
-       91,    0,    0,  100,  500,  109,    0,   74,   95,   87,
-       32,   96,  105,  110,   77,   97,   40,  113,  116,  112,
-      118,  120,  121,  122,  123,  125,    0,  137,    0,    0,
-      147,    0,    0,  494,  129,  126,  134,  143,  145,  147,
-      148,  149,  151,  153,  156,  160,  155,  167,  162,  175,
-      168,  159,  188,    0,    0,  665,  166,  197,  179,  185,
-      176,  200,  460,  186,  193,  216,  225,  205,  234,  221,
-
-      237,  247,  204,  230,  244,  213,  254,    0,  256,    0,
-      251,  258,  254,  279,  256,  259,  267,    0,  269,    0,
-      286,    0,  288,    0,  290,    0,  297,    0,  267,  299,
-        0,  301,    0,  288,  297,  459,  302,  310,    0,    0,
-        0,    0,  305,  665,  307,  319,    0,  321,    0,  322,
-      332,  335,    0,    0,    0,    0,  339,    0,    0,    0,
-        0,  342,    0,    0,    0,    0,  340,  349,    0,    0,
-        0,    0,  337,  345,  450,  665,  442,  350,  352,  360,
-        0,    0,    0,    0,  424,  362,    0,  364,    0,  420,
-      319,  371,  419,  665,  418,  665,  417,  276,  368,  416,
-
-      665,  373,    0,    0,    0,    0,  415,  665,  382,  414,
-        0,  413,  665,  412,  665,  368,  411,  665,  384,  352,
-      410,  665,  409,  665,  391,    0,  395,    0,    0,  405,
-      665,  382,  316,  665,  385,  397,  399,    0,    0,    0,
-        0,  396,  403,  406,  271,  407,  228,  200,  665,  175,
-      665,   77,  665,  665,  665,  429,  433,  436,  440,  444,
-      448,  452,  456,  460,  464,  468,  472,  476,  480,  484,
-      488,  492,  496,  500,  504,  508,  512,  516,  520,  524,
-      528,  532,  536,  540,  544,  548,  552,  556,  560,  564,
-      568,  572,  576,  580,  584,  588,  592,  596,  600,  604,
-
-      608,  612,  616,  620,  624,  628,  632,  636,  640,  644,
-      648,  652,  656,  660
+        0,    0,  673,  657,  648,  616,   32,   35,  691,  691,
+       44,   62,   30,   41,   50,   51,  598,   64,   47,   66,
+       67,  586,   68,  582,   72,    0,  691,  584,  691,   87,
+       91,    0,    0,  100,  574,  109,    0,   74,   95,   87,
+       71,   96,   97,  105,  110,  112,  113,   40,  116,  119,
+      115,  120,  121,  124,  125,  127,  126,    0,  151,    0,
+        0,  153,    0,    0,  572,  128,  135,  139,  137,  136,
+      153,  156,  154,  148,  161,  162,  165,  166,  167,  170,
+      169,  178,  179,  171,  189,    0,    0,  691,  172,  202,
+      187,  174,  184,  202,  205,  558,  198,  206,  228,  237,
+
+      211,  246,  210,  215,  255,  216,  251,  234,  199,  263,
+        0,  265,    0,  252,  266,  261,  263,  292,  269,  270,
+      278,    0,  280,    0,  299,    0,  301,    0,  303,    0,
+      310,    0,  280,  312,    0,  314,    0,  301,  310,  556,
+      243,  321,    0,    0,    0,    0,  275,  691,  312,  322,
+      331,    0,  333,    0,  334,  344,  347,    0,    0,    0,
+        0,  351,    0,    0,    0,    0,  354,    0,    0,    0,
+        0,  352,  361,    0,    0,    0,    0,  318,  357,  548,
+      691,  540,  359,  362,  388,  370,    0,    0,    0,    0,
+      532,  372,    0,  374,    0,  520,  323,  381,  492,  691,
+
+      485,  691,  476,  349,  382,  369,  468,  691,  395,    0,
+        0,    0,    0,  449,  691,  406,  448,    0,  447,  691,
+      446,  691,  391,  444,  691,  399,  273,  395,  442,  691,
+      438,  691,  414,    0,  416,    0,    0,  398,  691,  412,
+      376,  691,  401,  417,  406,  426,    0,    0,    0,    0,
+      422,  423,  429,  426,  337,  433,  295,  423,  218,  691,
+       83,  691,  425,   38,  691,  691,  436,  691,  691,  455,
+      459,  462,  466,  470,  474,  478,  482,  486,  490,  494,
+      498,  502,  506,  510,  514,  518,  522,  526,  530,  534,
+      538,  542,  546,  550,  554,  558,  562,  566,  570,  574,
+
+      578,  582,  586,  590,  594,  598,  602,  606,  610,  614,
+      618,  622,  626,  630,  634,  638,  642,  646,  650,  654,
+      658,  662,  666,  670,  674,  678,  682,  686
     } ;
 
-static yyconst flex_int16_t yy_def[315] =
+static yyconst flex_int16_t yy_def[329] =
     {   0,
-      255,    1,  256,  256,  255,  257,  258,  258,  255,  255,
-      259,  259,   12,   12,   12,   12,   12,   12,   12,   12,
-       12,   12,   12,   12,   12,  260,  255,  257,  255,  261,
-      258,  262,  262,  263,   12,  257,  264,   12,   12,   12,
+      269,    1,  270,  270,  269,  271,  272,  272,  269,  269,
+      273,  273,   12,   12,   12,   12,   12,   12,   12,   12,
+       12,   12,   12,   12,   12,  274,  269,  271,  269,  275,
+      272,  276,  276,  277,   12,  271,  278,   12,   12,   12,
        12,   12,   12,   12,   12,   12,   12,   12,   12,   12,
-       12,   12,   12,   12,   12,   12,  260,  261,  262,  262,
-      265,  266,  266,  255,   12,   12,   12,   12,   12,   12,
+       12,   12,   12,   12,   12,   12,   12,  274,  275,  276,
+      276,  279,  280,  280,  269,   12,   12,   12,   12,   12,
        12,   12,   12,   12,   12,   12,   12,   12,   12,   12,
-       12,   12,  265,  266,  266,  255,   12,  267,   12,   12,
-       12,   12,   12,   12,   12,  268,  269,   12,  270,   12,
-
-       12,  271,   12,   12,   12,   12,  272,  273,  267,  273,
-       12,   12,   12,  274,   12,   12,  275,  276,  268,  276,
-      277,  278,  269,  278,  279,  280,  270,  280,   12,  281,
-      282,  271,  282,   12,   12,  283,   12,  272,  273,  273,
-      284,  284,   12,  255,   12,  285,  286,  274,  286,   12,
-      287,  275,  276,  276,  288,  288,  277,  278,  278,  289,
-      289,  279,  280,  280,  290,  290,   12,  281,  282,  282,
-      291,  291,   12,   12,  292,  255,  293,   12,   12,  285,
-      286,  286,  294,  294,  295,  296,  297,  287,  297,  298,
-       12,  299,  292,  255,  300,  255,  301,   12,  302,  303,
-
-      255,  296,  297,  297,  304,  304,  305,  255,  306,  307,
-      307,  300,  255,  308,  255,   12,  309,  255,  309,  309,
-      303,  255,  305,  255,  310,  311,  306,  311,  307,  308,
-      255,   12,  309,  255,  309,  309,  310,  311,  311,  312,
-      312,   12,  309,  309,  313,  309,  309,  314,  255,  309,
-      255,  314,  255,  255,    0,  255,  255,  255,  255,  255,
-      255,  255,  255,  255,  255,  255,  255,  255,  255,  255,
-      255,  255,  255,  255,  255,  255,  255,  255,  255,  255,
-      255,  255,  255,  255,  255,  255,  255,  255,  255,  255,
-      255,  255,  255,  255,  255,  255,  255,  255,  255,  255,
-
-      255,  255,  255,  255,  255,  255,  255,  255,  255,  255,
-      255,  255,  255,  255
+       12,   12,   12,   12,  279,  280,  280,  269,   12,  281,
+       12,   12,   12,   12,   12,   12,   12,   12,  282,  283,
+
+       12,  284,   12,   12,  285,   12,   12,   12,   12,  286,
+      287,  281,  287,   12,   12,   12,   12,  288,   12,   12,
+      289,  290,  282,  290,  291,  292,  283,  292,  293,  294,
+      284,  294,   12,  295,  296,  285,  296,   12,   12,  297,
+       12,  286,  287,  287,  298,  298,   12,  269,   12,   12,
+      299,  300,  288,  300,   12,  301,  289,  290,  290,  302,
+      302,  291,  292,  292,  303,  303,  293,  294,  294,  304,
+      304,   12,  295,  296,  296,  305,  305,   12,   12,  306,
+      269,  307,   12,   12,  277,  299,  300,  300,  308,  308,
+      309,  310,  311,  301,  311,  312,   12,  313,  306,  269,
+
+      314,  269,  315,   12,  316,  185,  317,  269,  310,  311,
+      311,  318,  318,  319,  269,  320,  321,  321,  314,  269,
+      322,  269,   12,  323,  269,  323,  323,  185,  317,  269,
+      319,  269,  324,  325,  320,  325,  321,  322,  269,   12,
+      323,  269,  323,  323,  185,  324,  325,  325,  326,  326,
+       12,  323,  323,  185,  327,  323,  323,  185,  328,  269,
+      323,  269,  185,  328,  269,  269,  185,  269,    0,  269,
+      269,  269,  269,  269,  269,  269,  269,  269,  269,  269,
+      269,  269,  269,  269,  269,  269,  269,  269,  269,  269,
+      269,  269,  269,  269,  269,  269,  269,  269,  269,  269,
+
+      269,  269,  269,  269,  269,  269,  269,  269,  269,  269,
+      269,  269,  269,  269,  269,  269,  269,  269,  269,  269,
+      269,  269,  269,  269,  269,  269,  269,  269
     } ;
 
-static yyconst flex_int16_t yy_nxt[699] =
+static yyconst flex_int16_t yy_nxt[725] =
     {   0,
         6,    7,    8,    9,    6,    6,    6,    6,   10,   11,
        12,   13,   14,   15,   16,   17,   17,   18,   17,   17,
        17,   17,   19,   17,   20,   21,   22,   23,   24,   17,
        25,   17,   17,   31,   31,   32,   31,   31,   32,   35,
-       33,   35,   41,   33,   28,   28,   28,   29,   34,   35,
-       35,   36,   37,   73,   42,   38,   35,   49,   68,   35,
-       35,   39,   28,   28,   28,   29,   34,   43,   45,   36,
-       37,   40,   44,   35,   46,   35,   35,   35,   51,   53,
-      249,   35,   50,   35,   55,   65,   35,   47,   56,   28,
-       59,   48,   31,   31,   32,   60,   35,   71,   67,   33,
-
-       28,   28,   28,   29,   35,   35,   35,   28,   37,   61,
-       61,   61,   62,   61,   35,   70,   61,   63,   66,   35,
-       49,   35,   35,   72,   74,   35,   69,   35,   75,   35,
-       35,   35,   35,   88,   35,   35,   82,   78,   35,   28,
-       59,   77,   87,   35,   76,   60,   80,   79,   81,   28,
-       84,   78,   35,   89,   35,   85,   35,   35,   35,   75,
-       35,   92,   35,   96,   35,   35,   90,   97,   35,   35,
-       93,   35,   94,   91,   99,   35,   35,   35,  254,  100,
-       95,  101,  102,  104,   35,   35,   98,  103,   35,  105,
-       28,   84,  111,  106,   35,   35,   85,  107,  107,   61,
-
-      108,  107,   35,  253,  107,  110,  112,  114,  113,   35,
-       75,   78,   99,   35,   35,  116,  117,  117,   61,  118,
-      117,  134,   35,  117,  120,  121,  121,   61,  122,  121,
-       35,  251,  121,  124,  125,  125,   61,  126,  125,   35,
-      137,  125,  128,  135,  102,  129,   35,  130,  130,   61,
-      131,  130,  136,   35,  130,  133,   28,  139,   28,  141,
-       35,  144,  140,   35,  142,   35,  151,   35,   35,   28,
-      153,   28,  155,  143,  249,  154,   35,  156,  145,  146,
-      146,   61,  147,  146,  150,   35,  146,  149,   28,  158,
-       28,  160,   28,  163,  159,  167,  161,   35,  164,   28,
-
-      165,   28,  169,   28,  171,  166,   35,  170,  216,  172,
-      177,   35,   28,  139,   35,  173,   35,  178,  140,  218,
-      179,   28,  181,   28,  183,  174,  209,  182,   35,  184,
-      185,   35,  186,  186,   61,  187,  186,   28,  153,  186,
-      189,   28,  158,  154,   28,  163,   35,  159,  190,   35,
-      164,   28,  169,  192,   35,  234,  191,  170,  197,   35,
-      199,   35,   28,  181,   28,  203,   28,  205,  182,  236,
-      204,  218,  206,   64,  211,   28,  203,   35,  198,  219,
-      220,  204,  225,  225,   61,  226,  225,  234,  218,  225,
-      228,   35,  232,   28,  238,  242,  235,   28,  240,  239,
-
-      218,   28,  238,  241,  245,   35,  218,  239,  215,  218,
-      218,  243,  208,  201,  234,  231,  196,  229,  224,  222,
-      215,  213,  176,  208,  244,  247,  246,  201,  250,   26,
-       26,   26,   26,   28,   28,   28,   30,   30,   30,   30,
-       35,   35,   35,   35,   57,  196,   57,   57,   58,   58,
-       58,   58,   60,  194,   60,   60,   34,   34,   34,   34,
-       64,   64,  176,   64,   83,   83,   83,   83,   85,  115,
-       85,   85,  109,  109,  109,  109,  119,  119,  119,  119,
-      123,  123,  123,  123,  127,  127,  127,  127,  132,  132,
-      132,  132,  138,  138,  138,  138,  140,   86,  140,  140,
-
-      148,  148,  148,  148,  152,  152,  152,  152,  154,   35,
-      154,  154,  157,  157,  157,  157,  159,   29,  159,  159,
-      162,  162,  162,  162,  164,   54,  164,  164,  168,  168,
-      168,  168,  170,   52,  170,  170,  175,  175,  175,  175,
-      142,   35,  142,  142,  180,  180,  180,  180,  182,   29,
-      182,  182,  188,  188,  188,  188,  156,  255,  156,  156,
-      161,   27,  161,  161,  166,   27,  166,  166,  172,  255,
-      172,  172,  193,  193,  193,  193,  195,  195,  195,  195,
-      184,  255,  184,  184,  200,  200,  200,  200,  202,  202,
-      202,  202,  204,  255,  204,  204,  207,  207,  207,  207,
-
-      210,  210,  210,  210,  212,  212,  212,  212,  214,  214,
-      214,  214,  217,  217,  217,  217,  221,  221,  221,  221,
-      206,  255,  206,  206,  223,  223,  223,  223,  227,  227,
-      227,  227,  211,  255,  211,  211,  230,  230,  230,  230,
-      233,  233,  233,  233,  237,  237,  237,  237,  239,  255,
-      239,  239,  241,  255,  241,  241,  248,  248,  248,  248,
-      252,  252,  252,  252,    5,  255,  255,  255,  255,  255,
-      255,  255,  255,  255,  255,  255,  255,  255,  255,  255,
-      255,  255,  255,  255,  255,  255,  255,  255,  255,  255,
-      255,  255,  255,  255,  255,  255,  255,  255
-
+       33,  260,   41,   33,   28,   28,   28,   29,   34,   35,
+       35,   36,   37,   75,   42,   38,   35,   50,   43,   35,
+       35,   39,   28,   28,   28,   29,   34,   44,   46,   36,
+       37,   40,   45,   35,   47,   35,   35,   35,   52,   54,
+       35,   35,   51,   35,   56,   66,  266,   48,   57,   28,
+       60,   49,   31,   31,   32,   61,   35,   69,   68,   33,
+
+       28,   28,   28,   29,   35,   35,   35,   28,   37,   62,
+       62,   62,   63,   62,   35,   72,   62,   64,   67,   35,
+       50,   35,   35,   71,   35,   35,   70,   76,   35,   35,
+       35,   77,   73,   35,   35,   35,   35,   35,   80,   74,
+       84,   89,   90,   79,   35,   35,   35,   78,   35,   82,
+       81,   83,   80,   28,   60,   28,   86,   35,   91,   61,
+       92,   87,   35,   35,   93,   35,   94,   95,   77,   96,
+       35,   35,   99,  100,   35,   35,   35,  102,   35,   35,
+       35,   35,   97,   35,  104,  105,  103,   35,   35,   98,
+      106,   28,   86,   35,  107,  115,   35,   87,  101,  109,
+
+      114,  108,  110,  110,   62,  111,  110,   35,   35,  110,
+      113,   35,  118,  117,   35,   35,  116,   80,  102,   35,
+       35,  265,  105,   77,   35,   35,  141,  120,  121,  121,
+       62,  122,  121,  138,  133,  121,  124,  125,  125,   62,
+      126,  125,  140,   35,  125,  128,  129,  129,   62,  130,
+      129,  182,   35,  129,  132,  134,  134,   62,  135,  134,
+       35,   35,  134,  137,  139,   28,  143,   28,  145,  148,
+       35,  144,   35,  146,  147,   35,  242,  156,   35,   35,
+       28,  158,   28,  160,   35,  149,  159,  183,  161,   35,
+      244,  150,  151,  151,   62,  152,  151,  155,  262,  151,
+
+      154,   28,  163,   28,  165,   28,  168,  164,  172,  166,
+       35,  169,   28,  170,   28,  174,   28,  176,  171,   35,
+      175,   35,  177,   28,  143,  184,  185,   35,  178,  144,
+      216,   35,   35,   28,  187,   28,  189,  197,  179,  188,
+      260,  190,  191,   35,  192,  192,   62,  193,  192,   28,
+      158,  192,  195,   28,  163,  159,   28,  168,   35,  164,
+      196,   35,  169,   28,  174,  198,   35,  203,   35,  175,
+      205,   35,   28,  187,   28,  210,   28,  212,  188,  225,
+      211,  223,  213,   65,  218,  225,   34,  204,   28,   28,
+       28,   29,  228,  226,  227,   28,   37,   28,  210,  245,
+
+       35,  222,  242,  211,  225,  206,  233,  233,   62,  234,
+      233,  243,   34,  233,  236,  240,   28,  247,   28,  249,
+      225,   35,  248,   34,  250,  251,  225,  252,   28,  247,
+      255,   35,  225,  254,  248,  258,  225,  263,  267,  268,
+       34,  215,   34,   34,  253,  208,  256,  242,  257,  239,
+      202,  237,  232,   34,  261,   26,   26,   26,   26,   28,
+       28,   28,   30,   30,   30,   30,   35,   35,   35,   35,
+       58,  230,   58,   58,   59,   59,   59,   59,   61,  222,
+       61,   61,   34,   34,   34,   34,   65,   65,  220,   65,
+       85,   85,   85,   85,   87,  181,   87,   87,  112,  112,
+
+      112,  112,  123,  123,  123,  123,  127,  127,  127,  127,
+      131,  131,  131,  131,  136,  136,  136,  136,  142,  142,
+      142,  142,  144,  215,  144,  144,  153,  153,  153,  153,
+      157,  157,  157,  157,  159,  208,  159,  159,  162,  162,
+      162,  162,  164,  202,  164,  164,  167,  167,  167,  167,
+      169,  200,  169,  169,  173,  173,  173,  173,  175,  181,
+      175,  175,  180,  180,  180,  180,  146,  119,  146,  146,
+      186,  186,  186,  186,  188,   88,  188,  188,  194,  194,
+      194,  194,  161,   35,  161,  161,  166,   29,  166,  166,
+      171,   55,  171,  171,  177,   53,  177,  177,  199,  199,
+
+      199,  199,  201,  201,  201,  201,  190,   35,  190,  190,
+      207,  207,  207,  207,  209,  209,  209,  209,  211,   29,
+      211,  211,  214,  214,  214,  214,  217,  217,  217,  217,
+      219,  219,  219,  219,  221,  221,  221,  221,  224,  224,
+      224,  224,  229,  229,  229,  229,  213,  269,  213,  213,
+      231,  231,  231,  231,  235,  235,  235,  235,  218,   27,
+      218,  218,  238,  238,  238,  238,  241,  241,  241,  241,
+      246,  246,  246,  246,  248,   27,  248,  248,  250,  269,
+      250,  250,  259,  259,  259,  259,  264,  264,  264,  264,
+        5,  269,  269,  269,  269,  269,  269,  269,  269,  269,
+
+      269,  269,  269,  269,  269,  269,  269,  269,  269,  269,
+      269,  269,  269,  269,  269,  269,  269,  269,  269,  269,
+      269,  269,  269,  269
     } ;
 
-static yyconst flex_int16_t yy_chk[699] =
+static yyconst flex_int16_t yy_chk[725] =
     {   0,
         1,    1,    1,    1,    1,    1,    1,    1,    1,    1,
         1,    1,    1,    1,    1,    1,    1,    1,    1,    1,
         1,    1,    1,    1,    1,    1,    1,    1,    1,    1,
         1,    1,    1,    7,    7,    7,    8,    8,    8,   13,
-        7,   41,   13,    8,   11,   11,   11,   11,   11,   47,
-       14,   11,   11,   47,   14,   11,   19,   19,   41,   15,
+        7,  264,   13,    8,   11,   11,   11,   11,   11,   48,
+       14,   11,   11,   48,   14,   11,   19,   19,   14,   15,
        16,   11,   12,   12,   12,   12,   12,   14,   16,   12,
        12,   12,   15,   18,   16,   20,   21,   23,   21,   23,
-      252,   25,   20,   38,   25,   38,   45,   18,   25,   30,
-       30,   18,   31,   31,   31,   30,   40,   45,   40,   31,
-
-       34,   34,   34,   34,   39,   42,   46,   34,   34,   36,
-       36,   36,   36,   36,   43,   43,   36,   36,   39,   44,
-       44,   50,   48,   46,   48,   49,   42,   51,   49,   52,
-       53,   54,   55,   66,   56,   66,   55,   56,   65,   58,
-       58,   51,   65,   67,   50,   58,   54,   53,   54,   61,
-       61,   52,   68,   67,   69,   61,   70,   71,   72,   70,
-       73,   71,   74,   75,   77,   75,   68,   76,   82,   76,
-       72,   79,   73,   69,   78,   87,   78,   81,  250,   79,
-       74,   80,   80,   81,   80,   91,   77,   80,   89,   82,
-       83,   83,   89,   87,   90,   94,   83,   88,   88,   88,
-
-       88,   88,   95,  248,   88,   88,   90,   92,   91,   92,
-       95,   98,   98,  103,   98,   94,   96,   96,   96,   96,
-       96,  103,  106,   96,   96,   97,   97,   97,   97,   97,
-      100,  247,   97,   97,   99,   99,   99,   99,   99,  104,
-      106,   99,   99,  104,  101,  100,  101,  102,  102,  102,
-      102,  102,  105,  105,  102,  102,  107,  107,  109,  109,
-      111,  112,  107,  113,  109,  115,  116,  112,  116,  117,
-      117,  119,  119,  111,  245,  117,  129,  119,  113,  114,
-      114,  114,  114,  114,  115,  198,  114,  114,  121,  121,
-      123,  123,  125,  125,  121,  129,  123,  134,  125,  127,
-
-      127,  130,  130,  132,  132,  127,  135,  130,  198,  132,
-      137,  137,  138,  138,  143,  134,  145,  143,  138,  233,
-      145,  146,  146,  148,  148,  135,  191,  146,  191,  148,
-      150,  150,  151,  151,  151,  151,  151,  152,  152,  151,
-      151,  157,  157,  152,  162,  162,  173,  157,  167,  167,
-      162,  168,  168,  174,  174,  220,  173,  168,  178,  178,
-      179,  179,  180,  180,  186,  186,  188,  188,  180,  220,
-      186,  199,  188,  192,  192,  202,  202,  216,  178,  199,
-      199,  202,  209,  209,  209,  209,  209,  219,  235,  209,
-      209,  232,  216,  225,  225,  232,  219,  227,  227,  225,
-
-      236,  237,  237,  227,  242,  242,  243,  237,  230,  244,
-      246,  235,  223,  221,  217,  214,  212,  210,  207,  200,
-      197,  195,  193,  190,  236,  244,  243,  185,  246,  256,
-      256,  256,  256,  257,  257,  257,  258,  258,  258,  258,
-      259,  259,  259,  259,  260,  177,  260,  260,  261,  261,
-      261,  261,  262,  175,  262,  262,  263,  263,  263,  263,
-      264,  264,  136,  264,  265,  265,  265,  265,  266,   93,
-      266,  266,  267,  267,  267,  267,  268,  268,  268,  268,
-      269,  269,  269,  269,  270,  270,  270,  270,  271,  271,
-      271,  271,  272,  272,  272,  272,  273,   64,  273,  273,
-
-      274,  274,  274,  274,  275,  275,  275,  275,  276,   35,
-      276,  276,  277,  277,  277,  277,  278,   28,  278,  278,
-      279,  279,  279,  279,  280,   24,  280,  280,  281,  281,
-      281,  281,  282,   22,  282,  282,  283,  283,  283,  283,
-      284,   17,  284,  284,  285,  285,  285,  285,  286,    6,
-      286,  286,  287,  287,  287,  287,  288,    5,  288,  288,
-      289,    4,  289,  289,  290,    3,  290,  290,  291,    0,
-      291,  291,  292,  292,  292,  292,  293,  293,  293,  293,
-      294,    0,  294,  294,  295,  295,  295,  295,  296,  296,
-      296,  296,  297,    0,  297,  297,  298,  298,  298,  298,
-
-      299,  299,  299,  299,  300,  300,  300,  300,  301,  301,
-      301,  301,  302,  302,  302,  302,  303,  303,  303,  303,
-      304,    0,  304,  304,  305,  305,  305,  305,  306,  306,
-      306,  306,  307,    0,  307,  307,  308,  308,  308,  308,
-      309,  309,  309,  309,  310,  310,  310,  310,  311,    0,
-      311,  311,  312,    0,  312,  312,  313,  313,  313,  313,
-      314,  314,  314,  314,  255,  255,  255,  255,  255,  255,
-      255,  255,  255,  255,  255,  255,  255,  255,  255,  255,
-      255,  255,  255,  255,  255,  255,  255,  255,  255,  255,
-      255,  255,  255,  255,  255,  255,  255,  255
-
+       41,   25,   20,   38,   25,   38,  261,   18,   25,   30,
+       30,   18,   31,   31,   31,   30,   40,   41,   40,   31,
+
+       34,   34,   34,   34,   39,   42,   43,   34,   34,   36,
+       36,   36,   36,   36,   44,   44,   36,   36,   39,   45,
+       45,   46,   47,   43,   51,   49,   42,   49,   50,   52,
+       53,   50,   46,   54,   55,   57,   56,   66,   57,   47,
+       56,   66,   67,   52,   67,   70,   69,   51,   68,   55,
+       54,   55,   53,   59,   59,   62,   62,   74,   68,   59,
+       69,   62,   71,   73,   70,   72,   71,   73,   72,   74,
+       75,   76,   77,   78,   77,   78,   79,   80,   81,   80,
+       84,   89,   75,   92,   82,   82,   81,   82,   83,   76,
+       82,   85,   85,   93,   83,   92,   91,   85,   79,   89,
+
+       91,   84,   90,   90,   90,   90,   90,   97,  109,   90,
+       90,   94,   95,   94,   95,   98,   93,  101,  101,  103,
+      101,  259,  104,   98,  104,  106,  109,   97,   99,   99,
+       99,   99,   99,  106,  103,   99,   99,  100,  100,  100,
+      100,  100,  108,  108,  100,  100,  102,  102,  102,  102,
+      102,  141,  141,  102,  102,  105,  105,  105,  105,  105,
+      107,  114,  105,  105,  107,  110,  110,  112,  112,  115,
+      116,  110,  117,  112,  114,  115,  227,  120,  119,  120,
+      121,  121,  123,  123,  147,  116,  121,  147,  123,  133,
+      227,  117,  118,  118,  118,  118,  118,  119,  257,  118,
+
+      118,  125,  125,  127,  127,  129,  129,  125,  133,  127,
+      138,  129,  131,  131,  134,  134,  136,  136,  131,  139,
+      134,  149,  136,  142,  142,  149,  150,  178,  138,  142,
+      197,  150,  197,  151,  151,  153,  153,  178,  139,  151,
+      255,  153,  155,  155,  156,  156,  156,  156,  156,  157,
+      157,  156,  156,  162,  162,  157,  167,  167,  204,  162,
+      172,  172,  167,  173,  173,  179,  179,  183,  183,  173,
+      184,  184,  186,  186,  192,  192,  194,  194,  186,  241,
+      192,  204,  194,  198,  198,  205,  206,  183,  185,  185,
+      185,  185,  206,  205,  205,  185,  185,  209,  209,  228,
+
+      223,  238,  226,  209,  243,  185,  216,  216,  216,  216,
+      216,  226,  228,  216,  216,  223,  233,  233,  235,  235,
+      244,  240,  233,  245,  235,  240,  252,  243,  246,  246,
+      251,  251,  253,  245,  246,  254,  256,  258,  263,  267,
+      258,  231,  263,  254,  244,  229,  252,  224,  253,  221,
+      219,  217,  214,  267,  256,  270,  270,  270,  270,  271,
+      271,  271,  272,  272,  272,  272,  273,  273,  273,  273,
+      274,  207,  274,  274,  275,  275,  275,  275,  276,  203,
+      276,  276,  277,  277,  277,  277,  278,  278,  201,  278,
+      279,  279,  279,  279,  280,  199,  280,  280,  281,  281,
+
+      281,  281,  282,  282,  282,  282,  283,  283,  283,  283,
+      284,  284,  284,  284,  285,  285,  285,  285,  286,  286,
+      286,  286,  287,  196,  287,  287,  288,  288,  288,  288,
+      289,  289,  289,  289,  290,  191,  290,  290,  291,  291,
+      291,  291,  292,  182,  292,  292,  293,  293,  293,  293,
+      294,  180,  294,  294,  295,  295,  295,  295,  296,  140,
+      296,  296,  297,  297,  297,  297,  298,   96,  298,  298,
+      299,  299,  299,  299,  300,   65,  300,  300,  301,  301,
+      301,  301,  302,   35,  302,  302,  303,   28,  303,  303,
+      304,   24,  304,  304,  305,   22,  305,  305,  306,  306,
+
+      306,  306,  307,  307,  307,  307,  308,   17,  308,  308,
+      309,  309,  309,  309,  310,  310,  310,  310,  311,    6,
+      311,  311,  312,  312,  312,  312,  313,  313,  313,  313,
+      314,  314,  314,  314,  315,  315,  315,  315,  316,  316,
+      316,  316,  317,  317,  317,  317,  318,    5,  318,  318,
+      319,  319,  319,  319,  320,  320,  320,  320,  321,    4,
+      321,  321,  322,  322,  322,  322,  323,  323,  323,  323,
+      324,  324,  324,  324,  325,    3,  325,  325,  326,    0,
+      326,  326,  327,  327,  327,  327,  328,  328,  328,  328,
+      269,  269,  269,  269,  269,  269,  269,  269,  269,  269,
+
+      269,  269,  269,  269,  269,  269,  269,  269,  269,  269,
+      269,  269,  269,  269,  269,  269,  269,  269,  269,  269,
+      269,  269,  269,  269
     } ;
 
 #define YY_TRAILING_MASK 0x2000
@@ -890,7 +900,7 @@ static int vdev_and_devtype(DiskParseContext *dpc, char *str) {
 #define DPC ((DiskParseContext*)yyextra)
 
 
-#line 894 "libxlu_disk_l.c"
+#line 904 "libxlu_disk_l.c"
 
 #define INITIAL 0
 #define LEXERR 1
@@ -1131,7 +1141,7 @@ YY_DECL
 
  /*----- the scanner rules which do the parsing -----*/
 
-#line 1135 "libxlu_disk_l.c"
+#line 1145 "libxlu_disk_l.c"
 
 	if ( !yyg->yy_init )
 		{
@@ -1195,14 +1205,14 @@ yy_match:
 			while ( yy_chk[yy_base[yy_current_state] + yy_c] != yy_current_state )
 				{
 				yy_current_state = (int) yy_def[yy_current_state];
-				if ( yy_current_state >= 256 )
+				if ( yy_current_state >= 270 )
 					yy_c = yy_meta[(unsigned int) yy_c];
 				}
 			yy_current_state = yy_nxt[yy_base[yy_current_state] + (unsigned int) yy_c];
 			*yyg->yy_state_ptr++ = yy_current_state;
 			++yy_cp;
 			}
-		while ( yy_current_state != 255 );
+		while ( yy_current_state != 269 );
 
 yy_find_action:
 		yy_current_state = *--yyg->yy_state_ptr;
@@ -1313,34 +1323,39 @@ YY_RULE_SETUP
 #line 175 "libxlu_disk_l.l"
 { STRIP(','); SAVESTRING("script", script, FROMEQUALS); }
 	YY_BREAK
-/* the target magic parameter, eats the rest of the string */
 case 12:
 YY_RULE_SETUP
-#line 179 "libxlu_disk_l.l"
+#line 176 "libxlu_disk_l.l"
+{ DPC->disk->direct_io_safe = 1; }
+	YY_BREAK
+/* the target magic parameter, eats the rest of the string */
+case 13:
+YY_RULE_SETUP
+#line 180 "libxlu_disk_l.l"
 { STRIP(','); SAVESTRING("target", pdev_path, FROMEQUALS); }
 	YY_BREAK
 /* unknown parameters */
-case 13:
-/* rule 13 can match eol */
+case 14:
+/* rule 14 can match eol */
 YY_RULE_SETUP
-#line 183 "libxlu_disk_l.l"
+#line 184 "libxlu_disk_l.l"
 { xlu__disk_err(DPC,yytext,"unknown parameter"); }
 	YY_BREAK
 /* deprecated prefixes */
 /* the "/.*" in these patterns ensures that they count as if they
    * matched the whole string, so these patterns take precedence */
-case 14:
+case 15:
 YY_RULE_SETUP
-#line 190 "libxlu_disk_l.l"
+#line 191 "libxlu_disk_l.l"
 {
                     STRIP(':');
                     DPC->had_depr_prefix=1; DEPRECATE("use `[format=]...,'");
                     setformat(DPC, yytext);
                  }
 	YY_BREAK
-case 15:
+case 16:
 YY_RULE_SETUP
-#line 196 "libxlu_disk_l.l"
+#line 197 "libxlu_disk_l.l"
 {
                     char *newscript;
                     STRIP(':');
@@ -1354,30 +1369,22 @@ YY_RULE_SETUP
                     free(newscript);
                 }
 	YY_BREAK
-case 16:
+case 17:
 *yy_cp = yyg->yy_hold_char; /* undo effects of setting up yytext */
 yyg->yy_c_buf_p = yy_cp = yy_bp + 8;
 YY_DO_BEFORE_ACTION; /* set up yytext again */
 YY_RULE_SETUP
-#line 209 "libxlu_disk_l.l"
-{ DPC->had_depr_prefix=1; DEPRECATE(0); }
-	YY_BREAK
-case 17:
-YY_RULE_SETUP
 #line 210 "libxlu_disk_l.l"
 { DPC->had_depr_prefix=1; DEPRECATE(0); }
 	YY_BREAK
 case 18:
-*yy_cp = yyg->yy_hold_char; /* undo effects of setting up yytext */
-yyg->yy_c_buf_p = yy_cp = yy_bp + 4;
-YY_DO_BEFORE_ACTION; /* set up yytext again */
 YY_RULE_SETUP
 #line 211 "libxlu_disk_l.l"
 { DPC->had_depr_prefix=1; DEPRECATE(0); }
 	YY_BREAK
 case 19:
 *yy_cp = yyg->yy_hold_char; /* undo effects of setting up yytext */
-yyg->yy_c_buf_p = yy_cp = yy_bp + 6;
+yyg->yy_c_buf_p = yy_cp = yy_bp + 4;
 YY_DO_BEFORE_ACTION; /* set up yytext again */
 YY_RULE_SETUP
 #line 212 "libxlu_disk_l.l"
@@ -1385,7 +1392,7 @@ YY_RULE_SETUP
 	YY_BREAK
 case 20:
 *yy_cp = yyg->yy_hold_char; /* undo effects of setting up yytext */
-yyg->yy_c_buf_p = yy_cp = yy_bp + 5;
+yyg->yy_c_buf_p = yy_cp = yy_bp + 6;
 YY_DO_BEFORE_ACTION; /* set up yytext again */
 YY_RULE_SETUP
 #line 213 "libxlu_disk_l.l"
@@ -1393,26 +1400,34 @@ YY_RULE_SETUP
 	YY_BREAK
 case 21:
 *yy_cp = yyg->yy_hold_char; /* undo effects of setting up yytext */
-yyg->yy_c_buf_p = yy_cp = yy_bp + 4;
+yyg->yy_c_buf_p = yy_cp = yy_bp + 5;
 YY_DO_BEFORE_ACTION; /* set up yytext again */
 YY_RULE_SETUP
 #line 214 "libxlu_disk_l.l"
 { DPC->had_depr_prefix=1; DEPRECATE(0); }
 	YY_BREAK
 case 22:
-/* rule 22 can match eol */
+*yy_cp = yyg->yy_hold_char; /* undo effects of setting up yytext */
+yyg->yy_c_buf_p = yy_cp = yy_bp + 4;
+YY_DO_BEFORE_ACTION; /* set up yytext again */
+YY_RULE_SETUP
+#line 215 "libxlu_disk_l.l"
+{ DPC->had_depr_prefix=1; DEPRECATE(0); }
+	YY_BREAK
+case 23:
+/* rule 23 can match eol */
 YY_RULE_SETUP
-#line 216 "libxlu_disk_l.l"
+#line 217 "libxlu_disk_l.l"
 {
 		  xlu__disk_err(DPC,yytext,"unknown deprecated disk prefix");
 		  return 0;
 		}
 	YY_BREAK
 /* positional parameters */
-case 23:
-/* rule 23 can match eol */
+case 24:
+/* rule 24 can match eol */
 YY_RULE_SETUP
-#line 223 "libxlu_disk_l.l"
+#line 224 "libxlu_disk_l.l"
 {
     STRIP(',');
 
@@ -1439,27 +1454,27 @@ YY_RULE_SETUP
     }
 }
 	YY_BREAK
-case 24:
+case 25:
 YY_RULE_SETUP
-#line 249 "libxlu_disk_l.l"
+#line 250 "libxlu_disk_l.l"
 {
     BEGIN(LEXERR);
     yymore();
 }
 	YY_BREAK
-case 25:
+case 26:
 YY_RULE_SETUP
-#line 253 "libxlu_disk_l.l"
+#line 254 "libxlu_disk_l.l"
 {
     xlu__disk_err(DPC,yytext,"bad disk syntax"); return 0;
 }
 	YY_BREAK
-case 26:
+case 27:
 YY_RULE_SETUP
-#line 256 "libxlu_disk_l.l"
+#line 257 "libxlu_disk_l.l"
 YY_FATAL_ERROR( "flex scanner jammed" );
 	YY_BREAK
-#line 1463 "libxlu_disk_l.c"
+#line 1478 "libxlu_disk_l.c"
 			case YY_STATE_EOF(INITIAL):
 			case YY_STATE_EOF(LEXERR):
 				yyterminate();
@@ -1723,7 +1738,7 @@ static int yy_get_next_buffer (yyscan_t yyscanner)
 		while ( yy_chk[yy_base[yy_current_state] + yy_c] != yy_current_state )
 			{
 			yy_current_state = (int) yy_def[yy_current_state];
-			if ( yy_current_state >= 256 )
+			if ( yy_current_state >= 270 )
 				yy_c = yy_meta[(unsigned int) yy_c];
 			}
 		yy_current_state = yy_nxt[yy_base[yy_current_state] + (unsigned int) yy_c];
@@ -1747,11 +1762,11 @@ static int yy_get_next_buffer (yyscan_t yyscanner)
 	while ( yy_chk[yy_base[yy_current_state] + yy_c] != yy_current_state )
 		{
 		yy_current_state = (int) yy_def[yy_current_state];
-		if ( yy_current_state >= 256 )
+		if ( yy_current_state >= 270 )
 			yy_c = yy_meta[(unsigned int) yy_c];
 		}
 	yy_current_state = yy_nxt[yy_base[yy_current_state] + (unsigned int) yy_c];
-	yy_is_jam = (yy_current_state == 255);
+	yy_is_jam = (yy_current_state == 269);
 	if ( ! yy_is_jam )
 		*yyg->yy_state_ptr++ = yy_current_state;
 
@@ -2551,4 +2566,4 @@ void xlu__disk_yyfree (void * ptr , yyscan_t yyscanner)
 
 #define YYTABLES_NAME "yytables"
 
-#line 256 "libxlu_disk_l.l"
+#line 257 "libxlu_disk_l.l"
diff --git a/tools/libxl/libxlu_disk_l.h b/tools/libxl/libxlu_disk_l.h
index 0176249..59ce7cf 100644
--- a/tools/libxl/libxlu_disk_l.h
+++ b/tools/libxl/libxlu_disk_l.h
@@ -344,7 +344,7 @@ extern int xlu__disk_yylex (yyscan_t yyscanner);
 #undef YY_DECL
 #endif
 
-#line 256 "libxlu_disk_l.l"
+#line 257 "libxlu_disk_l.l"
 
 #line 350 "libxlu_disk_l.h"
 #undef xlu__disk_yyIN_HEADER
diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l
index 7c4e7f1..ba8577c 100644
--- a/tools/libxl/libxlu_disk_l.l
+++ b/tools/libxl/libxlu_disk_l.l
@@ -173,6 +173,7 @@ backendtype=[^,]*,? { STRIP(','); setbackendtype(DPC,FROMEQUALS); }
 
 vdev=[^,]*,?	{ STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); }
 script=[^,]*,?	{ STRIP(','); SAVESTRING("script", script, FROMEQUALS); }
+direct-io-safe,? { DPC->disk->direct_io_safe = 1; }
 
  /* the target magic parameter, eats the rest of the string */
 
diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
index b9b9d98..2dbb12f 100644
--- a/xen/include/public/io/blkif.h
+++ b/xen/include/public/io/blkif.h
@@ -97,6 +97,28 @@
  *
  *      The type of the backing device/object.
  *
+ *
+ * direct-io-safe
+ *      Values:         0/1 (boolean)
+ *      Default Value:  0
+ *
+ *      The underlying storage is not affected by the direct IO memory
+ *      lifetime bug.  See:
+ *        http://lists.xen.org/archives/html/xen-devel/2012-12/msg01154.html
+ *
+ *      Therefore this option gives the backend permission to use
+ *      O_DIRECT, notwithstanding that bug.
+ *
+ *      That is, if this option is enabled, use of O_DIRECT is safe,
+ *      in circumstances where we would normally have avoided it as a
+ *      workaround for that bug.  This option is not relevant for all
+ *      backends, and even not necessarily supported for those for
+ *      which it is relevant.  A backend which knows that it is not
+ *      affected by the bug can ignore this option.
+ *
+ *      This option doesn't require a backend to use O_DIRECT, so it
+ *      should not be used to try to control the caching behaviour.
+ *
  *--------------------------------- Features ---------------------------------
  *
  * feature-barrier

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 18:01         ` Ian Jackson
  2013-06-27 18:04           ` Ian Jackson
@ 2013-06-27 18:12           ` Stefano Stabellini
  1 sibling, 0 replies; 21+ messages in thread
From: Stefano Stabellini @ 2013-06-27 18:12 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Ian Campbell, Stefano Stabellini

On Thu, 27 Jun 2013, Ian Jackson wrote:
> Here's the result.  I took Anthony Ligouri's ack off it.
> 
> Ian.
> 
> commit 21538601f7d4f2de4d518b92ec9212c99be829e8
> Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Date:   Thu Jun 27 14:20:51 2013 +0000
> 
>     xen_disk: support "direct-io-safe" backend option
>     
>     Support backend option "direct-io-safe".  This is documented as
>     follows in the Xen backend specification:
>     
>      * direct-io-safe
>      *      Values:         0/1 (boolean)
>      *      Default Value:  0
>      *
>      *      The underlying storage is not affected by the direct IO memory
>      *      lifetime bug.  See:
>      *        http://lists.xen.org/archives/html/xen-devel/2012-12/msg01154.html
>      *
>      *      Therefore this option gives the backend permission to use
>      *      O_DIRECT, notwithstanding that bug.
>      *
>      *      That is, if this option is enabled, use of O_DIRECT is safe,
>      *      in circumstances where we would normally have avoided it as a
>      *      workaround for that bug.  This option is not relevant for all
>      *      backends, and even not necessarily supported for those for
>      *      which it is relevant.  A backend which knows that it is not
>      *      affected by the bug can ignore this option.
>      *
>      *      This option doesn't require a backend to use O_DIRECT, so it
>      *      should not be used to try to control the caching behaviour.
>     
>     Also, BDRV_O_NATIVE_AIO is ignored if BDRV_O_NOCACHE, so clarify the
>     default flags passed to the qemu block layer.
>     
>     Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

I'll send to qemu-devel and add your signed-off-by if you are OK with
that


> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 247f32f..091284b 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -93,6 +93,7 @@ struct XenBlkDev {
>      char                *type;
>      char                *dev;
>      char                *devtype;
> +    bool                directiosafe;
>      const char          *fileproto;
>      const char          *filename;
>      int                 ring_ref;
> @@ -701,6 +702,7 @@ static int blk_init(struct XenDevice *xendev)
>  {
>      struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
>      int info = 0;
> +    char *directiosafe = NULL;
>  
>      /* read xenstore entries */
>      if (blkdev->params == NULL) {
> @@ -733,6 +735,8 @@ static int blk_init(struct XenDevice *xendev)
>      if (blkdev->devtype == NULL) {
>          blkdev->devtype = xenstore_read_be_str(&blkdev->xendev, "device-type");
>      }
> +    directiosafe = xenstore_read_be_str(&blkdev->xendev, "direct-io-safe");
> +    blkdev->directiosafe = (directiosafe && atoi(directiofsafe));
>  
>      /* do we have all we need? */
>      if (blkdev->params == NULL ||
> @@ -760,6 +764,8 @@ static int blk_init(struct XenDevice *xendev)
>      xenstore_write_be_int(&blkdev->xendev, "feature-flush-cache", 1);
>      xenstore_write_be_int(&blkdev->xendev, "feature-persistent", 1);
>      xenstore_write_be_int(&blkdev->xendev, "info", info);
> +
> +    g_free(directiosafe);
>      return 0;
>  
>  out_error:
> @@ -773,6 +779,8 @@ out_error:
>      blkdev->dev = NULL;
>      g_free(blkdev->devtype);
>      blkdev->devtype = NULL;
> +    g_free(directiosafe);
> +    blkdev->directiosafe = false;
>      return -1;
>  }
>  
> @@ -783,7 +791,11 @@ static int blk_connect(struct XenDevice *xendev)
>      bool readonly = true;
>  
>      /* read-only ? */
> -    qflags = BDRV_O_CACHE_WB | BDRV_O_NATIVE_AIO;
> +    if (blkdev->directiosafe) {
> +        qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
> +    } else {
> +        qflags = BDRV_O_CACHE_WB;
> +    }
>      if (strcmp(blkdev->mode, "w") == 0) {
>          qflags |= BDRV_O_RDWR;
>          readonly = false;
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] introduce a cache options for PV disks
  2013-06-27 18:04           ` Ian Jackson
@ 2013-06-28  7:39             ` Ian Campbell
  0 siblings, 0 replies; 21+ messages in thread
From: Ian Campbell @ 2013-06-28  7:39 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Stefano Stabellini

On Thu, 2013-06-27 at 19:04 +0100, Ian Jackson wrote:
> Ian Jackson writes ("Re: [PATCH v2] introduce a cache options for PV disks"):
> > Here's the result.  I took Anthony Ligouri's ack off it.
> 
> And the libxl half.
> 
> commit 98c0ba29e33ae875e8b854fa1ff58862ac09a530
> Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Date:   Thu Jun 27 12:40:20 2013 +0100
> 
>     libxl: introduce an option for disabling the non-O_DIRECT workaround
>     
>     Document and implement a new option that permits disk backends which
>     would otherwise have to avoid O_DIRECT (because of the network memory
>     lifetime bug) to use it anyway.  This is:
>      direct-io-safe   in the xl domain disk config specification
>      direct_io_safe   in the libxl disk API
>      direct-io-safe   in the backend xenstore interface
>     
>     Add a reference to xen/include/public/io/blkif.h in
>     docs/misc/vbd-interface.txt.
>     
>     Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>     Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

I assume you will apply yourself. Since this mail is not git
format-patch compliant I don't think I can feed it to git am without
having to massage the commit message.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2013-06-28  7:39 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-27 11:40 [PATCH v2] introduce a cache options for PV disks Stefano Stabellini
2013-06-27 14:21 ` Ian Jackson
2013-06-27 16:09   ` Ian Jackson
2013-06-27 16:24     ` Ian Campbell
2013-06-27 16:30       ` Ian Jackson
2013-06-27 16:34         ` Ian Jackson
2013-06-27 16:41           ` Ian Campbell
2013-06-27 16:50             ` Ian Jackson
2013-06-27 16:58               ` Ian Campbell
2013-06-27 16:41         ` Ian Campbell
2013-06-27 16:35       ` Ian Jackson
2013-06-27 16:20 ` Ian Jackson
2013-06-27 17:06   ` Stefano Stabellini
2013-06-27 17:13     ` Stefano Stabellini
2013-06-27 17:44     ` Ian Jackson
2013-06-27 17:50       ` Stefano Stabellini
2013-06-27 17:59         ` Ian Jackson
2013-06-27 18:01         ` Ian Jackson
2013-06-27 18:04           ` Ian Jackson
2013-06-28  7:39             ` Ian Campbell
2013-06-27 18:12           ` Stefano Stabellini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.