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@xxxxxxxxx> >> --- >> 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. >