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

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(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... :-) 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(a)linux.ibm.com&gt; Thanks, Erik -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list