[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Xen-devel] [PATCH RFC 1/3] xen_disk: handle disk files
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [Xen-devel] [PATCH RFC 1/3] xen_disk: handle disk files on ramfs/tmpfs |
Date: |
Fri, 4 Jan 2013 14:54:18 +0000 |
User-agent: |
Alpine 2.02 (DEB 1266 2009-07-14) |
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é <address@hidden>
> > Cc: address@hidden
> > Cc: Stefano Stabellini <address@hidden>
> > Cc: Anthony PERARD <address@hidden>
> > ---
> > 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;
> > }
>
>
>