[libvirt] [PATCH v2 00/15] Move qemu command line controller checks to qemuDomainDeviceDefValidateController* checks
v1: https://www.redhat.com/archives/libvir-list/2017-December/msg00079.html Changes since v1: * Remove USB from the pile of things to move. That simplifies the Skip helper to remove the @flags argument. Even though it seems there was some sort of objection to Skip'ing during Validation I left that call in place since I feel it's better to have one common function rather than cutting out each individual check and placing it into the various ValidateController{IDE|PCI|SATA} helpers. Having two places to maintain such checks is less optimal in my viewpoint. * Be sure to Unref qemuCaps in patch 4 John Ferlan (12): qemu: Introduce qemuDomainDeviceDefValidateController qemu: Introduce qemuDomainDeviceDefSkipController qemu: Use virDomainControllerType in qemuBuildControllerDevStr switch qemu: Move CCW S390 Address check to controller def validate qemu: Introduce qemuDomainDeviceDefValidateControllerSCSI qemu: Introduce qemuDomainDeviceDefValidateControllerPCI qemu: Use virDomainPCIControllerOpts in qemuBuildControllerDevStr qemu: Move PCI command modelName check to controller def validate qemu: Move PCI command modelName TypeToString to controller def validate qemu: Move PCI more command checks to controller def validate qemu: Complete PCI command checks to controller def validate qemu: Introduce qemuDomainDeviceDefValidateControllerSATA Lin Ma (3): tests: Remove use of IDE disk for pseries floppy test tests: Drop IDE controller in CCW qemu: Introduce qemuDomainDeviceDefValidateControllerIDE src/qemu/qemu_command.c | 467 ++------------------ src/qemu/qemu_domain.c | 483 ++++++++++++++++++++- src/qemu/qemu_domain.h | 4 + .../qemuhotplug-base-ccw-live+ccw-virtio.xml | 4 - ...ive-with-2-ccw-virtio+ccw-virtio-1-explicit.xml | 4 - ...live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml | 4 - ...qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml | 4 - ...-live-with-ccw-virtio+ccw-virtio-2-explicit.xml | 4 - ...-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml | 4 - .../qemuhotplug-base-ccw-live-with-ccw-virtio.xml | 4 - .../qemuhotplug-base-ccw-live.xml | 4 - tests/qemumemlocktest.c | 14 + tests/qemuxml2argvdata/disk-floppy-pseries.args | 24 - tests/qemuxml2argvdata/disk-floppy-pseries.xml | 7 - tests/qemuxml2xmltest.c | 5 +- 15 files changed, 549 insertions(+), 487 deletions(-) delete mode 100644 tests/qemuxml2argvdata/disk-floppy-pseries.args -- 2.13.6
Introduce the bare bones helper to validate whether the controller definition is valid. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cd213479c..19e86797e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3893,6 +3893,27 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk) static int +qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, + const virDomainDef *def ATTRIBUTE_UNUSED) +{ + switch ((virDomainControllerType) controller->type) { + case VIR_DOMAIN_CONTROLLER_TYPE_IDE: + case VIR_DOMAIN_CONTROLLER_TYPE_FDC: + case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: + case VIR_DOMAIN_CONTROLLER_TYPE_SATA: + case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: + case VIR_DOMAIN_CONTROLLER_TYPE_CCID: + case VIR_DOMAIN_CONTROLLER_TYPE_USB: + case VIR_DOMAIN_CONTROLLER_TYPE_PCI: + case VIR_DOMAIN_CONTROLLER_TYPE_LAST: + break; + } + + return 0; +} + + +static int qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, const virDomainDef *def, void *opaque ATTRIBUTE_UNUSED) @@ -3936,11 +3957,14 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, ret = qemuDomainDeviceDefValidateDisk(dev->data.disk); break; + case VIR_DOMAIN_DEVICE_CONTROLLER: + ret = qemuDomainDeviceDefValidateController(dev->data.controller, def); + break; + case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: - case VIR_DOMAIN_DEVICE_CONTROLLER: case VIR_DOMAIN_DEVICE_GRAPHICS: case VIR_DOMAIN_DEVICE_HUB: case VIR_DOMAIN_DEVICE_MEMBALLOON: -- 2.13.6
In order to be able to reuse some code for both controller def validation and command line building, create a convenience helper that will skip the same devices for validation that are being skipped for command line building. This is in preference to copying the individual checks in subsequent patches which split out the various command line validation checks into a domain definition validation check. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 32 ++++++-------------------------- src/qemu/qemu_domain.c | 43 ++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_domain.h | 4 ++++ 3 files changed, 52 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6a8da1d58..d7696fb93 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3216,6 +3216,9 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, if (cont->type != contOrder[j]) continue; + if (qemuDomainDeviceDefSkipController(cont, def)) + continue; + /* skip USB controllers with type none.*/ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) { @@ -3223,30 +3226,6 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, continue; } - /* skip pcie-root */ - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && - cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) { - continue; - } - - /* Skip pci-root, except for pSeries guests (which actually - * support more than one PCI Host Bridge per guest) */ - if (!qemuDomainIsPSeries(def) && - cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && - cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) { - continue; - } - - /* first SATA controller on Q35 machines is implicit */ - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA && - cont->idx == 0 && qemuDomainIsQ35(def)) - continue; - - /* first IDE controller is implicit on various machines */ - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && - cont->idx == 0 && qemuDomainHasBuiltinIDE(def)) - continue; - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && cont->model == -1 && !qemuDomainIsQ35(def) && @@ -3268,11 +3247,12 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Multiple legacy USB controllers are " "not supported")); - goto cleanup; + goto cleanup; } usblegacy = true; continue; - } + } + if (qemuBuildControllerDevStr(def, cont, qemuCaps, &devstr, &usbcontroller) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 19e86797e..e4452c419 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3892,10 +3892,51 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk) } +/** + * qemuDomainDeviceDefSkipController: + * @controller: Controller to check + * @def: Domain definition + * + * Returns true if this controller can be skipped for command line + * generation or device validation. + */ +bool +qemuDomainDeviceDefSkipController(const virDomainControllerDef *controller, + const virDomainDef *def) +{ + /* skip pcie-root */ + if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) + return true; + + /* Skip pci-root, except for pSeries guests (which actually + * support more than one PCI Host Bridge per guest) */ + if (!qemuDomainIsPSeries(def) && + controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) + return true; + + /* first SATA controller on Q35 machines is implicit */ + if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA && + controller->idx == 0 && qemuDomainIsQ35(def)) + return true; + + /* first IDE controller is implicit on various machines */ + if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && + controller->idx == 0 && qemuDomainHasBuiltinIDE(def)) + return true; + + return false; +} + + static int qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, - const virDomainDef *def ATTRIBUTE_UNUSED) + const virDomainDef *def) { + if (qemuDomainDeviceDefSkipController(controller, def)) + return 0; + switch ((virDomainControllerType) controller->type) { case VIR_DOMAIN_CONTROLLER_TYPE_IDE: case VIR_DOMAIN_CONTROLLER_TYPE_FDC: diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index ba807ca0b..b1292a1f0 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1000,4 +1000,8 @@ qemuDomainPrepareDiskSource(virConnectPtr conn, qemuDomainObjPrivatePtr priv, virQEMUDriverConfigPtr cfg); +bool +qemuDomainDeviceDefSkipController(const virDomainControllerDef *controller, + const virDomainDef *def); + #endif /* __QEMU_DOMAIN_H__ */ -- 2.13.6
Make sure all types of virDomainControllerType are handled in the switch statement. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d7696fb93..c5066b16b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2695,7 +2695,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, } } - switch (def->type) { + switch ((virDomainControllerType) def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: switch (model) { case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI: @@ -3140,7 +3140,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, "this QEMU binary or machine type")); goto error; - default: + case VIR_DOMAIN_CONTROLLER_TYPE_FDC: + case VIR_DOMAIN_CONTROLLER_TYPE_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported controller type: %s"), virDomainControllerTypeToString(def->type)); -- 2.13.6
Move the call to qemuDomainCheckCCWS390AddressSupport from qemuBuildControllerDevStr to qemuDomainDeviceDefValidateController. This means we will get the qemuCaps from the driver opaque variable passed to qemuDomainDeviceDefValidate. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 4 ---- src/qemu/qemu_domain.c | 19 ++++++++++++++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c5066b16b..fa8585678 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2662,10 +2662,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, *devstr = NULL; - if (!qemuDomainCheckCCWS390AddressSupport(domainDef, def->info, qemuCaps, - "controller")) - return -1; - if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0) return -1; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e4452c419..c6666be73 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3932,11 +3932,16 @@ qemuDomainDeviceDefSkipController(const virDomainControllerDef *controller, static int qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, - const virDomainDef *def) + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) { if (qemuDomainDeviceDefSkipController(controller, def)) return 0; + if (!qemuDomainCheckCCWS390AddressSupport(def, controller->info, qemuCaps, + "controller")) + return -1; + switch ((virDomainControllerType) controller->type) { case VIR_DOMAIN_CONTROLLER_TYPE_IDE: case VIR_DOMAIN_CONTROLLER_TYPE_FDC: @@ -3957,9 +3962,15 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, static int qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, const virDomainDef *def, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { int ret = 0; + virQEMUDriverPtr driver = opaque; + virQEMUCapsPtr qemuCaps = NULL; + + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, + def->emulator))) + return -1; switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_NET: @@ -3999,7 +4010,8 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, break; case VIR_DOMAIN_DEVICE_CONTROLLER: - ret = qemuDomainDeviceDefValidateController(dev->data.controller, def); + ret = qemuDomainDeviceDefValidateController(dev->data.controller, def, + qemuCaps); break; case VIR_DOMAIN_DEVICE_LEASE: @@ -4020,6 +4032,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, break; } + virObjectUnref(qemuCaps); return ret; } -- 2.13.6
From: Lin Ma <lma@suse.com> Adding an IDE controller for a machinetype that has no built-in IDE controller, libvirt will log an error. Currently the machinetype list which returns by qemuDomainMachineHasBuiltinIDE only includes 440fx, malta, sun4u and g3beige. Remove the disk and the .args file since the expectation is the test will fail in qemuxml2argvtest because floppy is not supported on pseries and thus no disk is necessary and no .args file would be created to compare against. Signed-off-by: Lin Ma <lma@suse.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- tests/qemuxml2argvdata/disk-floppy-pseries.args | 24 ------------------------ tests/qemuxml2argvdata/disk-floppy-pseries.xml | 7 ------- 2 files changed, 31 deletions(-) delete mode 100644 tests/qemuxml2argvdata/disk-floppy-pseries.args diff --git a/tests/qemuxml2argvdata/disk-floppy-pseries.args b/tests/qemuxml2argvdata/disk-floppy-pseries.args deleted file mode 100644 index 72a418302..000000000 --- a/tests/qemuxml2argvdata/disk-floppy-pseries.args +++ /dev/null @@ -1,24 +0,0 @@ -LC_ALL=C \ -PATH=/bin \ -HOME=/home/test \ -USER=test \ -LOGNAME=test \ -QEMU_AUDIO_DRV=none \ -/usr/bin/qemu-system-ppc64 \ --name QEMUGuest1 \ --S \ --M pseries \ --m 214 \ --smp 1 \ --uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ --nographic \ --monitor unix:/tmp/test-monitor,server,nowait \ --no-acpi \ --boot c \ --usb \ --drive file=/dev/HostVG/QEMUGuest1,format=raw,if=ide,bus=0,unit=0 \ --drive file=/dev/fd0,format=raw,if=floppy,unit=0 \ --drive file=/tmp/firmware.img,format=raw,if=floppy,unit=1 \ --net none \ --serial none \ --parallel none diff --git a/tests/qemuxml2argvdata/disk-floppy-pseries.xml b/tests/qemuxml2argvdata/disk-floppy-pseries.xml index be0ede6bd..a4191abe1 100644 --- a/tests/qemuxml2argvdata/disk-floppy-pseries.xml +++ b/tests/qemuxml2argvdata/disk-floppy-pseries.xml @@ -14,12 +14,6 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-ppc64</emulator> - <disk type='block' device='disk'> - <driver name='qemu' type='raw'/> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='hda' bus='ide'/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> - </disk> <disk type='block' device='floppy'> <driver name='qemu' type='raw'/> <source dev='/dev/fd0'/> @@ -34,7 +28,6 @@ </disk> <controller type='usb' index='0'/> <controller type='fdc' index='0'/> - <controller type='ide' index='0'/> <controller type='pci' index='0' model='pci-root'/> <memballoon model='none'/> </devices> -- 2.13.6
From: Lin Ma <lma@suse.com> Adding an IDE controller for a machinetype that has no built-in IDE controller, libvirt will log an error. Currently the machinetype list which returns by qemuDomainMachineHasBuiltinIDE only includes 440fx, malta, sun4u and g3beige. Signed-off-by: Lin Ma <lma@suse.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml | 4 ---- ...uhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-explicit.xml | 4 ---- ...muhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml | 4 ---- .../qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml | 4 ---- ...emuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml | 4 ---- .../qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml | 4 ---- .../qemuhotplug-base-ccw-live-with-ccw-virtio.xml | 4 ---- tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml | 4 ---- 8 files changed, 32 deletions(-) diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml index cd03d0e09..75948f324 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml @@ -33,10 +33,6 @@ <alias name='usb'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> - <controller type='ide' index='0'> - <alias name='ide0'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> - </controller> <controller type='scsi' index='0' model='virtio-scsi'> <alias name='scsi0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-explicit.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-explicit.xml index 519d8161f..301658178 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-explicit.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-explicit.xml @@ -43,10 +43,6 @@ <alias name='usb'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> - <controller type='ide' index='0'> - <alias name='ide0'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> - </controller> <controller type='scsi' index='0' model='virtio-scsi'> <alias name='scsi0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml index 7be75f977..bb9d427c2 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml @@ -43,10 +43,6 @@ <alias name='usb'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> - <controller type='ide' index='0'> - <alias name='ide0'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> - </controller> <controller type='scsi' index='0' model='virtio-scsi'> <alias name='scsi0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml index a83f1b5d7..c41f5affd 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml @@ -33,10 +33,6 @@ <alias name='usb'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> - <controller type='ide' index='0'> - <alias name='ide0'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> - </controller> <controller type='scsi' index='0' model='virtio-scsi'> <alias name='scsi0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml index 3e9020751..b1114dba9 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml @@ -42,10 +42,6 @@ <alias name='usb'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> - <controller type='ide' index='0'> - <alias name='ide0'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> - </controller> <controller type='scsi' index='0' model='virtio-scsi'> <alias name='scsi0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml index 3e9020751..b1114dba9 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml @@ -42,10 +42,6 @@ <alias name='usb'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> - <controller type='ide' index='0'> - <alias name='ide0'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> - </controller> <controller type='scsi' index='0' model='virtio-scsi'> <alias name='scsi0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml index 0fa8d036b..3476f61db 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml @@ -32,10 +32,6 @@ <alias name='usb'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> - <controller type='ide' index='0'> - <alias name='ide0'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> - </controller> <controller type='scsi' index='0' model='virtio-scsi'> <alias name='scsi0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml index d4434a18c..3a202d069 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml @@ -23,10 +23,6 @@ <alias name='usb'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> - <controller type='ide' index='0'> - <alias name='ide0'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> - </controller> <controller type='scsi' index='0' model='virtio-scsi'> <alias name='scsi0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> -- 2.13.6
From: Lin Ma <lma@suse.com> Move the IDE controller check from command line building to controller def validation. Cause the IDE case for command line building to generate a failure if called to add an IDE since that shouldn't happen if the Validate code did the right thing. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 16 ---------------- src/qemu/qemu_domain.c | 28 +++++++++++++++++++++++++++- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fa8585678..f8981c64e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3120,22 +3120,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, break; case VIR_DOMAIN_CONTROLLER_TYPE_IDE: - /* Since we currently only support the integrated IDE - * controller on various boards, if we ever get to here, it's - * because some other machinetype had an IDE controller - * specified, or one with a single IDE contraller had multiple - * ide controllers specified. - */ - if (qemuDomainHasBuiltinIDE(domainDef)) - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Only a single IDE controller is supported " - "for this machine type")); - else - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("IDE controllers are unsupported for " - "this QEMU binary or machine type")); - goto error; - case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c6666be73..ce814df79 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3931,10 +3931,33 @@ qemuDomainDeviceDefSkipController(const virDomainControllerDef *controller, static int +qemuDomainDeviceDefValidateControllerIDE(const virDomainDef *def) +{ + /* Since we currently only support the integrated IDE + * controller on various boards, if we ever get to here, it's + * because some other machinetype had an IDE controller + * specified, or one with a single IDE controller had multiple + * IDE controllers specified. + */ + if (qemuDomainHasBuiltinIDE(def)) + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only a single IDE controller is supported " + "for this machine type")); + else + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IDE controllers are unsupported for " + "this QEMU binary or machine type")); + return -1; +} + + +static int qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, const virDomainDef *def, virQEMUCapsPtr qemuCaps) { + int ret = 0; + if (qemuDomainDeviceDefSkipController(controller, def)) return 0; @@ -3944,6 +3967,9 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, switch ((virDomainControllerType) controller->type) { case VIR_DOMAIN_CONTROLLER_TYPE_IDE: + ret = qemuDomainDeviceDefValidateControllerIDE(def); + break; + case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: case VIR_DOMAIN_CONTROLLER_TYPE_SATA: @@ -3955,7 +3981,7 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, break; } - return 0; + return ret; } -- 2.13.6
Move SCSI validation from qemu_command into qemu_domain. This includes the @model reset/check when the controller model hasn't yet been set. While at it modify the switch to account for all virDomainControllerModelSCSI types rather than using the default label. Rename/reorder the args in qemuCheckSCSIControllerIOThreads to match the caller and also remove the unnecessary model check as well as fixing up the comments to remove the previously removed qemuCaps arg. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 94 +++++------------------------------------------ src/qemu/qemu_domain.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 105 insertions(+), 86 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f8981c64e..d373a68de 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2583,52 +2583,6 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def, } -/* qemuCheckSCSIControllerIOThreads: - * @domainDef: Pointer to domain def - * @def: Pointer to controller def - * @qemuCaps: Capabilities - * - * If this controller definition has iothreads set, let's make sure the - * configuration is right before adding to the command line - * - * Returns true if either supported or there are no iothreads for controller; - * otherwise, returns false if configuration is not quite right. - */ -static bool -qemuCheckSCSIControllerIOThreads(const virDomainDef *domainDef, - virDomainControllerDefPtr def) -{ - if (!def->iothread) - return true; - - if (def->model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("IOThreads only supported for virtio-scsi " - "controllers model is '%s'"), - virDomainControllerModelSCSITypeToString(def->model)); - return false; - } - - if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("IOThreads only available for virtio pci and " - "virtio ccw controllers")); - return false; - } - - /* Can we find the controller iothread in the iothreadid list? */ - if (!virDomainIOThreadIDFind(domainDef, def->iothread)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("controller iothread '%u' not defined in iothreadid"), - def->iothread); - return false; - } - - return true; -} - - /** * qemuBuildControllerDevStr: * @domainDef: domain definition @@ -2662,47 +2616,17 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, *devstr = NULL; - if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { - if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0) - return -1; - } - - if (!(def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI && - model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) { - if (def->queues) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("'queues' is only supported by virtio-scsi controller")); - return -1; - } - if (def->cmd_per_lun) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("'cmd_per_lun' is only supported by virtio-scsi controller")); - return -1; - } - if (def->max_sectors) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("'max_sectors' is only supported by virtio-scsi controller")); - return -1; - } - if (def->ioeventfd) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("'ioeventfd' is only supported by virtio-scsi controller")); - return -1; - } - } - switch ((virDomainControllerType) def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: - switch (model) { + if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0) + return -1; + switch ((virDomainControllerModelSCSI) model) { case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI: if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { virBufferAddLit(&buf, "virtio-scsi-ccw"); - if (def->iothread) { - if (!qemuCheckSCSIControllerIOThreads(domainDef, def)) - goto error; + if (def->iothread) virBufferAsprintf(&buf, ",iothread=iothread%u", def->iothread); - } } else if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { virBufferAddLit(&buf, "virtio-scsi-s390"); @@ -2711,12 +2635,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAddLit(&buf, "virtio-scsi-device"); } else { virBufferAddLit(&buf, "virtio-scsi-pci"); - if (def->iothread) { - if (!qemuCheckSCSIControllerIOThreads(domainDef, def)) - goto error; + if (def->iothread) virBufferAsprintf(&buf, ",iothread=iothread%u", def->iothread); - } } if (qemuBuildVirtioOptionsStr(&buf, def->virtio, qemuCaps) < 0) goto error; @@ -2733,7 +2654,10 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078: virBufferAddLit(&buf, "megasas"); break; - default: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported controller model: %s"), virDomainControllerModelSCSITypeToString(def->model)); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ce814df79..07886ae59 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3951,6 +3951,97 @@ qemuDomainDeviceDefValidateControllerIDE(const virDomainDef *def) } +/* qemuDomainCheckSCSIControllerIOThreads: + * @controller: Pointer to controller def + * @def: Pointer to domain def + * + * If this controller definition has iothreads set, let's make sure the + * configuration is right before adding to the command line + * + * Returns true if either supported or there are no iothreads for controller; + * otherwise, returns false if configuration is not quite right. + */ +static bool +qemuDomainCheckSCSIControllerIOThreads(const virDomainControllerDef *controller, + const virDomainDef *def) +{ + if (!controller->iothread) + return true; + + if (controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && + controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads only available for virtio pci and " + "virtio ccw controllers")); + return false; + } + + /* Can we find the controller iothread in the iothreadid list? */ + if (!virDomainIOThreadIDFind(def, controller->iothread)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("controller iothread '%u' not defined in iothreadid"), + controller->iothread); + return false; + } + + return true; +} + + +static int +qemuDomainDeviceDefValidateControllerSCSI(const virDomainControllerDef *controller, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + int model = controller->model; + + if ((qemuDomainSetSCSIControllerModel(def, qemuCaps, &model)) < 0) + return -1; + + if (model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI) { + if (controller->queues) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'queues' is only supported by virtio-scsi controller")); + return -1; + } + if (controller->cmd_per_lun) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'cmd_per_lun' is only supported by virtio-scsi controller")); + return -1; + } + if (controller->max_sectors) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'max_sectors' is only supported by virtio-scsi controller")); + return -1; + } + if (controller->ioeventfd) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'ioeventfd' is only supported by virtio-scsi controller")); + return -1; + } + } + + switch ((virDomainControllerModelSCSI) model) { + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI: + if (!qemuDomainCheckSCSIControllerIOThreads(controller, def)) + return -1; + break; + + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST: + break; + } + + return 0; +} + + static int qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, const virDomainDef *def, @@ -3970,8 +4061,12 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, ret = qemuDomainDeviceDefValidateControllerIDE(def); break; - case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: + ret = qemuDomainDeviceDefValidateControllerSCSI(controller, def, + qemuCaps); + break; + + case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_SATA: case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: case VIR_DOMAIN_CONTROLLER_TYPE_CCID: -- 2.13.6
Move PCI validation checks out of qemu_command into the proper qemu_domain validation helper. Since there's a lot to move, we'll start slow by replicating the check in qemuBuildControllerDevCommandLine to skip the pcie-root and pci-root tests for not pseries guests. Next we'll move the controller index check switch. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 20 -------------------- src/qemu/qemu_domain.c | 36 +++++++++++++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d373a68de..3ecdac688 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2719,26 +2719,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, case VIR_DOMAIN_CONTROLLER_TYPE_PCI: switch ((virDomainControllerModelPCI) def->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: - case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: - if (def->idx == 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("index for pci controllers of model '%s' must be > 0"), - virDomainControllerModelPCITypeToString(def->model)); - goto error; - } - break; - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: - break; - } - switch ((virDomainControllerModelPCI) def->model) { - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: if (def->opts.pciopts.modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || def->opts.pciopts.chassisNr == -1) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 07886ae59..a83375f80 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4043,6 +4043,37 @@ qemuDomainDeviceDefValidateControllerSCSI(const virDomainControllerDef *controll static int +qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller) +{ + virDomainControllerModelPCI model = controller->model; + + switch (model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: + if (controller->idx == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("index for pci controllers of model '%s' must be > 0"), + virDomainControllerModelPCITypeToString(model)); + return -1; + } + break; + + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: + break; + } + + return 0; +} + + +static int qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, const virDomainDef *def, virQEMUCapsPtr qemuCaps) @@ -4066,12 +4097,15 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, qemuCaps); break; + case VIR_DOMAIN_CONTROLLER_TYPE_PCI: + ret = qemuDomainDeviceDefValidateControllerPCI(controller); + break; + case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_SATA: case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: case VIR_DOMAIN_CONTROLLER_TYPE_CCID: case VIR_DOMAIN_CONTROLLER_TYPE_USB: - case VIR_DOMAIN_CONTROLLER_TYPE_PCI: case VIR_DOMAIN_CONTROLLER_TYPE_LAST: break; } -- 2.13.6
Shorten up a few characters and reference the pciopts pointer Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 115 +++++++++++++++++++++++++----------------------- 1 file changed, 59 insertions(+), 56 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3ecdac688..6d52e6abf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2612,6 +2612,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, { virBuffer buf = VIR_BUFFER_INITIALIZER; int model = def->model; + const virDomainPCIControllerOpts *pciopts; const char *modelName = NULL; *devstr = NULL; @@ -2717,24 +2718,26 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, break; case VIR_DOMAIN_CONTROLLER_TYPE_PCI: + pciopts = &def->opts.pciopts; + switch ((virDomainControllerModelPCI) def->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: - if (def->opts.pciopts.modelName + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - def->opts.pciopts.chassisNr == -1) { + pciopts->chassisNr == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pci-bridge options not set")); goto error; } - modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); + modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown pci-bridge model name value %d"), - def->opts.pciopts.modelName); + pciopts->modelName); goto error; } - if (def->opts.pciopts.modelName + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("PCI controller model name '%s' " @@ -2749,26 +2752,26 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, goto error; } virBufferAsprintf(&buf, "%s,chassis_nr=%d,id=%s", - modelName, def->opts.pciopts.chassisNr, + modelName, pciopts->chassisNr, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: - if (def->opts.pciopts.modelName + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - def->opts.pciopts.busNr == -1) { + pciopts->busNr == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pci-expander-bus options not set")); goto error; } - modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); + modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown pci-expander-bus model name value %d"), - def->opts.pciopts.modelName); + pciopts->modelName); goto error; } - if (def->opts.pciopts.modelName + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("PCI controller model name '%s' " @@ -2783,28 +2786,28 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, goto error; } virBufferAsprintf(&buf, "%s,bus_nr=%d,id=%s", - modelName, def->opts.pciopts.busNr, + modelName, pciopts->busNr, def->info.alias); - if (def->opts.pciopts.numaNode != -1) + if (pciopts->numaNode != -1) virBufferAsprintf(&buf, ",numa_node=%d", - def->opts.pciopts.numaNode); + pciopts->numaNode); break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: - if (def->opts.pciopts.modelName + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated dmi-to-pci-bridge options not set")); goto error; } - modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); + modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown dmi-to-pci-bridge model name value %d"), - def->opts.pciopts.modelName); + pciopts->modelName); goto error; } - if (def->opts.pciopts.modelName + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("PCI controller model name '%s' " @@ -2821,22 +2824,22 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: - if (def->opts.pciopts.modelName + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pcie-root-port options not set")); goto error; } - modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); + modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown pcie-root-port model name value %d"), - def->opts.pciopts.modelName); + pciopts->modelName); goto error; } - if ((def->opts.pciopts.modelName != + if ((pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) && - (def->opts.pciopts.modelName != + (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("PCI controller model name '%s' " @@ -2844,7 +2847,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, modelName); goto error; } - if ((def->opts.pciopts.modelName == + if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -2852,7 +2855,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, "controller is not supported in this QEMU binary")); goto error; } - if ((def->opts.pciopts.modelName == + if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT) && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -2862,24 +2865,24 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, } virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s", - modelName, def->opts.pciopts.port, - def->opts.pciopts.chassis, def->info.alias); + modelName, pciopts->port, + pciopts->chassis, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: - if (def->opts.pciopts.modelName + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pcie-switch-upstream-port options not set")); goto error; } - modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); + modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown pcie-switch-upstream-port model name value %d"), - def->opts.pciopts.modelName); + pciopts->modelName); goto error; } - if (def->opts.pciopts.modelName + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("PCI controller model name '%s' " @@ -2897,24 +2900,24 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: - if (def->opts.pciopts.modelName + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - def->opts.pciopts.chassis == -1 || - def->opts.pciopts.port == -1) { + pciopts->chassis == -1 || + pciopts->port == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pcie-switch-downstream-port " "options not set")); goto error; } - modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); + modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown pcie-switch-downstream-port model name value %d"), - def->opts.pciopts.modelName); + pciopts->modelName); goto error; } - if (def->opts.pciopts.modelName + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("PCI controller model name '%s' " @@ -2930,26 +2933,26 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, goto error; } virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s", - modelName, def->opts.pciopts.port, - def->opts.pciopts.chassis, def->info.alias); + modelName, pciopts->port, + pciopts->chassis, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: - if (def->opts.pciopts.modelName + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - def->opts.pciopts.busNr == -1) { + pciopts->busNr == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pcie-expander-bus options not set")); goto error; } - modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); + modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown pcie-expander-bus model name value %d"), - def->opts.pciopts.modelName); + pciopts->modelName); goto error; } - if (def->opts.pciopts.modelName + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("PCI controller model name '%s' " @@ -2964,32 +2967,32 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, goto error; } virBufferAsprintf(&buf, "%s,bus_nr=%d,id=%s", - modelName, def->opts.pciopts.busNr, + modelName, pciopts->busNr, def->info.alias); - if (def->opts.pciopts.numaNode != -1) + if (pciopts->numaNode != -1) virBufferAsprintf(&buf, ",numa_node=%d", - def->opts.pciopts.numaNode); + pciopts->numaNode); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: - if (def->opts.pciopts.modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - def->opts.pciopts.targetIndex == -1) { + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || + pciopts->targetIndex == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pci-root options not set")); goto error; } /* Skip the implicit one */ - if (def->opts.pciopts.targetIndex == 0) + if (pciopts->targetIndex == 0) goto done; - modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); + modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown pci-root model name value %d"), - def->opts.pciopts.modelName); + pciopts->modelName); goto error; } - if (def->opts.pciopts.modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) { + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("PCI controller model name '%s' is not valid for a pci-root"), modelName); @@ -3002,17 +3005,17 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, goto error; } virBufferAsprintf(&buf, "%s,index=%d,id=%s", - modelName, def->opts.pciopts.targetIndex, + modelName, pciopts->targetIndex, def->info.alias); - if (def->opts.pciopts.numaNode != -1) { + if (pciopts->numaNode != -1) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the spapr-pci-host-bridge controller " "doesn't support numa_node on this QEMU binary")); goto error; } - virBufferAsprintf(&buf, ",numa_node=%d", def->opts.pciopts.numaNode); + virBufferAsprintf(&buf, ",numa_node=%d", pciopts->numaNode); } break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: -- 2.13.6
Move the various modelName == NAME_NONE from the command line generation into domain controller validation. Also rather than have multiple cases with the same check, let's make the code more generic, but also note that it was the modelName option that caused the failure. We also have to be sure not to check the PCI models that we don't care about. For the remaining checks in command line building, we can use the field name in the error message to be more specific about what causes the failure. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 52 ++++++++++++------------------------------------- src/qemu/qemu_domain.c | 12 ++++++++++++ 2 files changed, 24 insertions(+), 40 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6d52e6abf..9a885bb1c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2722,11 +2722,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, switch ((virDomainControllerModelPCI) def->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: - if (pciopts->modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - pciopts->chassisNr == -1) { + if (pciopts->chassisNr == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("autogenerated pci-bridge options not set")); + _("autogenerated pci-bridge option chassisNr not set")); goto error; } @@ -2756,12 +2754,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: - if (pciopts->modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - pciopts->busNr == -1) { + if (pciopts->busNr == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("autogenerated pci-expander-bus options not set")); - goto error; + _("autogenerated pci-expander-bus option busNr not set")); } modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); @@ -2793,13 +2788,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->numaNode); break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: - if (pciopts->modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("autogenerated dmi-to-pci-bridge options not set")); - goto error; - } - modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2824,12 +2812,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: - if (pciopts->modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("autogenerated pcie-root-port options not set")); - goto error; - } modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2869,12 +2851,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->chassis, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: - if (pciopts->modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("autogenerated pcie-switch-upstream-port options not set")); - goto error; - } modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2900,13 +2876,12 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: - if (pciopts->modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - pciopts->chassis == -1 || + if (pciopts->chassis == -1 || pciopts->port == -1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_INTERNAL_ERROR, _("autogenerated pcie-switch-downstream-port " - "options not set")); + "options chassis=%d or port=%d not set"), + pciopts->chassis, pciopts->port); goto error; } @@ -2937,11 +2912,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->chassis, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: - if (pciopts->modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - pciopts->busNr == -1) { + if (pciopts->busNr == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("autogenerated pcie-expander-bus options not set")); + _("autogenerated pcie-expander-bus option busNr not set")); goto error; } @@ -2974,10 +2947,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->numaNode); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: - if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - pciopts->targetIndex == -1) { + if (pciopts->targetIndex == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("autogenerated pci-root options not set")); + _("autogenerated pci-root option targetIndex not set")); goto error; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a83375f80..013266383 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4046,6 +4046,7 @@ static int qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller) { virDomainControllerModelPCI model = controller->model; + const virDomainPCIControllerOpts *pciopts; switch (model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: @@ -4069,6 +4070,17 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle break; } + pciopts = &controller->opts.pciopts; + if (controller->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT && + controller->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST) { + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("autogenerated %s option modelName not set"), + virDomainControllerModelPCITypeToString(controller->model)); + return -1; + } + } + return 0; } -- 2.13.6
Similar to the checking the modelName vs. NAME_NONE, let's make the ModelNameTypeToString check more generic too within the checking done in controller validation (with the same ignore certain models. NB: We need to keep the ModelNameTypeToString fetch in command line validation since we use it, but at least we can assume it returns something valid now. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 59 +++---------------------------------------------- src/qemu/qemu_domain.c | 10 +++++++++ 2 files changed, 13 insertions(+), 56 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9a885bb1c..7f2102644 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2719,6 +2719,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, case VIR_DOMAIN_CONTROLLER_TYPE_PCI: pciopts = &def->opts.pciopts; + if (def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT && + def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST) + modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); switch ((virDomainControllerModelPCI) def->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: @@ -2728,13 +2731,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, goto error; } - modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); - if (!modelName) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown pci-bridge model name value %d"), - pciopts->modelName); - goto error; - } if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -2759,13 +2755,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, _("autogenerated pci-expander-bus option busNr not set")); } - modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); - if (!modelName) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown pci-expander-bus model name value %d"), - pciopts->modelName); - goto error; - } if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -2788,13 +2777,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->numaNode); break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: - modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); - if (!modelName) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown dmi-to-pci-bridge model name value %d"), - pciopts->modelName); - goto error; - } if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -2812,13 +2794,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: - modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); - if (!modelName) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown pcie-root-port model name value %d"), - pciopts->modelName); - goto error; - } if ((pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) && (pciopts->modelName != @@ -2851,13 +2826,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->chassis, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: - modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); - if (!modelName) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown pcie-switch-upstream-port model name value %d"), - pciopts->modelName); - goto error; - } if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -2885,13 +2853,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, goto error; } - modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); - if (!modelName) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown pcie-switch-downstream-port model name value %d"), - pciopts->modelName); - goto error; - } if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -2918,13 +2879,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, goto error; } - modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); - if (!modelName) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown pcie-expander-bus model name value %d"), - pciopts->modelName); - goto error; - } if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -2957,13 +2911,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, if (pciopts->targetIndex == 0) goto done; - modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); - if (!modelName) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown pci-root model name value %d"), - pciopts->modelName); - goto error; - } if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("PCI controller model name '%s' is not valid for a pci-root"), diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 013266383..5f417d5d7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4047,6 +4047,7 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle { virDomainControllerModelPCI model = controller->model; const virDomainPCIControllerOpts *pciopts; + const char *modelName = NULL; switch (model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: @@ -4079,6 +4080,15 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle virDomainControllerModelPCITypeToString(controller->model)); return -1; } + + modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); + if (!modelName) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown %s modelName value %d"), + virDomainControllerModelPCITypeToString(controller->model), + pciopts->modelName); + return -1; + } } return 0; -- 2.13.6
Excluding the qemuCaps checks, move the remainder of the checks that validate whether the PCI definition is valid or not into qemuDomainControllerDefValidatePCI. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 96 ---------------------------------- src/qemu/qemu_domain.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 96 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7f2102644..95a6f56e7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2725,20 +2725,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, switch ((virDomainControllerModelPCI) def->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: - if (pciopts->chassisNr == -1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("autogenerated pci-bridge option chassisNr not set")); - goto error; - } - - if (pciopts->modelName - != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("PCI controller model name '%s' " - "is not valid for a pci-bridge"), - modelName); - goto error; - } if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the pci-bridge controller " @@ -2750,19 +2736,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: - if (pciopts->busNr == -1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("autogenerated pci-expander-bus option busNr not set")); - } - - if (pciopts->modelName - != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("PCI controller model name '%s' " - "is not valid for a pci-expander-bus"), - modelName); - goto error; - } if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the pxb controller " @@ -2777,14 +2750,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->numaNode); break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: - if (pciopts->modelName - != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("PCI controller model name '%s' " - "is not valid for a dmi-to-pci-bridge"), - modelName); - goto error; - } if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the dmi-to-pci-bridge (i82801b11-bridge) " @@ -2794,16 +2759,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: - if ((pciopts->modelName != - VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) && - (pciopts->modelName != - VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("PCI controller model name '%s' " - "is not valid for a pcie-root-port"), - modelName); - goto error; - } if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) { @@ -2826,14 +2781,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->chassis, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: - if (pciopts->modelName - != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("PCI controller model name '%s' " - "is not valid for a pcie-switch-upstream-port"), - modelName); - goto error; - } if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the pcie-switch-upstream-port (x3130-upstream) " @@ -2844,23 +2791,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: - if (pciopts->chassis == -1 || - pciopts->port == -1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("autogenerated pcie-switch-downstream-port " - "options chassis=%d or port=%d not set"), - pciopts->chassis, pciopts->port); - goto error; - } - - if (pciopts->modelName - != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("PCI controller model name '%s' " - "is not valid for a pcie-switch-downstream-port"), - modelName); - goto error; - } if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("The pcie-switch-downstream-port " @@ -2873,20 +2803,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->chassis, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: - if (pciopts->busNr == -1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("autogenerated pcie-expander-bus option busNr not set")); - goto error; - } - - if (pciopts->modelName - != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("PCI controller model name '%s' " - "is not valid for a pcie-expander-bus"), - modelName); - goto error; - } if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB_PCIE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the pxb-pcie controller " @@ -2901,22 +2817,10 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->numaNode); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: - if (pciopts->targetIndex == -1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("autogenerated pci-root option targetIndex not set")); - goto error; - } - /* Skip the implicit one */ if (pciopts->targetIndex == 0) goto done; - if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("PCI controller model name '%s' is not valid for a pci-root"), - modelName); - goto error; - } if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the spapr-pci-host-bridge controller " diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5f417d5d7..54cc6ac9c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4049,6 +4049,8 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle const virDomainPCIControllerOpts *pciopts; const char *modelName = NULL; + /* First pass - just check the controller index for the model's + * that we care to check... */ switch (model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: @@ -4091,6 +4093,138 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle } } + /* Second pass - now the model specific checks */ + switch (model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: + if (pciopts->chassisNr == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("autogenerated pci-bridge option chassisNr not set")); + return -1; + } + + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller model name '%s' is not valid " + "for a pci-bridge"), + modelName); + return -1; + } + + break; + + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: + if (pciopts->busNr == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("autogenerated pci-expander-bus option busNr not set")); + return -1; + } + + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller model name '%s' is not valid " + "for a pci-expander-bus"), + modelName); + return -1; + } + + break; + + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller model name '%s' is not valid " + "for a dmi-to-pci-bridge"), + modelName); + return -1; + } + + break; + + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: + if ((pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) && + (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller model name '%s' is not valid " + "for a pcie-root-port"), + modelName); + return -1; + } + + break; + + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller model name '%s' is not valid " + "for a pcie-switch-upstream-port"), + modelName); + return -1; + } + + break; + + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: + if (pciopts->chassis == -1 || pciopts->port == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("autogenerated pcie-switch-downstream-port " + "options chassis=%d or port=%d not set"), + pciopts->chassis, pciopts->port); + return -1; + } + + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller model name '%s' is not valid " + "for a pcie-switch-downstream-port"), + modelName); + return -1; + } + + break; + + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: + if (pciopts->busNr == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("autogenerated pcie-expander-bus option busNr not set")); + return -1; + } + + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller model name '%s' is not valid " + "for a pcie-expander-bus"), + modelName); + return -1; + } + + break; + + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + if (pciopts->targetIndex == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("autogenerated pci-root option targetIndex not set")); + return -1; + } + + /* Skip the implicit one */ + if (pciopts->targetIndex == 0) + return 0; + + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller model name '%s' is not valid " + "for a pci-root"), + modelName); + return 0; + } + + break; + + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: + break; + } + return 0; } -- 2.13.6
Move the qemuCaps checks over to qemuDomainControllerDefValidatePCI and add empty lines between cases so it's a bit easier on the eyes to read. This requires two test updates in order to set the correct capability bit for an xml2xml test as well as setting up the similar capability for the pseries memlocktest. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 78 ++++++------------------------------------------ src/qemu/qemu_domain.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++-- tests/qemumemlocktest.c | 14 +++++++++ tests/qemuxml2xmltest.c | 5 +++- 4 files changed, 104 insertions(+), 72 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 95a6f56e7..414c6a13f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2725,23 +2725,12 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, switch ((virDomainControllerModelPCI) def->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the pci-bridge controller " - "is not supported in this QEMU binary")); - goto error; - } virBufferAsprintf(&buf, "%s,chassis_nr=%d,id=%s", modelName, pciopts->chassisNr, def->info.alias); break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the pxb controller " - "is not supported in this QEMU binary")); - goto error; - } virBufferAsprintf(&buf, "%s,bus_nr=%d,id=%s", modelName, pciopts->busNr, def->info.alias); @@ -2749,66 +2738,28 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, ",numa_node=%d", pciopts->numaNode); break; + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the dmi-to-pci-bridge (i82801b11-bridge) " - "controller is not supported in this QEMU binary")); - goto error; - } virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); break; - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: - if ((pciopts->modelName == - VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the pcie-root-port (ioh3420) " - "controller is not supported in this QEMU binary")); - goto error; - } - if ((pciopts->modelName == - VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the pcie-root-port (pcie-root-port) " - "controller is not supported in this QEMU binary")); - goto error; - } + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s", modelName, pciopts->port, pciopts->chassis, def->info.alias); break; - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the pcie-switch-upstream-port (x3130-upstream) " - "controller is not supported in this QEMU binary")); - goto error; - } + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("The pcie-switch-downstream-port " - "(xio3130-downstream) controller " - "is not supported in this QEMU binary")); - goto error; - } virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s", modelName, pciopts->port, pciopts->chassis, def->info.alias); break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB_PCIE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the pxb-pcie controller " - "is not supported in this QEMU binary")); - goto error; - } virBufferAsprintf(&buf, "%s,bus_nr=%d,id=%s", modelName, pciopts->busNr, def->info.alias); @@ -2816,31 +2767,20 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, ",numa_node=%d", pciopts->numaNode); break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: /* Skip the implicit one */ if (pciopts->targetIndex == 0) goto done; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the spapr-pci-host-bridge controller " - "is not supported in this QEMU binary")); - goto error; - } virBufferAsprintf(&buf, "%s,index=%d,id=%s", modelName, pciopts->targetIndex, def->info.alias); - if (pciopts->numaNode != -1) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the spapr-pci-host-bridge controller " - "doesn't support numa_node on this QEMU binary")); - goto error; - } + if (pciopts->numaNode != -1) virBufferAsprintf(&buf, ",numa_node=%d", pciopts->numaNode); - } break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 54cc6ac9c..4c2daa176 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4043,7 +4043,8 @@ qemuDomainDeviceDefValidateControllerSCSI(const virDomainControllerDef *controll static int -qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller) +qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller, + virQEMUCapsPtr qemuCaps) { virDomainControllerModelPCI model = controller->model; const virDomainPCIControllerOpts *pciopts; @@ -4110,6 +4111,13 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle return -1; } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the pci-bridge controller is not supported " + "in this QEMU binary")); + return -1; + } + break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: @@ -4127,6 +4135,13 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle return -1; } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the pxb controller is not supported in this " + "QEMU binary")); + return -1; + } + break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: @@ -4138,6 +4153,13 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle return -1; } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the dmi-to-pci-bridge (i82801b11-bridge) " + "controller is not supported in this QEMU binary")); + return -1; + } + break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: @@ -4150,6 +4172,22 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle return -1; } + if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the pcie-root-port (ioh3420) controller " + "is not supported in this QEMU binary")); + return -1; + } + + if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the pcie-root-port (pcie-root-port) controller " + "is not supported in this QEMU binary")); + return -1; + } + break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: @@ -4161,6 +4199,13 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle return -1; } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the pcie-switch-upstream-port (x3130-upstream) " + "controller is not supported in this QEMU binary")); + return -1; + } + break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: @@ -4180,6 +4225,14 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle return -1; } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("The pcie-switch-downstream-port " + "(xio3130-downstream) controller is not " + "supported in this QEMU binary")); + return -1; + } + break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: @@ -4197,6 +4250,13 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle return -1; } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB_PCIE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the pxb-pcie controller is not supported " + "in this QEMU binary")); + return -1; + } + break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: @@ -4218,6 +4278,21 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle return 0; } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the spapr-pci-host-bridge controller is not " + "supported in this QEMU binary")); + return -1; + } + + if (pciopts->numaNode != -1 && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the spapr-pci-host-bridge controller doesn't " + "support numa_node in this QEMU binary")); + return -1; + } + break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: @@ -4254,7 +4329,7 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, break; case VIR_DOMAIN_CONTROLLER_TYPE_PCI: - ret = qemuDomainDeviceDefValidateControllerPCI(controller); + ret = qemuDomainDeviceDefValidateControllerPCI(controller, qemuCaps); break; case VIR_DOMAIN_CONTROLLER_TYPE_FDC: diff --git a/tests/qemumemlocktest.c b/tests/qemumemlocktest.c index 62bd25450..bc0b53e6f 100644 --- a/tests/qemumemlocktest.c +++ b/tests/qemumemlocktest.c @@ -63,6 +63,7 @@ mymain(void) { int ret = 0; char *fakerootdir; + virQEMUCapsPtr qemuCaps = NULL; if (VIR_STRDUP_QUIET(fakerootdir, FAKEROOTDIRTEMPLATE) < 0) { fprintf(stderr, "Out of memory\n"); @@ -127,6 +128,16 @@ mymain(void) DO_TEST("pc-locked+hostdev", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED); qemuTestSetHostArch(driver.caps, VIR_ARCH_PPC64); + if (!(qemuCaps = virQEMUCapsNew())) { + ret = -1; + goto cleanup; + } + + virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); + if (qemuTestCapsCacheInsert(driver.qemuCapsCache, qemuCaps) < 0) { + ret = -1; + goto cleanup; + }; DO_TEST("pseries-kvm", 20971520); DO_TEST("pseries-tcg", 0); @@ -140,6 +151,9 @@ mymain(void) DO_TEST("pseries-hardlimit+locked+hostdev", 2147483648); DO_TEST("pseries-locked+hostdev", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED); + cleanup: + virObjectUnref(qemuCaps); + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakerootdir); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 2be8eb2c1..e866fb724 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1310,7 +1310,10 @@ mymain(void) DO_TEST("intel-iommu-machine", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_IOMMU); - DO_TEST("intel-iommu-caching-mode", NONE); + DO_TEST("intel-iommu-caching-mode", + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420); DO_TEST("intel-iommu-eim", NONE); DO_TEST("intel-iommu-device-iotlb", NONE); -- 2.13.6
Move the SATA controller check from command line building to controller def validation. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 6 ------ src/qemu/qemu_domain.c | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 414c6a13f..cc4b138a6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2699,12 +2699,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, break; case VIR_DOMAIN_CONTROLLER_TYPE_SATA: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_AHCI)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("SATA is not supported with this " - "QEMU binary")); - goto error; - } virBufferAsprintf(&buf, "ahci,id=%s", def->info.alias); break; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4c2daa176..80327be11 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4305,6 +4305,18 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle static int +qemuDomainDeviceDefValidateControllerSATA(virQEMUCapsPtr qemuCaps) +{ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_AHCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("SATA is not supported with this QEMU binary")); + return -1; + } + return 0; +} + + +static int qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, const virDomainDef *def, virQEMUCapsPtr qemuCaps) @@ -4332,8 +4344,11 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, ret = qemuDomainDeviceDefValidateControllerPCI(controller, qemuCaps); break; - case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_SATA: + ret = qemuDomainDeviceDefValidateControllerSATA(qemuCaps); + break; + + case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: case VIR_DOMAIN_CONTROLLER_TYPE_CCID: case VIR_DOMAIN_CONTROLLER_TYPE_USB: -- 2.13.6
On 12/06/2017 10:27 AM, John Ferlan wrote:
v1: https://www.redhat.com/archives/libvir-list/2017-December/msg00079.html
Changes since v1:
* Remove USB from the pile of things to move. That simplifies the Skip helper to remove the @flags argument.
Even though it seems there was some sort of objection to Skip'ing during Validation I left that call in place since I feel it's better to have one common function rather than cutting out each individual check and placing it into the various ValidateController{IDE|PCI|SATA} helpers. Having two places to maintain such checks is less optimal in my viewpoint.
* Be sure to Unref qemuCaps in patch 4
Based on Jan's most recent response to my v1: https://www.redhat.com/archives/libvir-list/2017-December/msg00190.html I'll SNACK this series... I've reworked things to not have a common Skip helper, but rather add the various skip logic to the IDE, PCI, and SATA validate functions. I'll also push the 3 patches that were ACK'd from v1 and then post a v3. John
John Ferlan (12): qemu: Introduce qemuDomainDeviceDefValidateController qemu: Introduce qemuDomainDeviceDefSkipController qemu: Use virDomainControllerType in qemuBuildControllerDevStr switch qemu: Move CCW S390 Address check to controller def validate qemu: Introduce qemuDomainDeviceDefValidateControllerSCSI qemu: Introduce qemuDomainDeviceDefValidateControllerPCI qemu: Use virDomainPCIControllerOpts in qemuBuildControllerDevStr qemu: Move PCI command modelName check to controller def validate qemu: Move PCI command modelName TypeToString to controller def validate qemu: Move PCI more command checks to controller def validate qemu: Complete PCI command checks to controller def validate qemu: Introduce qemuDomainDeviceDefValidateControllerSATA
Lin Ma (3): tests: Remove use of IDE disk for pseries floppy test tests: Drop IDE controller in CCW qemu: Introduce qemuDomainDeviceDefValidateControllerIDE
src/qemu/qemu_command.c | 467 ++------------------ src/qemu/qemu_domain.c | 483 ++++++++++++++++++++- src/qemu/qemu_domain.h | 4 + .../qemuhotplug-base-ccw-live+ccw-virtio.xml | 4 - ...ive-with-2-ccw-virtio+ccw-virtio-1-explicit.xml | 4 - ...live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml | 4 - ...qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml | 4 - ...-live-with-ccw-virtio+ccw-virtio-2-explicit.xml | 4 - ...-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml | 4 - .../qemuhotplug-base-ccw-live-with-ccw-virtio.xml | 4 - .../qemuhotplug-base-ccw-live.xml | 4 - tests/qemumemlocktest.c | 14 + tests/qemuxml2argvdata/disk-floppy-pseries.args | 24 - tests/qemuxml2argvdata/disk-floppy-pseries.xml | 7 - tests/qemuxml2xmltest.c | 5 +- 15 files changed, 549 insertions(+), 487 deletions(-) delete mode 100644 tests/qemuxml2argvdata/disk-floppy-pseries.args
participants (1)
-
John Ferlan