[libvirt] [PATCH 1/2] qemu: Extract MDEV VFIO PCI validation code into a separate helper

[Boris Fiuczynski]

On 11/13/18 9:27 AM, Erik Skultety wrote:
> On Mon, Nov 12, 2018 at 04:28:09PM +0100, Boris Fiuczynski wrote:
>> On 11/12/18 1:14 PM, Erik Skultety wrote:
>>> Since we'll need to validate other models apart from VFIO PCI too,
>>> having a helper for each model should keep the code base cleaner.
>>>
>>> Signed-off-by: Erik Skultety <eskultet at redhat.com>
>>> ---
>>>    src/qemu/qemu_domain.c | 35 +++++++++++++++++++++++++++++------
>>>    1 file changed, 29 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>>> index e25afdad6b..17d207513d 100644
>>> --- a/src/qemu/qemu_domain.c
>>> +++ b/src/qemu/qemu_domain.c
>>> @@ -4564,11 +4564,11 @@ qemuDomainDeviceDefValidateNetwork(const virDomainNetDef *net)
>>>    static int
>>> -qemuDomainMdevDefValidate(const virDomainHostdevSubsysMediatedDev *mdevsrc,
>>> -                          const virDomainDef *def,
>>> -                          virQEMUCapsPtr qemuCaps)
>>> +qemuDomainMdevDefVFIOPCIValidate(const virDomainHostdevSubsysMediatedDev *dev,
>>> +                                 const virDomainDef *def,
>>> +                                 virQEMUCapsPtr qemuCaps)
>>>    {
>>> -    if (mdevsrc->display == VIR_TRISTATE_SWITCH_ABSENT)
>>> +    if (dev->display == VIR_TRISTATE_SWITCH_ABSENT)
>>>            return 0;
>>>        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VFIO_PCI_DISPLAY)) {
>>> @@ -4578,7 +4578,7 @@ qemuDomainMdevDefValidate(const virDomainHostdevSubsysMediatedDev *mdevsrc,
>>>            return -1;
>>>        }
>>> -    if (mdevsrc->model != VIR_MDEV_MODEL_TYPE_VFIO_PCI) {
>>> +    if (dev->model != VIR_MDEV_MODEL_TYPE_VFIO_PCI) {
>>>            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>>                           _("<hostdev> attribute 'display' is only supported"
>>>                             " with model='vfio-pci'"));
>>> @@ -4586,7 +4586,7 @@ qemuDomainMdevDefValidate(const virDomainHostdevSubsysMediatedDev *mdevsrc,
>>>            return -1;
>>>        }
>>> -    if (mdevsrc->display == VIR_TRISTATE_SWITCH_ON) {
>>> +    if (dev->display == VIR_TRISTATE_SWITCH_ON) {
>>>            if (def->ngraphics == 0) {
>>>                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>>                               _("graphics device is needed for attribute value "
>>> @@ -4599,6 +4599,29 @@ qemuDomainMdevDefValidate(const virDomainHostdevSubsysMediatedDev *mdevsrc,
>>>    }
>>> +static int
>>> +qemuDomainMdevDefValidate(const virDomainHostdevSubsysMediatedDev *mdevsrc,
>>> +                          const virDomainDef *def,
>>> +                          virQEMUCapsPtr qemuCaps)
>>> +{
>>> +
>> Syntax-check does not like the above blank line... :-)
> 
> Wow, good catch, quite interesting though, because I always build my patches on
> a bunch of different distros (a reasonable subset of those in our CI) and none
> of them reported this. What's your setup if may ask?
Fedora 28 on s390 running in a VPATH build make check followed by make 
syntax-check

> 
>>
>>> +    switch ((virMediatedDeviceModelType) mdevsrc->model) {
>>> +    case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
>>> +        return qemuDomainMdevDefVFIOPCIValidate(mdevsrc, def, qemuCaps);
>>> +    case VIR_MDEV_MODEL_TYPE_VFIO_AP:
>>> +        break;
>>> +    case VIR_MDEV_MODEL_TYPE_VFIO_CCW:
>>> +        break;
>>> +    case VIR_MDEV_MODEL_TYPE_LAST:
>>> +        virReportEnumRangeError(virMediatedDeviceModelType,
>>> +                                mdevsrc->model);
>>> +        return -1;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +
>>>    static int
>>>    qemuDomainDeviceDefValidateHostdev(const virDomainHostdevDef *hostdev,
>>>                                       const virDomainDef *def,
>>>
>> Besides Marc question regard the default switch case
>> Reviewed-by: Boris Fiuczynski <fiuczy at linux.ibm.com>
> 
> Thanks,
> Erik
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 


-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294