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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list