[libvirt PATCHv2 0/2] Small virtiofs fixes (virtio-fs epopee)

Ján Tomko (2): conf: move filesystem target validation conf: require target for external virtiofsd docs/formatdomain.rst | 1 + docs/kbase/virtiofs.rst | 1 + src/conf/domain_conf.c | 6 ------ src/conf/domain_validate.c | 10 ++++++++++ tests/qemuxml2argvdata/vhost-user-fs-sock.xml | 1 + 5 files changed, 13 insertions(+), 6 deletions(-) -- 2.31.1

Check the presence of the target in the validation phase. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 6 ------ src/conf/domain_validate.c | 8 ++++++++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 139cdfc0a7..f65509d8ec 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9896,12 +9896,6 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, goto error; } - if (target == NULL && !sock) { - virReportError(VIR_ERR_NO_TARGET, - source ? "%s" : NULL, source); - goto error; - } - if (def->type == VIR_DOMAIN_FS_TYPE_RAM) { if (!usage) { virReportError(VIR_ERR_XML_ERROR, "%s", diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 9422b00964..bba5a85657 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2036,6 +2036,14 @@ virDomainShmemDefValidate(const virDomainShmemDef *shmem) static int virDomainFSDefValidate(const virDomainFSDef *fs) { + if (fs->dst == NULL && !fs->sock) { + const char *source = fs->src->path; + + virReportError(VIR_ERR_NO_TARGET, + source ? "%s" : NULL, source); + return -1; + } + if (fs->info.bootIndex && fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 2.31.1

On Wed, Jun 16, 2021 at 17:09:06 +0200, Ján Tomko wrote:
Check the presence of the target in the validation phase.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 6 ------ src/conf/domain_validate.c | 8 ++++++++ 2 files changed, 8 insertions(+), 6 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

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_validate.c | 4 +++- tests/qemuxml2argvdata/vhost-user-fs-sock.xml | 1 + 4 files changed, 6 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_validate.c b/src/conf/domain_validate.c index bba5a85657..2124d25d16 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2036,8 +2036,10 @@ virDomainShmemDefValidate(const virDomainShmemDef *shmem) static int virDomainFSDefValidate(const virDomainFSDef *fs) { - if (fs->dst == NULL && !fs->sock) { + if (fs->dst == NULL) { const char *source = fs->src->path; + if (!source) + source = fs->sock; virReportError(VIR_ERR_NO_TARGET, source ? "%s" : NULL, source); 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 17:09:07 +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_validate.c | 4 +++- tests/qemuxml2argvdata/vhost-user-fs-sock.xml | 1 + 4 files changed, 6 insertions(+), 1 deletion(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

On 6/16/21 5:09 PM, 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
It actually fixes 56dcdec1ac8104f94371c210585bab91eb36395d which currently breaks master.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/formatdomain.rst | 1 + docs/kbase/virtiofs.rst | 1 + src/conf/domain_validate.c | 4 +++- tests/qemuxml2argvdata/vhost-user-fs-sock.xml | 1 + 4 files changed, 6 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_validate.c b/src/conf/domain_validate.c index bba5a85657..2124d25d16 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2036,8 +2036,10 @@ virDomainShmemDefValidate(const virDomainShmemDef *shmem) static int virDomainFSDefValidate(const virDomainFSDef *fs) { - if (fs->dst == NULL && !fs->sock) { + if (fs->dst == NULL) { const char *source = fs->src->path; + if (!source) + source = fs->sock;
virReportError(VIR_ERR_NO_TARGET, source ? "%s" : NULL, source); 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'/>
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On Wed, Jun 16, 2021 at 20:25:44 +0200, Boris Fiuczynski wrote:
On 6/16/21 5:09 PM, 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
It actually fixes 56dcdec1ac8104f94371c210585bab91eb36395d which currently breaks master.
Well, partially. It fixes indeed what Jano put in the commit message originally, but as he pushed commits out of sequence it also introduced another breakage. The real intention is to fix that the virtiofs instance requires a target even when it's running against a externally managed daemon.
participants (3)
-
Boris Fiuczynski
-
Ján Tomko
-
Peter Krempa