Re: [Xen-devel] [PATCH RFC 1/3] xen_disk: handle disk files on ramfs/tmpfs

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 3 Jan 2013, Ian Campbell wrote:
> On Mon, 2012-12-31 at 12:16 +0000, Roger Pau Monne wrote:
> > Files that reside on ramfs or tmpfs cannot be opened with O_DIRECT,
> > if first call to bdrv_open fails with errno = EINVAL, try a second
> > call without BDRV_O_NOCACHE.
> 
> Doesn't that risk spuriously turning of NOCACHE on other sorts of
> devices as well which (potentially) opens up a data loss issue?

I agree, we shouldn't have this kind of critical configuration changes
behind the user's back.

I would rather let the user set the cache attributes, QEMU has already a
command line option for it, but we can't use it directly because
xen_disk gets the configuration solely from xenstore at the moment.

I guess we could add a key pair cache=foobar to the xl disk
configuration spec, that gets translated somehow to a key on xenstore.
Xen_disk would read the key and sets qflags accordingly.
We could use the same cache parameters supported by QEMU, see
bdrv_parse_cache_flags.

As an alternative, we could reuse the already defined "access" key, like
this:

access=rw|nocache

or

access=rw|unsafe


> > Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
> > Cc: xen-devel@xxxxxxxxxxxxx
> > Cc: Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
> > Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > ---
> >  hw/xen_disk.c |   16 +++++++++++++---
> >  1 files changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/xen_disk.c b/hw/xen_disk.c
> > index e6bb2f2..a159ee5 100644
> > --- a/hw/xen_disk.c
> > +++ b/hw/xen_disk.c
> > @@ -562,7 +562,7 @@ static void blk_alloc(struct XenDevice *xendev)
> >  static int blk_init(struct XenDevice *xendev)
> >  {
> >      struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, 
> > xendev);
> > -    int index, qflags, info = 0;
> > +    int index, qflags, info = 0, rc;
> >  
> >      /* read xenstore entries */
> >      if (blkdev->params == NULL) {
> > @@ -625,8 +625,18 @@ static int blk_init(struct XenDevice *xendev)
> >          xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus 
> > setup)\n");
> >          blkdev->bs = bdrv_new(blkdev->dev);
> >          if (blkdev->bs) {
> > -            if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
> > -                        bdrv_find_whitelisted_format(blkdev->fileproto)) 
> > != 0) {
> > +            rc = bdrv_open(blkdev->bs, blkdev->filename, qflags,
> > +                        bdrv_find_whitelisted_format(blkdev->fileproto));
> > +            if (rc != 0 && errno == EINVAL) {
> > +                /* Files on ramfs or tmpfs cannot be opened with O_DIRECT,
> > +                 * remove the BDRV_O_NOCACHE flag, and try to open
> > +                 * the file again.
> > +                 */
> > +                qflags &= ~BDRV_O_NOCACHE;
> > +                rc = bdrv_open(blkdev->bs, blkdev->filename, qflags,
> > +                        bdrv_find_whitelisted_format(blkdev->fileproto));
> > +            }
> > +            if (rc != 0) {
> >                  bdrv_delete(blkdev->bs);
> >                  blkdev->bs = NULL;
> >              }
> 
> 
>

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