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@xxxxxxxxxx> > > --- > > 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@xxxxxxxxxxxxx> Thanks, Erik -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list