[libvirt PATCH 0/3] Small virtiofs fixes (virtio-fs epopee)

Today we fix a bug: https://bugzilla.redhat.com/show_bug.cgi?id=1969232 and an issue: https://gitlab.com/libvirt/libvirt/-/issues/178 Ján Tomko (3): conf: require target for external virtiofsd docs: fix filesystem schema indentation conf: reject duplicate virtiofs tags docs/formatdomain.rst | 1 + docs/kbase/virtiofs.rst | 1 + docs/schemas/domaincommon.rng | 32 +++++++++---------- src/conf/domain_conf.c | 2 +- src/conf/domain_validate.c | 30 +++++++++++++++++ tests/qemuxml2argvdata/vhost-user-fs-sock.xml | 1 + 6 files changed, 50 insertions(+), 17 deletions(-) -- 2.31.1

When adding support for externally launched virtiofsd, I was too liberal and did not require a target. But the target is required, because it's passed to the QEMU device, not to virtiofsd. https://bugzilla.redhat.com/show_bug.cgi?id=1969232 Fixes: 12967c3e1333a6e106110f449ccb1e96279b9527 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/formatdomain.rst | 1 + docs/kbase/virtiofs.rst | 1 + src/conf/domain_conf.c | 2 +- tests/qemuxml2argvdata/vhost-user-fs-sock.xml | 1 + 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index da4d93a787..c6dede053f 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3261,6 +3261,7 @@ A directory on the host that can be accessed directly from the guest. <filesystem type='mount'> <driver type='virtiofs' queue='1024'/> <source socket='/tmp/sock'/> + <target dir='tag'/> </filesystem> ... </devices> diff --git a/docs/kbase/virtiofs.rst b/docs/kbase/virtiofs.rst index 8cf7567bf8..6ba7299a72 100644 --- a/docs/kbase/virtiofs.rst +++ b/docs/kbase/virtiofs.rst @@ -180,4 +180,5 @@ control and need to be set by the application running virtiofsd. <filesystem type='mount'/> <driver type='virtiofs' queue='1024'/> <source socket='/var/virtiofsd.sock'/> + <target dir='tag'/> </filesystem> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 139cdfc0a7..ef784575d2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9896,7 +9896,7 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, goto error; } - if (target == NULL && !sock) { + if (target == NULL) { virReportError(VIR_ERR_NO_TARGET, source ? "%s" : NULL, source); goto error; diff --git a/tests/qemuxml2argvdata/vhost-user-fs-sock.xml b/tests/qemuxml2argvdata/vhost-user-fs-sock.xml index aef005d3fd..e5a380c9b6 100644 --- a/tests/qemuxml2argvdata/vhost-user-fs-sock.xml +++ b/tests/qemuxml2argvdata/vhost-user-fs-sock.xml @@ -29,6 +29,7 @@ <filesystem type='mount'> <driver type='virtiofs' queue='1024'/> <source socket='/tmp/sock'/> + <target dir='tag'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </filesystem> <input type='mouse' bus='ps2'/> -- 2.31.1

On Wed, Jun 16, 2021 at 15:58:30 +0200, Ján Tomko wrote:
When adding support for externally launched virtiofsd, I was too liberal and did not require a target.
But the target is required, because it's passed to the QEMU device, not to virtiofsd.
https://bugzilla.redhat.com/show_bug.cgi?id=1969232
Fixes: 12967c3e1333a6e106110f449ccb1e96279b9527 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/formatdomain.rst | 1 + docs/kbase/virtiofs.rst | 1 + src/conf/domain_conf.c | 2 +- tests/qemuxml2argvdata/vhost-user-fs-sock.xml | 1 + 4 files changed, 4 insertions(+), 1 deletion(-)
[...]
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 139cdfc0a7..ef784575d2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9896,7 +9896,7 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, goto error; }
- if (target == NULL && !sock) { + if (target == NULL) { virReportError(VIR_ERR_NO_TARGET, source ? "%s" : NULL, source); goto error;
This will have to end up in a validation functions or existing configs with missing target will vanish which is unacceptable.

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/schemas/domaincommon.rng | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 233805cb66..5ea14b6dbf 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2768,18 +2768,18 @@ <ref name="fsBinary"/> </optional> <element name="source"> - <choice> - <group> - <attribute name="dir"> - <ref name="absDirPath"/> - </attribute> - </group> - <group> - <attribute name="socket"> - <ref name="absFilePath"/> - </attribute> - </group> - </choice> + <choice> + <group> + <attribute name="dir"> + <ref name="absDirPath"/> + </attribute> + </group> + <group> + <attribute name="socket"> + <ref name="absFilePath"/> + </attribute> + </group> + </choice> <empty/> </element> </interleave> @@ -2842,10 +2842,10 @@ </choice> <interleave> <optional> - <element name="target"> - <attribute name="dir"/> - <empty/> - </element> + <element name="target"> + <attribute name="dir"/> + <empty/> + </element> </optional> <optional> <attribute name="accessmode"> -- 2.31.1

On Wed, Jun 16, 2021 at 15:58:31 +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/schemas/domaincommon.rng | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

https://gitlab.com/libvirt/libvirt/-/issues/178 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_validate.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 98202a3adc..9422b00964 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1495,6 +1495,33 @@ virDomainDefIOMMUValidate(const virDomainDef *def) } +static int +virDomainDefFSValidate(const virDomainDef *def) +{ + size_t i; + g_autoptr(GHashTable) dsts = virHashNew(NULL); + + for (i = 0; i < def->nfss; i++) { + const virDomainFSDef *fs = def->fss[i]; + + if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) + continue; + + if (virHashHasEntry(dsts, fs->dst)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("filesystem target '%s' specified twice"), + fs->dst); + return -1; + } + + if (virHashAddEntry(dsts, fs->dst, (void *) 0x1) < 0) + return -1; + } + + return 0; +} + + static int virDomainDefValidateInternal(const virDomainDef *def, virDomainXMLOption *xmlopt) @@ -1541,6 +1568,9 @@ virDomainDefValidateInternal(const virDomainDef *def, if (virDomainNumaDefValidate(def->numa) < 0) return -1; + if (virDomainDefFSValidate(def) < 0) + return -1; + return 0; } -- 2.31.1

On Wed, Jun 16, 2021 at 15:58:32 +0200, Ján Tomko wrote:
https://gitlab.com/libvirt/libvirt/-/issues/178
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_validate.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (2)
-
Ján Tomko
-
Peter Krempa