[PATCH 1/1] domain_validate.c: fix virDomainDefFSValidate() when fs->dst is NULL
Commit 56dcdec1ac81 ("conf: reject duplicate virtiofs tags") added validation code to reject duplicated virtiofs tags. But the <target> element is not always present, meaning that fs->dst can be NULL at that point. If there is no "<target>" tag then the validation will fail in virHashAddEntry() because fs->dst will be NULL. This is tested in qemuxml2xml vhost-user-fs-sock, which is since then not passing: 1020) QEMU XML-2-XML-inactive vhost-user-fs-sock ... Expected result code=0 but received code=1 FAILED 1021) QEMU XML-2-XML-active vhost-user-fs-sock ... Expected result code=0 but received code=1 FAILED The '<target>' tag is not mandatory, so let's consider that fs->dst being NULL is a feasible scenario an adapt virDomainDefFSValidate() accordingly. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_validate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 9422b00964..cf8b43b845 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1504,7 +1504,7 @@ virDomainDefFSValidate(const virDomainDef *def) for (i = 0; i < def->nfss; i++) { const virDomainFSDef *fs = def->fss[i]; - if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) + if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS || !fs->dst) continue; if (virHashHasEntry(dsts, fs->dst)) { -- 2.31.1
On Wed, Jun 16, 2021 at 18:30:08 -0300, Daniel Henrique Barboza wrote:
Commit 56dcdec1ac81 ("conf: reject duplicate virtiofs tags") added validation code to reject duplicated virtiofs tags. But the <target> element is not always present, meaning that fs->dst can be NULL at that point.
If there is no "<target>" tag then the validation will fail in virHashAddEntry() because fs->dst will be NULL. This is tested in qemuxml2xml vhost-user-fs-sock, which is since then not passing:
1020) QEMU XML-2-XML-inactive vhost-user-fs-sock ... Expected result code=0 but received code=1 FAILED 1021) QEMU XML-2-XML-active vhost-user-fs-sock ... Expected result code=0 but received code=1 FAILED
The '<target>' tag is not mandatory, so let's consider that fs->dst being NULL is a feasible scenario an adapt virDomainDefFSValidate() accordingly.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_validate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 9422b00964..cf8b43b845 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1504,7 +1504,7 @@ virDomainDefFSValidate(const virDomainDef *def) for (i = 0; i < def->nfss; i++) { const virDomainFSDef *fs = def->fss[i];
- if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) + if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS || !fs->dst) continue;
if (virHashHasEntry(dsts, fs->dst)) { --
This should not be necessary once Jano pushes the patch moving the validation of the presence of the virtiofs target, which was originally before the patch that introduced the breakage, but I've requested changes to it. But lets see what Jano thinks about this.
On 6/17/21 9:03 AM, Peter Krempa wrote:
On Wed, Jun 16, 2021 at 18:30:08 -0300, Daniel Henrique Barboza wrote:
Commit 56dcdec1ac81 ("conf: reject duplicate virtiofs tags") added validation code to reject duplicated virtiofs tags. But the <target> element is not always present, meaning that fs->dst can be NULL at that point.
The tag should be mandatory, the test case in question was broken. It should be fixed now. Jano
If there is no "<target>" tag then the validation will fail in virHashAddEntry() because fs->dst will be NULL. This is tested in qemuxml2xml vhost-user-fs-sock, which is since then not passing:
1020) QEMU XML-2-XML-inactive vhost-user-fs-sock ... Expected result code=0 but received code=1 FAILED 1021) QEMU XML-2-XML-active vhost-user-fs-sock ... Expected result code=0 but received code=1 FAILED
The '<target>' tag is not mandatory, so let's consider that fs->dst being NULL is a feasible scenario an adapt virDomainDefFSValidate() accordingly.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_validate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 9422b00964..cf8b43b845 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1504,7 +1504,7 @@ virDomainDefFSValidate(const virDomainDef *def) for (i = 0; i < def->nfss; i++) { const virDomainFSDef *fs = def->fss[i];
- if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) + if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS || !fs->dst) continue;
if (virHashHasEntry(dsts, fs->dst)) { --
This should not be necessary once Jano pushes the patch moving the validation of the presence of the virtiofs target, which was originally before the patch that introduced the breakage, but I've requested changes to it.
But lets see what Jano thinks about this.
On 6/17/21 5:55 AM, Jano Tomko wrote:
On 6/17/21 9:03 AM, Peter Krempa wrote:
On Wed, Jun 16, 2021 at 18:30:08 -0300, Daniel Henrique Barboza wrote:
Commit 56dcdec1ac81 ("conf: reject duplicate virtiofs tags") added validation code to reject duplicated virtiofs tags. But the <target> element is not always present, meaning that fs->dst can be NULL at that point.
The tag should be mandatory, the test case in question was broken.
I see. That was my first guess, but then by reading the docs I got the impression that the 'target' tag was optional for filesystem=mount. In fact, by reading it again now, I see that other optional attributes (e.g. binary) are marked as optional right on the start. Not sure why I thought 'target' was optional first time I read this ...
It should be fixed now.
It is. Thanks!
Jano
If there is no "<target>" tag then the validation will fail in virHashAddEntry() because fs->dst will be NULL. This is tested in qemuxml2xml vhost-user-fs-sock, which is since then not passing:
1020) QEMU XML-2-XML-inactive vhost-user-fs-sock ... Expected result code=0 but received code=1 FAILED 1021) QEMU XML-2-XML-active vhost-user-fs-sock ... Expected result code=0 but received code=1 FAILED
The '<target>' tag is not mandatory, so let's consider that fs->dst being NULL is a feasible scenario an adapt virDomainDefFSValidate() accordingly.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_validate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 9422b00964..cf8b43b845 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1504,7 +1504,7 @@ virDomainDefFSValidate(const virDomainDef *def) for (i = 0; i < def->nfss; i++) { const virDomainFSDef *fs = def->fss[i];
- if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) + if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS || !fs->dst) continue;
if (virHashHasEntry(dsts, fs->dst)) { --
This should not be necessary once Jano pushes the patch moving the validation of the presence of the virtiofs target, which was originally before the patch that introduced the breakage, but I've requested changes to it.
But lets see what Jano thinks about this.
participants (3)
-
Daniel Henrique Barboza -
Jano Tomko -
Peter Krempa