On Thu, Jan 23, 2020 at 18:46:06 +0100, Ján Tomko wrote:
> Add more elements for tuning the virtiofsd daemon
> and the vhost-user-fs device:
>
> <driver type='virtiofs' queue='1024' xattr='on'>
> <binary>/usr/libexec/virtiofsd</binary>
> <cache mode='always' size='2097152' unit='KiB'/>
> <lock posix='off' flock='off'/>
> </driver>
>
> Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx>
> ---
> docs/formatdomain.html.in | 18 ++-
> docs/schemas/domaincommon.rng | 51 ++++++
> src/conf/domain_conf.c | 146 +++++++++++++++++-
> src/conf/domain_conf.h | 16 ++
> src/libvirt_private.syms | 1 +
> .../vhost-user-fs-fd-memory.xml | 6 +-
> .../vhost-user-fs-hugepages.xml | 4 +-
> 7 files changed, 238 insertions(+), 4 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 21f2a92ed6..cfdb1a7a9a 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3922,10 +3922,15 @@
> <readonly/>
> </filesystem>
> <filesystem type='mount' accessmode='passthrough'>
> - <driver type='virtiofs'/>
> + <driver type='virtiofs queue='1024' xattr='on''>
> + <binary>/usr/libexec/virtiofsd</binary>
> + <cache mode='always' size='2097152' unit='KiB'/>
> + <lock posix='off' flock='off'/>
> + </driver>
> <source dir='/path'/>
> <target dir='mount_tag'>
> </filesystem>
> +
> ...
> </devices>
> ...</pre>
> @@ -4046,6 +4051,17 @@
> <a href="#elementsVirtio">Virtio-specific options</a> can also be
> set. (<span class="since">Since 3.5.0</span>)
> </li>
> + <li>
> + For <code>virtiofs</code>, the <code>queue</code> attribute can be used
> + to specify the queue size,
The queue size is ambiguous. Is that in bytes?
> + <code>xattr</code> enables the use of filesystem
> + extended attributes. The <code>binary</code> sub-element contains the path
> + to the virtiofs daemon.
I heard that there are ideas to rewrite the virtiofs daemon after some
time. Should we also expose a 'type' or model or something to prepare
for the inevitable break of command line options between those two?
> + Caching can be tuned via the <code>cache</code> element, possible <code>mode</code>
> + values being <code>none</code> and <code>always</code>.
Size is not mentioned.
> + Locking can be controlled via the <code>lock</code>
> + element - attributes <code>posix</code> and <code>flock</code> both accepting
> + values <code>yes</code> or <code>no</code>. (<span class="since">Since 6.1.0</span>)
> + </li>
> </ul>
> </dd>
[...]
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 94e1fd64b5..e4f20bb0bf 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
[...]
> @@ -11193,12 +11210,39 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
> } else if (virXMLNodeNameEqual(cur, "readonly")) {
> def->readonly = true;
> } else if (virXMLNodeNameEqual(cur, "driver")) {
> + xmlNodePtr child;
> +
> if (!fsdriver)
> fsdriver = virXMLPropString(cur, "type");
> if (!wrpolicy)
> wrpolicy = virXMLPropString(cur, "wrpolicy");
> if (!format)
> format = virXMLPropString(cur, "format");
> + if (!queue_size)
> + queue_size = virXMLPropString(cur, "queue");
> + if (!xattr)
> + xattr = virXMLPropString(cur, "xattr");
> +
> + child = cur->children;
> + while (child != NULL) {
> + if (virXMLNodeNameEqual(child, "cache")) {
> + if (!cache)
> + cache = virXMLPropString(child, "mode");
> + if (!cache_size)
> + cache_size = virXMLPropString(child, "size");
> + if (!cache_unit)
> + cache_unit = virXMLPropString(child, "unit");
> + } else if (virXMLNodeNameEqual(child, "lock")) {
> + if (!posix_lock)
> + posix_lock = virXMLPropString(child, "posix");
> + if (!flock)
> + flock = virXMLPropString(child, "flock");
> + } else if (virXMLNodeNameEqual(child, "binary")) {
> + if (!binary)
> + binary = virXMLNodeContentString(child);
> + }
Please use XPath queries instead of this ancient method.
> + child = child->next;
> + }
>
> if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0)
> goto error;
> @@ -11208,11 +11252,72 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
> }
>
> if (fsdriver) {
> + int val;
> +
> if ((def->fsdriver = virDomainFSDriverTypeFromString(fsdriver)) <= 0) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("unknown fs driver type '%s'"), fsdriver);
> goto error;
> }
> +
> + if (binary)
> + def->binary = virFileSanitizePath(binary);
> +
> + if (queue_size && virStrToLong_ull(queue_size, NULL, 10, &def->queue_size) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("cannot parse queue size '%s' for virtiofs"),
> + cache_size);
> + goto error;
> + }
> +
> + if (xattr) {
> + if ((val = virTristateSwitchTypeFromString(xattr)) <= 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unknown xattr value '%s'"), xattr);
> + goto error;
> + }
> + def->xattr = val;
> + }
> +
> + if (cache) {
> + if ((val = virDomainFSCacheModeTypeFromString(cache)) <= 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("cannot parse cache mode '%s' for virtiofs"),
> + cache);
> + goto error;
> + }
> + def->cache = val;
> + }
> +
> + if (cache_size && virStrToLong_ull(cache_size, NULL, 10, &def->cache_size) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("cannot parse cache size '%s' for virtiofs"),
> + cache_size);
> + goto error;
> + }
> +
> + if (virScaleInteger(&def->cache_size, cache_unit,
> + 1024, ULLONG_MAX) < 0)
> + goto error;
> + def->cache_size = VIR_DIV_UP(def->cache_size, 1024);
virDomainParseScaledValue
> +
> + if (posix_lock) {
> + if ((val = virTristateSwitchTypeFromString(posix_lock)) <= 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unknown posix lock value '%s'"), posix_lock);
> + goto error;
> + }
> + def->posix_lock = val;
> + }
> +
> + if (flock) {
> + if ((val = virTristateSwitchTypeFromString(flock)) <= 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unknown flock value '%s'"), flock);
> + goto error;
> + }
> + def->flock = val;
> + }
> }
>
> if (format) {
> @@ -24998,6 +25103,7 @@ virDomainFSDefFormat(virBufferPtr buf,
> const char *wrpolicy = virDomainFSWrpolicyTypeToString(def->wrpolicy);
> const char *src = def->src->path;
> g_auto(virBuffer) driverAttrBuf = VIR_BUFFER_INITIALIZER;
> + g_auto(virBuffer) driverBuf = VIR_BUFFER_INIT_CHILD(buf);
>
> if (!type) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -25021,7 +25127,11 @@ virDomainFSDefFormat(virBufferPtr buf,
> virBufferAddLit(buf, ">\n");
>
> virBufferAdjustIndent(buf, 2);
> + virBufferAdjustIndent(&driverBuf, 2);
> if (def->fsdriver) {
> + g_auto(virBuffer) lockAttrBuf = VIR_BUFFER_INITIALIZER;
> + g_auto(virBuffer) cacheAttrBuf = VIR_BUFFER_INITIALIZER;
> +
> virBufferAsprintf(&driverAttrBuf, " type='%s'", fsdriver);
>
> if (def->format)
> @@ -25032,11 +25142,45 @@ virDomainFSDefFormat(virBufferPtr buf,
> if (def->wrpolicy)
> virBufferAsprintf(&driverAttrBuf, " wrpolicy='%s'", wrpolicy);
>
> + if (def->queue_size)
if (def->queue_size > 0)
> + virBufferAsprintf(&driverAttrBuf, " queue='%llu'", def->queue_size);
> +
> + if (def->xattr) {
if (def->xattr != VIR_TRISTATE_SWITCH_ABSENT) {
> + virBufferAsprintf(&driverAttrBuf, " xattr='%s'",
> + virTristateSwitchTypeToString(def->xattr));
> + }
> +
> + virBufferEscapeString(&driverBuf, "<binary>%s</binary>\n", def->binary);
> +
> + if (def->cache) {
!= VIR_DOMAIN_FS_CACHE_MODE_DEFAULT
> + virBufferAsprintf(&cacheAttrBuf, " mode='%s'",
> + virDomainFSCacheModeTypeToString(def->cache));
> + }
> +
> + if (def->cache_size) {
> 0
> + virBufferAsprintf(&cacheAttrBuf, " size='%llu' unit='KiB'",
> + def->cache_size);
> + }
> +
> + virXMLFormatElement(&driverBuf, "cache", &cacheAttrBuf, NULL);
> +
> + if (def->posix_lock) {
!= VIR_TRISTATE_SWITCH_ABSENT
> + virBufferAsprintf(&lockAttrBuf, " posix='%s'",
> + virTristateSwitchTypeToString(def->posix_lock));
> + }
> +
> + if (def->flock) {
!= VIR_TRISTATE_SWITCH_ABSENT
> + virBufferAsprintf(&lockAttrBuf, " flock='%s'",
> + virTristateSwitchTypeToString(def->flock));
> + }
> +
> + virXMLFormatElement(&driverBuf, "lock", &lockAttrBuf, NULL);
> }
>
> +
> virDomainVirtioOptionsFormat(&driverAttrBuf, def->virtio);
>
> - virXMLFormatElement(buf, "driver", &driverAttrBuf, NULL);
> + virXMLFormatElement(buf, "driver", &driverAttrBuf, &driverBuf);
>
> switch (def->type) {
> case VIR_DOMAIN_FS_TYPE_MOUNT:
