[PATCH 1/1] domain_validate.c: fix virDomainDefFSValidate() when fs->dst is NULL

[Jano Tomko]

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 at 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.
>