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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel