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

[Erik Skultety]

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?

>
> > +    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