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(a)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.