[PATCH libvirt v1] conf: verify for duplicate hostdevs

[Shalini Chellathurai Saroja]

On 6/30/21 4:49 PM, Michal Prívozník wrote:
> On 6/18/21 12:46 PM, Shalini Chellathurai Saroja wrote:
>> It is possible to define/edit(in shut off state) a domain XML with
>> same hostdev device repeated more than once, as shown below. This
>> behavior is not expected. So, this patch fixes it.
>>
>> vser1:
>> <domain type='kvm'>
>> [...]
>>    <devices>
>>   [...]
>>      <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
>>        <source>
>>          <address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
>>        </source>
>>        <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0005'/>
>>      </hostdev>
>>      <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
>>        <source>
>>          <address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
>>        </source>
>>        <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0006'/>
>>      </hostdev>
>> [...]
>>    </devices>
>> </domain>
>>
>> $ virsh define vser1
>> Domain 'vser1' defined from vser1
>>
>> Signed-off-by: Shalini Chellathurai Saroja <shalini at linux.ibm.com>
>> Reviewed-by: Bjoern Walk <bwalk at linux.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy at linux.ibm.com>
>> ---
>>   src/conf/domain_conf.c                        |  2 +-
>>   src/conf/domain_conf.h                        |  2 +
>>   src/conf/domain_validate.c                    | 21 ++++++++++
>>   src/libvirt_private.syms                      |  1 +
>>   .../hostdev-mdev-duplicate.err                |  1 +
>>   .../hostdev-mdev-duplicate.xml                | 41 +++++++++++++++++++
>>   .../hostdev-pci-duplicate.err                 |  1 +
>>   .../hostdev-pci-duplicate.xml                 | 40 ++++++++++++++++++
>>   .../hostdev-scsi-duplicate.err                |  1 +
>>   .../hostdev-scsi-duplicate.xml                | 40 ++++++++++++++++++
>>   .../hostdev-usb-duplicate.err                 |  1 +
>>   .../hostdev-usb-duplicate.xml                 | 40 ++++++++++++++++++
>>   tests/qemuxml2argvtest.c                      |  8 ++++
>>   13 files changed, 198 insertions(+), 1 deletion(-)
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.err
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.err
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.xml
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.err
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.err
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.xml
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index f65509d8..5746f69e 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -15369,7 +15369,7 @@ virDomainHostdevMatchCaps(virDomainHostdevDef *a,
>>   }
>>   
>>   
>> -static int
>> +int
>>   virDomainHostdevMatch(virDomainHostdevDef *a,
>>                         virDomainHostdevDef *b)
>>   {
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index f706c498..4d9d499b 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -3590,6 +3590,8 @@ virDomainHostdevDef *
>>   virDomainHostdevRemove(virDomainDef *def, size_t i);
>>   int virDomainHostdevFind(virDomainDef *def, virDomainHostdevDef *match,
>>                            virDomainHostdevDef **found);
>> +int virDomainHostdevMatch(virDomainHostdevDef *a,
>> +                          virDomainHostdevDef *b);
>>   
>>   virDomainGraphicsListenDef *
>>   virDomainGraphicsGetListen(virDomainGraphicsDef *def, size_t i);
>> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
>> index 2124d25d..df2ab473 100644
>> --- a/src/conf/domain_validate.c
>> +++ b/src/conf/domain_validate.c
>> @@ -1068,7 +1068,25 @@ virDomainDefDuplicateDiskInfoValidate(const virDomainDef *def)
>>       return 0;
>>   }
>>   
>> +static int
>> +virDomainDefDuplicateHostdevInfoValidate(const virDomainDef *def)
>> +{
>> +    size_t i;
>> +    size_t j;
>>   
>> +    for (i = 0; i < def->nhostdevs; i++) {
>> +        for (j = i + 1; j < def->nhostdevs; j++) {
>> +            if (virDomainHostdevMatch(def->hostdevs[i],
>> +                                      def->hostdevs[j])) {
>> +                virReportError(VIR_ERR_XML_ERROR, "%s",
>> +                    _("Hostdev already exists in the domain configuration"));
>> +                return -1;
>> +            }
>> +        }
>> +    }
>> +
>> +    return 0;
> So this forbids duplicated hostdevs. While there are some devices that
> definitely can not be assigned twice (e.g. PCI devices), I'm not so sure
> about some other types (e.g. iSCSI or scsi-host). But one can also argue
> that such use case is broken. So let's do the following, I'll merge it
> tomorrow, after the release so that we give users the longest window
> possible to complain.
>
> Reviewed-by: Michal Privoznik <mprivozn at redhat.com>
ok, thank you Michal.
>
> Michal
>
-- 
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294