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

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@xxxxxxxxxx]
> Sent: Wednesday, March 28, 2012 5:11 PM
> To: Ian Jackson
> Cc: Stefano Stabellini; Zhang, Yang Z; xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH 1/3] qemu-xen-traditional: use O_DIRECT to
> open disk images for IDE
> 
> 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
>          }
> 
IIRC, start several guests at same time are very slowly w/o this patch. 

Yes, correctness is important. But in some cases, the user may put the 
performance at the first place. For example, our QA team has many cases which 
will boot many guest at same time. If using no-cache mode, they need to spend 
more time to wait the case finished. And this is not they wanted.
For KVM, the qemu argument allow user to determine which cache mode they like. 
I think we need to follow it. How about to add an option in config file to 
allow user to choose the cache mode and the default value can be no-cache.

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