[libvirt] [PATCH 1/2] conf, docs: Add qcow2 cache configuration support

[Liu Qing]

On Tue, Sep 12, 2017 at 05:21:53PM -0400, John Ferlan wrote:
> 
> 
> On 09/07/2017 02:09 AM, Liu Qing wrote:
> > Random write IOPS will drop dramatically if qcow2 l2 cache could not
> > cover the whole disk. This patch gives libvirt user a chance to adjust
> > the qcow2 cache configuration.
> > 
> > Three new qcow2 driver parameters are added. They are l2-cache-size,
> > refcount-cache-size and cache-clean-interval.
> > 
> > The following are from qcow2-cache.txt.
> > The amount of virtual disk that can be mapped by the L2 and refcount
> > caches (in bytes) is:
> > disk_size = l2_cache_size * cluster_size / 8
> > disk_size = refcount_cache_size * cluster_size * 8 / refcount_bits
> > 
> > The parameter "cache-clean-interval" defines an interval (in seconds).
> > All cache entries that haven't been accessed during that interval are
> > removed from memory.
> 
> Suggestion, rewrite a bit:
> 
> Three new qcow2 driver parameters (l2-cache-size, refcount-cache-size
> and cache-clean-interval) are added as attributes to a new <qcow2>
> subelement for a <driver name='qemu' type='qcow2'...> of a <disk>
> element. The QEMU source docs/qcow2-cache.txt provides the guidelines
> for defining/configuring values for each.
> 
> ...
> 
> That is - let's not bother with repeating what's in the document since
> it can change, but provide enough information that someone with
> curiosity would know where to look.
I am thinking add a web link but afraid of 404 error when the link is
not accessable.
A indirect link like docs/qcow2-cache.txt is a good idea.
> 
> Question from me: does "cache='none'" need to be set in the <driver>
> element for this to work properly? Do any of the cache values make
> sense?  If it must be none, then we need to check that. If something
> doesn't make ense, we need to check that too.
The cache attribute in driver element is how qemu will deals with page
cache of qcow2 file on the host. When opening a qcow2 file on host, the
oflag will be dertimined by cache.

L2 and refcount tables are metadatas of qcow2.

So cache='xxx' determines how the guest data will be cached, and
l2/refcount table cache controls the metadata cache.

> >      if (def->src->auth) {
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index ed8cb6e..391aaba 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -1433,6 +1433,12 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
> >              qemuformat = "luks";
> >          virBufferAsprintf(buf, "format=%s,", qemuformat);
> >      }
> > +    if (disk->src->l2_cache_size > 0)
> > +        virBufferAsprintf(buf, "l2-cache-size=%llu,", disk->src->l2_cache_size);
> > +    if (disk->src->refcount_cache_size > 0)
> > +        virBufferAsprintf(buf, "refcount-cache-size=%llu,", disk->src->refcount_cache_size);
> > +    if (disk->src->cache_clean_interval > 0)
> > +        virBufferAsprintf(buf, "cache-clean-interval=%llu,", disk->src->cache_clean_interval);
> 
> These  don't belong in this patch, they belong in the next patch. We're
> not adding the .args here.
> 
> 
> I can make all the above changes for you - not a problem.  But please
> let me know your thoughts on whether we should keep the sizing
> recommendation in formatdomain.html.in or just point at the qemu source
> document. It's a fine line - I like the example, but I also see the
> downside.
A point to source document is fine. I have done all the modifications in
v5. If any missing please let me know. Thanks.
Qing
> 
> John