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