From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [Qemu-devel] [PATCH v2] xen_disk: support "direct-io-safe" backend option Date: Fri, 28 Jun 2013 17:16:07 +0100 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: george.dunlap@eu.citrix.com, xen-devel@lists.xensource.com, Ian Jackson List-Id: xen-devel@lists.xenproject.org On Thu, 27 Jun 2013, Stefano Stabellini wrote: > 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. > > The original proposal for a "cache" backend option has been dropped > because it was believed too wide, especially considering that at the > moment the backend doesn't have a way to tell the toolstack that it is > capable of supporting it. > > Signed-off-by: Stefano Stabellini > Signed-off-by: Ian Jackson George, should I go ahead and commit to the qemu-xen tree? > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c > index 247f32f..727f433 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(directiosafe)); > > /* 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; >