[libvirt] [PATCH 0/5] Reject parallel ports on s390, and pseries
Hi, this is a simple series to reject parallel ports on s390 architectures, and pseries machines -- both simply do not support them. This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1487499 Pino Toscano (5): tests: qemuxml2argv: fix expected type for usb-bus-missing tests: qemuxml2argv: fail also on unexpected pass qemu: pass the virDomainDef to qemuDomainChrDefValidate qemu: reject parallel ports for s390 archs qemu: reject parallel ports for pseries machines src/qemu/qemu_domain.c | 16 ++++++++++++---- .../qemuxml2argv-pseries-no-parallel.xml | 18 ++++++++++++++++++ .../qemuxml2argv-s390-no-parallel.xml | 22 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 20 +++++++++++++++++--- 4 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-no-parallel.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-no-parallel.xml -- 2.13.5
The guest of usb-bus-missing does not cause a parse error, but a validation issue -- hence, switch from DO_TEST_PARSE_ERROR to DO_TEST_FAILURE. Fixes commit b003b9781b6ae633cfe4fdf6b9620ca246fa2432. Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- tests/qemuxml2argvtest.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d6ada227a..8aac919c8 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1398,9 +1398,9 @@ mymain(void) DO_TEST("usb-port-missing", QEMU_CAPS_USB_HUB, QEMU_CAPS_NODEFCONFIG); - DO_TEST_PARSE_ERROR("usb-bus-missing", - QEMU_CAPS_USB_HUB, - QEMU_CAPS_NODEFCONFIG); + DO_TEST_FAILURE("usb-bus-missing", + QEMU_CAPS_USB_HUB, + QEMU_CAPS_NODEFCONFIG); DO_TEST("usb-ports", QEMU_CAPS_USB_HUB, QEMU_CAPS_NODEFCONFIG); -- 2.13.5
If a test expects either a parse error or a failure but then there is neither a parse error nor a failure, then properly mark the test as failing, instead of failing later on (e.g. trying to open a non-existing .args file). Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- tests/qemuxml2argvtest.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 8aac919c8..3f7d1db3e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -463,6 +463,10 @@ testCompareXMLToArgv(const void *data) goto ok; goto cleanup; } + if (flags & FLAG_EXPECT_PARSE_ERROR) { + VIR_TEST_DEBUG("passed instead of expected parse error"); + goto cleanup; + } priv = vm->privateData; if (virBitmapParse("0-3", &priv->autoNodeset, 4) < 0) @@ -502,6 +506,10 @@ testCompareXMLToArgv(const void *data) goto ok; goto cleanup; } + if (flags & FLAG_EXPECT_FAILURE) { + VIR_TEST_DEBUG("passed instead of expected failure"); + goto cleanup; + } if (!(actualargv = virCommandToString(cmd))) goto cleanup; -- 2.13.5
This will be used to improve the validation for this type of devices. The former @def parameter is renamed to @dev, leaving @def for the virDomainDef (following the style used elsewhere). Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/qemu/qemu_domain.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 72031893f..0b8a66307 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3291,9 +3291,10 @@ qemuDomainChrSourceDefValidate(const virDomainChrSourceDef *def) static int -qemuDomainChrDefValidate(const virDomainChrDef *def) +qemuDomainChrDefValidate(const virDomainChrDef *dev, + const virDomainDef *def ATTRIBUTE_UNUSED) { - if (qemuDomainChrSourceDefValidate(def->source) < 0) + if (qemuDomainChrSourceDefValidate(dev->source) < 0) return -1; return 0; @@ -3334,7 +3335,7 @@ qemuDomainRedirdevDefValidate(const virDomainRedirdevDef *def) static int qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, - const virDomainDef *def ATTRIBUTE_UNUSED, + const virDomainDef *def, void *opaque ATTRIBUTE_UNUSED) { int ret = -1; @@ -3378,7 +3379,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, goto cleanup; } } else if (dev->type == VIR_DOMAIN_DEVICE_CHR) { - if (qemuDomainChrDefValidate(dev->data.chr) < 0) + if (qemuDomainChrDefValidate(dev->data.chr, def) < 0) goto cleanup; } else if (dev->type == VIR_DOMAIN_DEVICE_SMARTCARD) { if (qemuDomainSmartcardDefValidate(dev->data.smartcard) < 0) -- 2.13.5
They are simply not supported on those architectures. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1487499 Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/qemu/qemu_domain.c | 9 ++++++++- .../qemuxml2argv-s390-no-parallel.xml | 22 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-no-parallel.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0b8a66307..6f1b453b2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3292,11 +3292,18 @@ qemuDomainChrSourceDefValidate(const virDomainChrSourceDef *def) static int qemuDomainChrDefValidate(const virDomainChrDef *dev, - const virDomainDef *def ATTRIBUTE_UNUSED) + const virDomainDef *def) { if (qemuDomainChrSourceDefValidate(dev->source) < 0) return -1; + if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL && + ARCH_IS_S390(def->os.arch)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("parallel ports are not supported")); + return -1; + } + return 0; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-no-parallel.xml b/tests/qemuxml2argvdata/qemuxml2argv-s390-no-parallel.xml new file mode 100644 index 000000000..5e6a23fe9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-no-parallel.xml @@ -0,0 +1,22 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch="s390x" machine="s390-ccw-virtio">hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + </disk> + <parallel type='pty'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3f7d1db3e..e9748bf1a 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1947,6 +1947,10 @@ mymain(void) QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST_PARSE_ERROR("s390-no-parallel", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_VIRTIO_S390); DO_TEST("ppc-dtb", QEMU_CAPS_KVM, -- 2.13.5
On 07.09.2017 14:19, Pino Toscano wrote:
They are simply not supported on those architectures.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1487499
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/qemu/qemu_domain.c | 9 ++++++++- .../qemuxml2argv-s390-no-parallel.xml | 22 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-no-parallel.xml
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0b8a66307..6f1b453b2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3292,11 +3292,18 @@ qemuDomainChrSourceDefValidate(const virDomainChrSourceDef *def)
static int qemuDomainChrDefValidate(const virDomainChrDef *dev, - const virDomainDef *def ATTRIBUTE_UNUSED) + const virDomainDef *def) { if (qemuDomainChrSourceDefValidate(dev->source) < 0) return -1;
+ if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL && + ARCH_IS_S390(def->os.arch)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("parallel ports are not supported")); + return -1; + } + return 0; }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-no-parallel.xml b/tests/qemuxml2argvdata/qemuxml2argv-s390-no-parallel.xml new file mode 100644 index 000000000..5e6a23fe9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-no-parallel.xml @@ -0,0 +1,22 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch="s390x" machine="s390-ccw-virtio">hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + </disk> + <parallel type='pty'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3f7d1db3e..e9748bf1a 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1947,6 +1947,10 @@ mymain(void) QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST_PARSE_ERROR("s390-no-parallel", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_VIRTIO_S390);
DO_TEST("ppc-dtb", QEMU_CAPS_KVM,
Reviewed-by: Thomas Huth <thuth@redhat.com>
They are simply not supported on that machine type. Partially-resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1487499 Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/qemu/qemu_domain.c | 2 +- .../qemuxml2argv-pseries-no-parallel.xml | 18 ++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-no-parallel.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6f1b453b2..799d89865 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3298,7 +3298,7 @@ qemuDomainChrDefValidate(const virDomainChrDef *dev, return -1; if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL && - ARCH_IS_S390(def->os.arch)) { + (ARCH_IS_S390(def->os.arch) || qemuDomainIsPSeries(def))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("parallel ports are not supported")); return -1; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-no-parallel.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-no-parallel.xml new file mode 100644 index 000000000..d7efaa72f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-no-parallel.xml @@ -0,0 +1,18 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <console type='pty'> + <address type="spapr-vio"/> + </console> + <parallel type='pty'/> + <memballoon model="none"/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e9748bf1a..345745841 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1757,6 +1757,8 @@ mymain(void) QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-cpu-exact", QEMU_CAPS_NODEFCONFIG); + DO_TEST_PARSE_ERROR("pseries-no-parallel", + QEMU_CAPS_NODEFCONFIG); qemuTestSetHostArch(driver.caps, VIR_ARCH_PPC64); DO_TEST("pseries-cpu-compat", QEMU_CAPS_KVM, -- 2.13.5
On 07.09.2017 14:19, Pino Toscano wrote:
They are simply not supported on that machine type.
Partially-resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1487499
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/qemu/qemu_domain.c | 2 +- .../qemuxml2argv-pseries-no-parallel.xml | 18 ++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-no-parallel.xml
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6f1b453b2..799d89865 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3298,7 +3298,7 @@ qemuDomainChrDefValidate(const virDomainChrDef *dev, return -1;
if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL && - ARCH_IS_S390(def->os.arch)) { + (ARCH_IS_S390(def->os.arch) || qemuDomainIsPSeries(def))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("parallel ports are not supported")); return -1; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-no-parallel.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-no-parallel.xml new file mode 100644 index 000000000..d7efaa72f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-no-parallel.xml @@ -0,0 +1,18 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <console type='pty'> + <address type="spapr-vio"/> + </console> + <parallel type='pty'/> + <memballoon model="none"/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e9748bf1a..345745841 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1757,6 +1757,8 @@ mymain(void) QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-cpu-exact", QEMU_CAPS_NODEFCONFIG); + DO_TEST_PARSE_ERROR("pseries-no-parallel", + QEMU_CAPS_NODEFCONFIG);
qemuTestSetHostArch(driver.caps, VIR_ARCH_PPC64); DO_TEST("pseries-cpu-compat", QEMU_CAPS_KVM,
Reviewed-by: Thomas Huth <thuth@redhat.com>
On 09/07/2017 02:19 PM, Pino Toscano wrote:
Hi,
this is a simple series to reject parallel ports on s390 architectures, and pseries machines -- both simply do not support them. This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1487499
Pino Toscano (5): tests: qemuxml2argv: fix expected type for usb-bus-missing tests: qemuxml2argv: fail also on unexpected pass qemu: pass the virDomainDef to qemuDomainChrDefValidate qemu: reject parallel ports for s390 archs qemu: reject parallel ports for pseries machines
src/qemu/qemu_domain.c | 16 ++++++++++++---- .../qemuxml2argv-pseries-no-parallel.xml | 18 ++++++++++++++++++ .../qemuxml2argv-s390-no-parallel.xml | 22 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 20 +++++++++++++++++--- 4 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-no-parallel.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-no-parallel.xml
ACK series. Michal
On Mon, 2017-09-18 at 14:59 +0200, Michal Privoznik wrote:
Pino Toscano (5): tests: qemuxml2argv: fix expected type for usb-bus-missing tests: qemuxml2argv: fail also on unexpected pass qemu: pass the virDomainDef to qemuDomainChrDefValidate qemu: reject parallel ports for s390 archs qemu: reject parallel ports for pseries machines
src/qemu/qemu_domain.c | 16 ++++++++++++---- .../qemuxml2argv-pseries-no-parallel.xml | 18 ++++++++++++++++++ .../qemuxml2argv-s390-no-parallel.xml | 22 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 20 +++++++++++++++++--- 4 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-no-parallel.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-no-parallel.xml
ACK series.
Do you plan to also push the series? AFAIK Pino doesn't have commit access to libvirt. -- Andrea Bolognani / Red Hat / Virtualization
On 09/21/2017 12:51 PM, Andrea Bolognani wrote:
On Mon, 2017-09-18 at 14:59 +0200, Michal Privoznik wrote:
Pino Toscano (5): tests: qemuxml2argv: fix expected type for usb-bus-missing tests: qemuxml2argv: fail also on unexpected pass qemu: pass the virDomainDef to qemuDomainChrDefValidate qemu: reject parallel ports for s390 archs qemu: reject parallel ports for pseries machines
src/qemu/qemu_domain.c | 16 ++++++++++++---- .../qemuxml2argv-pseries-no-parallel.xml | 18 ++++++++++++++++++ .../qemuxml2argv-s390-no-parallel.xml | 22 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 20 +++++++++++++++++--- 4 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-no-parallel.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-no-parallel.xml
ACK series.
Do you plan to also push the series?
AFAIK Pino doesn't have commit access to libvirt.
Oh, right. He even told me yesterday in person but I completely forgot. Sorry, pushed now. Michal
participants (4)
-
Andrea Bolognani -
Michal Privoznik -
Pino Toscano -
Thomas Huth