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 <eskul...@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 <eskul...@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to