Re: [Xen-devel] [PATCH 1/3] qemu-xen-traditional: use O_DIRECT to open disk images for IDE

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2012-03-27 at 18:22 +0100, Ian Jackson wrote:
> Stefano Stabellini writes ("RE: [Xen-devel] [PATCH 1/3] qemu-xen-traditional: 
> use O_DIRECT to open disk images for IDE"):
> > On Tue, 27 Mar 2012, Zhang, Yang Z wrote:
> > > Doesn't cache mode have better performance than NOCACHE?
> > 
> > Actually you are correct. I think that this patch should be dropped from
> > the series. Of course we need O_DIRECT for QDISK otherwise we do loose
> > correctness but considering that IDE should only be used during
> > installation it can stay as it is.
> 
> I don't think this assumption about IDE is correct.  To say that "IDE
> should only be used during installation" is not an excuse for
> providing an IDE controller which violates the usual correctness
> rules.

The changeset which originally made this use BDRV_O_CACHE is below, do
the arguments made there no longer apply? To my non-qemu eye it looks
like hw/ide.c is doing an appropriate amount of bdrv_flush().

I think it is possible that we've incorrectly determined that
BDRV_O_CACHE has issues with correctness?

My recollection is that way-back-when that installation to an emulated
IDE device with O_DIRECT was so slow that it was deemed an acceptable
trade-off, presumably given the understanding that IDE cache control was
working.

I think Stefano measured it again recently, Stefano -- can you share the
numbers you saw?

Ian.

commit 82787c6f689d869ad349df83ec3f58702afe00fe
Author: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Date:   Mon Mar 2 11:21:51 2009 +0000

    Override default cache mode for disk images to write-back
    
    Upstream qemu changed the default cache mode to write-through (ie,
    O_DSYNC) which is much slower.  We do not need this as we have
    explicit control of cacheing with the IDE cache control commands.
    
    Original patch by Yang Zhang modified by Ian Jackson.
    
    Signed-off-by: Yang Zhang <yang.zhang@xxxxxxxxx>
    Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

diff --git a/xenstore.c b/xenstore.c
index 6bfcdbb..928e950 100644
--- a/xenstore.c
+++ b/xenstore.c
@@ -472,7 +472,7 @@ void xenstore_parse_domain_config(int hvm_domid)
 #ifdef CONFIG_STUBDOM
         if (pasprintf(&danger_buf, "%s/device/vbd/%s", danger_path, e_danger[i]
             continue;
-       if (bdrv_open2(bs, danger_buf, 0 /* snapshot */, &bdrv_vbd) == 0) {
+       if (bdrv_open2(bs, danger_buf, BDRV_O_CACHE_WB /* snapshot and write-bac
            pstrcpy(bs->filename, sizeof(bs->filename), params);
        } else
 #endif
@@ -498,7 +498,7 @@ void xenstore_parse_domain_config(int hvm_domid)
                }
            }
             pstrcpy(bs->filename, sizeof(bs->filename), params);
-            if (bdrv_open2(bs, params, 0 /* snapshot */, format) < 0)
+            if (bdrv_open2(bs, params, BDRV_O_CACHE_WB /* snapshot and write-ba
                 fprintf(stderr, "qemu: could not open vbd '%s' or hard disk ima
         }
 

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



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