[PATCH] qemu: virtiofs: Add pool element to limit thread pool
Daniel Henrique Barboza
danielhb413 at gmail.com
Fri Oct 23 12:10:23 UTC 2020
On 10/20/20 2:54 PM, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma <m.mizuma at jp.fujitsu.com>
>
> virtiofsd has --thread-pool-size=NUM option to limit the
> thread pool size. It will be useful to tune the performance.
>
> Add pool element to use the option like as:
>
> <filesystem type='mount' accessmode='passthrough'>
> <driver type='virtiofs' queue='1024'/>
> <binary path='/usr/libexec/virtiofsd' xattr='on' pool='32'/>
> <source dir='/path'/>
> <target dir='mount_tag'/>
> </filesystem>
>
> Signed-off-by: Masayoshi Mizuma <m.mizuma at jp.fujitsu.com>
> ---
Code looks good. We generally split the docs from the parser/qemu logic
in the commits though. For new options it's also good to have unit tests
covering them as well.
I suggest you split this patch in at least 2 commits. First commit contains
the changes in the /docs files, introducing the new 'pool' option. The
second commit contains the changes in domain_conf and qemu_virtiofs that
implements it.
For unit tests, you'll want something in the lines of what is done in the
case of 'vhost-user-fs-fd-memory' test in qemuxml2argvtest.c. You'll need
a new XML file (e.g. vhost-user-fs-pool.xml) in qemuxml2argvdata that
implements the 'pool' option, then a .args file that will contains the
expected QEMU command line generated by that XML. You can see how it is being
done for 'vhost-user-fs-fd-memory' and go from there. All the extra files
and changes for this new test goes in another patch.
Last but not the least, if you want to make the maintainers life easier, you
can also add an extra patch documenting this new feature in NEWS.rst.
Thanks,
DHB
> docs/formatdomain.rst | 6 ++++--
> docs/schemas/domaincommon.rng | 5 +++++
> src/conf/domain_conf.c | 12 ++++++++++++
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_virtiofs.c | 3 +++
> 5 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index bfa80e4bc2..0f66af5dc3 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -3070,7 +3070,7 @@ A directory on the host that can be accessed directly from the guest.
> </filesystem>
> <filesystem type='mount' accessmode='passthrough'>
> <driver type='virtiofs' queue='1024'/>
> - <binary path='/usr/libexec/virtiofsd' xattr='on'>
> + <binary path='/usr/libexec/virtiofsd' xattr='on' pool='32'>
> <cache mode='always'/>
> <lock posix='on' flock='on'/>
> </binary>
> @@ -3188,7 +3188,9 @@ A directory on the host that can be accessed directly from the guest.
> the use of filesystem extended attributes. Caching can be tuned via the
> ``cache`` element, possible ``mode`` values being ``none`` and ``always``.
> Locking can be controlled via the ``lock`` element - attributes ``posix`` and
> - ``flock`` both accepting values ``on`` or ``off``. ( :since:`Since 6.2.0` )
> + ``flock`` both accepting values ``on`` or ``off``. ( :since:`Since 6.2.0` ).
> + Thread pool size limit can be tuned via the ``pool`` element.
> + ( :since:`Since 6.9.0` )
> ``source``
> The resource on the host that is being accessed in the guest. The ``name``
> attribute must be used with ``type='template'``, and the ``dir`` attribute
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index c25a742581..cd5de2ba80 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2839,6 +2839,11 @@
> <ref name="virOnOff"/>
> </attribute>
> </optional>
> + <optional>
> + <attribute name="pool">
> + <ref name="unsignedInt"/>
> + </attribute>
> + </optional>
> <optional>
> <element name="cache">
> <optional>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index efa5ac527b..9d06f8c75f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -11588,6 +11588,7 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
> g_autofree char *cache = virXPathString("string(./binary/cache/@mode)", ctxt);
> g_autofree char *posix_lock = virXPathString("string(./binary/lock/@posix)", ctxt);
> g_autofree char *flock = virXPathString("string(./binary/lock/@flock)", ctxt);
> + g_autofree char *thread_pool_size = virXPathString("string(./binary/@pool)", ctxt);
> int val;
>
> if (queue_size && virStrToLong_ull(queue_size, NULL, 10, &def->queue_size) < 0) {
> @@ -11597,6 +11598,14 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
> goto error;
> }
>
> + if (thread_pool_size &&
> + virStrToLong_ull(thread_pool_size, NULL, 10, &def->thread_pool_size) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("cannot parse thread pool size '%s' for virtiofs"),
> + thread_pool_size);
> + goto error;
> + }
> +
> if (binary)
> def->binary = virFileSanitizePath(binary);
>
> @@ -26191,6 +26200,9 @@ virDomainFSDefFormat(virBufferPtr buf,
> virTristateSwitchTypeToString(def->xattr));
> }
>
> + if (def->thread_pool_size)
> + virBufferAsprintf(&binaryAttrBuf, " pool='%llu'", def->thread_pool_size);
> +
> if (def->cache != VIR_DOMAIN_FS_CACHE_MODE_DEFAULT) {
> virBufferAsprintf(&binaryBuf, "<cache mode='%s'/>\n",
> virDomainFSCacheModeTypeToString(def->cache));
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index cd344716a3..98ab0a51c7 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -860,6 +860,7 @@ struct _virDomainFSDef {
> bool symlinksResolved;
> char *binary;
> unsigned long long queue_size;
> + unsigned long long thread_pool_size;
> virTristateSwitch xattr;
> virDomainFSCacheMode cache;
> virTristateSwitch posix_lock;
> diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
> index 2e239cad66..e35c278a1b 100644
> --- a/src/qemu/qemu_virtiofs.c
> +++ b/src/qemu/qemu_virtiofs.c
> @@ -126,6 +126,9 @@ qemuVirtioFSBuildCommandLine(virQEMUDriverConfigPtr cfg,
> virCommandPassFD(cmd, *fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> *fd = -1;
>
> + if (fs->thread_pool_size)
> + virCommandAddArgFormat(cmd, "--thread-pool-size=%llu", fs->thread_pool_size);
> +
> virCommandAddArg(cmd, "-o");
> virBufferAddLit(&opts, "source=");
> virQEMUBuildBufferEscapeComma(&opts, fs->src->path);
>
More information about the libvir-list
mailing list