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

[Erik Skultety]

On Mon, Nov 12, 2018 at 03:03:34PM +0100, Marc Hartmayer wrote:
> On Mon, Nov 12, 2018 at 01:14 PM +0100, Erik Skultety <eskultet at redhat.com> 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)
> > +{
> > +
> > +    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;
> > +    }
>
> default case is missing? Otherwise looking good.

Yeah, it could only happen due to memory corruption, but you're right we should
stay both consistent and safe, consider it added.

Thanks,
Erik