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

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(a)redhat.com&gt; > --- > 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... :-)

> + 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(a)linux.ibm.com&gt;