[libvirt] [PATCH 00/14] Fix serial console behavior on non-x86 architectures

See the last commit for a short explanation of what this series is all about. Andrea Bolognani (10): qemu: Add QEMU_CAPS_DEVICE_SPAPR_VTY conf,qemu: Use type-aware switches where possible qemu: Introduce qemuDomainChrDefPostParse() conf: Run devicePostParse() again for the first serial device conf: Introduce VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE conf: Drop virDomainChrDeviceType.targetTypeAttr conf: Add VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR qemu: Support usb-serial and pci-serial on pSeries conf: Add VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PL011 news: Update for serial console fixes Pino Toscano (4): conf: add VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLP/SCLPLM conf: pass parseFlags down to virDomainDefAddConsoleCompat conf: convert sclp/sclplm <console> as <serial> qemu: switch s390/s390x default console back to serial docs/formatdomain.html.in | 13 +- docs/news.xml | 12 ++ docs/schemas/domaincommon.rng | 4 + src/conf/domain_conf.c | 132 +++++++++++++++----- src/conf/domain_conf.h | 11 +- src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 136 +++++++++++++-------- src/qemu/qemu_domain.c | 95 ++++++++++++-- src/qemu/qemu_domain_address.c | 8 +- src/vz/vz_sdk.c | 5 +- .../qemuargv2xml-console-compat.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-dev.xml | 2 +- .../qemuargv2xmldata/qemuargv2xml-serial-file.xml | 2 +- .../qemuargv2xmldata/qemuargv2xml-serial-many.xml | 4 +- tests/qemuargv2xmldata/qemuargv2xml-serial-pty.xml | 2 +- .../qemuargv2xml-serial-tcp-telnet.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-tcp.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-udp.xml | 4 +- .../qemuargv2xmldata/qemuargv2xml-serial-unix.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-vc.xml | 2 +- tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + ...otplug-console-compat-2-live+console-virtio.xml | 4 +- .../qemuhotplug-console-compat-2-live.xml | 4 +- .../qemuxml2argv-mach-virt-console-native.args | 1 + .../qemuxml2argv-mach-virt-console-native.xml | 17 +++ ... => qemuxml2argv-mach-virt-console-virtio.args} | 15 +-- .../qemuxml2argv-mach-virt-console-virtio.xml | 19 +++ ...muxml2argv-mach-virt-serial+console-native.args | 1 + ...emuxml2argv-mach-virt-serial+console-native.xml | 18 +++ .../qemuxml2argv-mach-virt-serial-compat.args | 1 + .../qemuxml2argv-mach-virt-serial-compat.xml | 19 +++ ...muxml2argv-mach-virt-serial-invalid-machine.xml | 19 +++ ...s => qemuxml2argv-mach-virt-serial-native.args} | 12 +- .../qemuxml2argv-mach-virt-serial-native.xml | 16 +++ .../qemuxml2argv-mach-virt-serial-pci.args | 26 ++++ .../qemuxml2argv-mach-virt-serial-pci.xml | 18 +++ .../qemuxml2argv-mach-virt-serial-usb.args | 27 ++++ .../qemuxml2argv-mach-virt-serial-usb.xml | 21 ++++ .../qemuxml2argv-pseries-basic.args | 2 +- .../qemuxml2argv-pseries-console-native.args | 1 + .../qemuxml2argv-pseries-console-native.xml | 17 +++ ...gs => qemuxml2argv-pseries-console-virtio.args} | 10 +- .../qemuxml2argv-pseries-console-virtio.xml | 19 +++ .../qemuxml2argv-pseries-cpu-compat-power9.args | 2 +- .../qemuxml2argv-pseries-cpu-compat.args | 2 +- .../qemuxml2argv-pseries-cpu-exact.args | 2 +- .../qemuxml2argv-pseries-cpu-le.args | 2 +- .../qemuxml2argv-pseries-panic-missing.args | 2 +- .../qemuxml2argv-pseries-panic-no-address.args | 2 +- ...qemuxml2argv-pseries-serial+console-native.args | 1 + .../qemuxml2argv-pseries-serial+console-native.xml | 18 +++ .../qemuxml2argv-pseries-serial-compat.args | 1 + .../qemuxml2argv-pseries-serial-compat.xml | 19 +++ ...qemuxml2argv-pseries-serial-invalid-machine.xml | 19 +++ ...rgs => qemuxml2argv-pseries-serial-native.args} | 7 +- .../qemuxml2argv-pseries-serial-native.xml | 16 +++ ...c.args => qemuxml2argv-pseries-serial-pci.args} | 7 +- .../qemuxml2argv-pseries-serial-pci.xml | 18 +++ ...c.args => qemuxml2argv-pseries-serial-usb.args} | 8 +- .../qemuxml2argv-pseries-serial-usb.xml | 21 ++++ .../qemuxml2argv-pseries-usb-default.args | 2 +- .../qemuxml2argv-pseries-usb-kbd.args | 2 +- .../qemuxml2argv-pseries-usb-multi.args | 2 +- .../qemuxml2argv-pseries-vio-user-assigned.args | 4 +- .../qemuxml2argvdata/qemuxml2argv-pseries-vio.args | 4 +- ....args => qemuxml2argv-s390-console2serial.args} | 11 +- .../qemuxml2argv-s390-console2serial.xml | 19 +++ ...power9.args => qemuxml2argv-s390-serial-2.args} | 14 +-- .../qemuxml2argv-s390-serial-2.xml | 17 +++ ....args => qemuxml2argv-s390-serial-console.args} | 11 +- .../qemuxml2argv-s390-serial-console.xml | 15 +++ ...es-basic.args => qemuxml2argv-s390-serial.args} | 11 +- .../qemuxml2argvdata/qemuxml2argv-s390-serial.xml | 14 +++ ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-user-aliases.xml | 4 +- tests/qemuxml2argvtest.c | 83 +++++++++++++ .../qemuxml2xmlout-aarch64-virtio-pci-default.xml | 2 +- .../qemuxml2xmlout-bios-nvram-os-interleave.xml | 2 +- .../qemuxml2xmlout-chardev-label.xml | 4 +- .../qemuxml2xmlout-console-compat-auto.xml | 2 +- .../qemuxml2xmlout-console-compat.xml | 2 +- .../qemuxml2xmlout-console-compat2.xml | 2 +- .../qemuxml2xmlout-console-virtio-many.xml | 2 +- .../qemuxml2xmlout-interface-driver.xml | 2 +- .../qemuxml2xmlout-interface-server.xml | 4 +- .../qemuxml2xmlout-mach-virt-console-native.xml | 1 + ...=> qemuxml2xmlout-mach-virt-console-virtio.xml} | 19 +-- ...uxml2xmlout-mach-virt-serial+console-native.xml | 1 + .../qemuxml2xmlout-mach-virt-serial-compat.xml | 29 +++++ .../qemuxml2xmlout-mach-virt-serial-native.xml | 1 + .../qemuxml2xmlout-mach-virt-serial-pci.xml | 42 +++++++ .../qemuxml2xmlout-mach-virt-serial-usb.xml | 39 ++++++ .../qemuxml2xmlout-net-bandwidth.xml | 2 +- .../qemuxml2xmlout-net-bandwidth2.xml | 2 +- .../qemuxml2xmlout-net-coalesce.xml | 2 +- .../qemuxml2xmloutdata/qemuxml2xmlout-net-mtu.xml | 2 +- .../qemuxml2xmlout-panic-pseries.xml | 2 +- .../qemuxml2xmlout-pseries-console-native.xml | 1 + ...l => qemuxml2xmlout-pseries-console-virtio.xml} | 16 +-- .../qemuxml2xmlout-pseries-cpu-compat-power9.xml | 2 +- .../qemuxml2xmlout-pseries-cpu-compat.xml | 2 +- .../qemuxml2xmlout-pseries-cpu-exact.xml | 2 +- .../qemuxml2xmlout-pseries-panic-missing.xml | 2 +- .../qemuxml2xmlout-pseries-panic-no-address.xml | 2 +- ...emuxml2xmlout-pseries-serial+console-native.xml | 1 + .../qemuxml2xmlout-pseries-serial-compat.xml | 1 + ...ml => qemuxml2xmlout-pseries-serial-native.xml} | 8 +- ...g.xml => qemuxml2xmlout-pseries-serial-pci.xml} | 14 +-- ...g.xml => qemuxml2xmlout-pseries-serial-usb.xml} | 11 +- .../qemuxml2xmlout-q35-virt-manager-basic.xml | 2 +- .../qemuxml2xmlout-s390-defaultconsole.xml | 6 +- .../qemuxml2xmlout-s390-serial-2.xml | 29 +++++ ....xml => qemuxml2xmlout-s390-serial-console.xml} | 18 +-- ...tconsole.xml => qemuxml2xmlout-s390-serial.xml} | 18 +-- .../qemuxml2xmlout-serial-spiceport-nospice.xml | 2 +- .../qemuxml2xmlout-serial-spiceport.xml | 2 +- .../qemuxml2xmlout-serial-target-port-auto.xml | 6 +- .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 4 +- .../qemuxml2xmlout-tap-vhost-incorrect.xml | 2 +- .../qemuxml2xmlout-tap-vhost.xml | 2 +- .../qemuxml2xmlout-vhost_queues.xml | 2 +- tests/qemuxml2xmltest.c | 54 ++++++++ 125 files changed, 1186 insertions(+), 284 deletions(-) create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-native.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-native.xml copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-basic.args => qemuxml2argv-mach-virt-console-virtio.args} (53%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-virtio.xml create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial+console-native.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial+console-native.xml create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-compat.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-compat.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-invalid-machine.xml copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-basic.args => qemuxml2argv-mach-virt-serial-native.args} (62%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-native.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-pci.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-pci.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-usb.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-usb.xml create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-pseries-console-native.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-console-native.xml copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-basic.args => qemuxml2argv-pseries-console-virtio.args} (59%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-console-virtio.xml create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial+console-native.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial+console-native.xml create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-compat.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-compat.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-invalid-machine.xml copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-basic.args => qemuxml2argv-pseries-serial-native.args} (70%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-native.xml copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-basic.args => qemuxml2argv-pseries-serial-pci.args} (70%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-pci.xml copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-basic.args => qemuxml2argv-pseries-serial-usb.args} (65%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-usb.xml copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-basic.args => qemuxml2argv-s390-console2serial.args} (71%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.xml copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-cpu-compat-power9.args => qemuxml2argv-s390-serial-2.args} (62%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-serial-2.xml copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-basic.args => qemuxml2argv-s390-serial-console.args} (71%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.xml copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-basic.args => qemuxml2argv-s390-serial.args} (71%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-serial.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-console-native.xml copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-s390-defaultconsole.xml => qemuxml2xmlout-mach-virt-console-virtio.xml} (50%) create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial+console-native.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-compat.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-native.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-pci.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-usb.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-console-native.xml copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-panic-pseries.xml => qemuxml2xmlout-pseries-console-virtio.xml} (75%) create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial+console-native.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-compat.xml copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-pseries-panic-missing.xml => qemuxml2xmlout-pseries-serial-native.xml} (82%) copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-pseries-panic-missing.xml => qemuxml2xmlout-pseries-serial-pci.xml} (74%) copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-pseries-panic-missing.xml => qemuxml2xmlout-pseries-serial-usb.xml} (75%) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-2.xml copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-s390-defaultconsole.xml => qemuxml2xmlout-s390-serial-console.xml} (50%) copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-s390-defaultconsole.xml => qemuxml2xmlout-s390-serial.xml} (50%) -- 2.13.6

Up until now we assumed the spapr-vty device would always be present, which is not very nice. Check for its availability before using it instead. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 6 ++++++ tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemuxml2argvtest.c | 12 ++++++++++++ 7 files changed, 24 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 1badadbc2..1e69a150a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -444,6 +444,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "vxhs", "virtio-blk.num-queues", "machine.pseries.resize-hpt", + "spapr-vty", ); @@ -1671,6 +1672,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "pcie-root-port", QEMU_CAPS_DEVICE_PCIE_ROOT_PORT }, { "qemu-xhci", QEMU_CAPS_DEVICE_QEMU_XHCI }, { "spapr-pci-host-bridge", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE }, + { "spapr-vty", QEMU_CAPS_DEVICE_SPAPR_VTY }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f0e2e9016..31abb0768 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -430,6 +430,7 @@ typedef enum { QEMU_CAPS_VXHS, /* -drive file.driver=vxhs via query-qmp-schema */ QEMU_CAPS_VIRTIO_BLK_NUM_QUEUES, /* virtio-blk-*.num-queues */ QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT, /* -machine pseries,resize-hpt */ + QEMU_CAPS_DEVICE_SPAPR_VTY, /* -device spapr-vty */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 56729e498..4c05a5f66 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10196,6 +10196,12 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, if (qemuDomainIsPSeries(def)) { if (serial->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && serial->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_VTY)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spapr-vty not supported in this QEMU binary")); + goto error; + } + virBufferAsprintf(&cmd, "spapr-vty,chardev=char%s", serial->info.alias); } diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml index b0ee3f152..b7b80799c 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml @@ -178,6 +178,7 @@ <flag name='vxhs'/> <flag name='virtio-blk.num-queues'/> <flag name='machine.pseries.resize-hpt'/> + <flag name='spapr-vty'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <package> (v2.10.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml index f1c9fc98a..5ff8598fc 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml @@ -167,6 +167,7 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='spapr-vty'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml index 786cea8ea..e1b0074c9 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml @@ -173,6 +173,7 @@ <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> <flag name='virtio-blk.num-queues'/> + <flag name='spapr-vty'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0)</package> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1bedc6874..632c59b7b 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1753,50 +1753,62 @@ mymain(void) DO_TEST_PARSE_ERROR("seclabel-device-duplicates", NONE); DO_TEST("pseries-basic", + QEMU_CAPS_DEVICE_SPAPR_VTY, QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-vio", + QEMU_CAPS_DEVICE_SPAPR_VTY, QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-usb-default", QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_SPAPR_VTY, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_PCI_MULTIFUNCTION); DO_TEST("pseries-usb-multi", QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_SPAPR_VTY, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_PCI_MULTIFUNCTION); DO_TEST("pseries-vio-user-assigned", + QEMU_CAPS_DEVICE_SPAPR_VTY, QEMU_CAPS_NODEFCONFIG); DO_TEST_PARSE_ERROR("pseries-vio-address-clash", QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-nvram", QEMU_CAPS_DEVICE_NVRAM); DO_TEST("pseries-usb-kbd", QEMU_CAPS_PCI_OHCI, QEMU_CAPS_DEVICE_USB_KBD, + QEMU_CAPS_DEVICE_SPAPR_VTY, QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-cpu-exact", + QEMU_CAPS_DEVICE_SPAPR_VTY, 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, + QEMU_CAPS_DEVICE_SPAPR_VTY, QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-cpu-le", QEMU_CAPS_KVM, + QEMU_CAPS_DEVICE_SPAPR_VTY, QEMU_CAPS_NODEFCONFIG); DO_TEST_FAILURE("pseries-cpu-compat-power9", QEMU_CAPS_KVM); qemuTestSetHostCPU(driver.caps, cpuPower9); DO_TEST("pseries-cpu-compat-power9", QEMU_CAPS_KVM, + QEMU_CAPS_DEVICE_SPAPR_VTY, QEMU_CAPS_NODEFCONFIG); qemuTestSetHostCPU(driver.caps, NULL); qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE); DO_TEST("pseries-panic-missing", + QEMU_CAPS_DEVICE_SPAPR_VTY, QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-panic-no-address", + QEMU_CAPS_DEVICE_SPAPR_VTY, QEMU_CAPS_NODEFCONFIG); DO_TEST_FAILURE("pseries-panic-address", QEMU_CAPS_NODEFCONFIG); -- 2.13.6

On Wed, Nov 15, 2017 at 12:50:04PM +0100, Andrea Bolognani wrote:
Up until now we assumed the spapr-vty device would always be present, which is not very nice. Check for its availability before using it instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 6 ++++++ tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemuxml2argvtest.c | 12 ++++++++++++ 7 files changed, 24 insertions(+)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

The compiler can warn us if we add a value to the virDomainChrSerialTargetType enumeration but forget to handle it properly in the code. Let's take advantage of that. This commit is best viewed with 'git diff -w'. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 47 ++++++++++++++++++++++++++++++----------------- src/qemu/qemu_command.c | 7 ++++++- 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 62d0a1683..9da8dd646 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4035,26 +4035,39 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def) def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; } } else if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && def->nserials > 0 && - def->serials[0]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && - def->serials[0]->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA) { - /* Create a stub console to match the serial port. - * console[0] either does not exist - * or has a different type than SERIAL or NONE. - */ - virDomainChrDefPtr chr; - if (!(chr = virDomainChrDefNew(NULL))) - return -1; + def->serials[0]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL) { - if (VIR_INSERT_ELEMENT(def->consoles, - 0, - def->nconsoles, - chr) < 0) { - virDomainChrDefFree(chr); - return -1; + switch ((virDomainChrSerialTargetType) def->serials[0]->targetType) { + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: { + + /* Create a stub console to match the serial port. + * console[0] either does not exist + * or has a different type than SERIAL or NONE. + */ + virDomainChrDefPtr chr; + if (!(chr = virDomainChrDefNew(NULL))) + return -1; + + if (VIR_INSERT_ELEMENT(def->consoles, + 0, + def->nconsoles, + chr) < 0) { + virDomainChrDefFree(chr); + return -1; + } + + def->consoles[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; + def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; + + break; } - def->consoles[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; - def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: + /* Nothing to do */ + break; + } } return 0; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4c05a5f66..010c2992f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10206,7 +10206,7 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, serial->info.alias); } } else { - switch (serial->targetType) { + switch ((virDomainChrSerialTargetType) serial->targetType) { case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_SERIAL)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -10245,6 +10245,11 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, goto error; } break; + + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Invalid target type for serial device")); + goto error; } virBufferAsprintf(&cmd, "%s,chardev=char%s,id=%s", -- 2.13.6

On Wed, Nov 15, 2017 at 12:50:05PM +0100, Andrea Bolognani wrote:
The compiler can warn us if we add a value to the virDomainChrSerialTargetType enumeration but forget to handle it properly in the code. Let's take advantage of that.
This commit is best viewed with 'git diff -w'.
This sentence should be placed under "---" as a note and not part of the commit message.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 47 ++++++++++++++++++++++++++++++----------------- src/qemu/qemu_command.c | 7 ++++++- 2 files changed, 36 insertions(+), 18 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

On Thu, 2017-11-16 at 11:27 +0100, Pavel Hrdina wrote:
On Wed, Nov 15, 2017 at 12:50:05PM +0100, Andrea Bolognani wrote:
The compiler can warn us if we add a value to the virDomainChrSerialTargetType enumeration but forget to handle it properly in the code. Let's take advantage of that.
This commit is best viewed with 'git diff -w'.
This sentence should be placed under "---" as a note and not part of the commit message.
I thought that hint could be useful to people looking at the commit a few years down the line just as it's useful to the reviewer right now, but I can leave it out if you prefer :) -- Andrea Bolognani / Red Hat / Virtualization

On Thu, Nov 16, 2017 at 01:01:57PM +0100, Andrea Bolognani wrote:
On Thu, 2017-11-16 at 11:27 +0100, Pavel Hrdina wrote:
On Wed, Nov 15, 2017 at 12:50:05PM +0100, Andrea Bolognani wrote:
The compiler can warn us if we add a value to the virDomainChrSerialTargetType enumeration but forget to handle it properly in the code. Let's take advantage of that.
This commit is best viewed with 'git diff -w'.
This sentence should be placed under "---" as a note and not part of the commit message.
I thought that hint could be useful to people looking at the commit a few years down the line just as it's useful to the reviewer right now, but I can leave it out if you prefer :)
I'm used to putting it as note, but on the other hand it might be useful to keep it in commit message. Feel free to leave it there :). Pavel

Having a separate function for char device handling is better than adding even more code to qemuDomainDeviceDefPostParse(). Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_domain.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cc7596bad..2d5eee01e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4036,6 +4036,19 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, return 0; } +static int +qemuDomainChrDefPostParse(virDomainChrDefPtr chr, + const virDomainDef *def) +{ + /* set the default console type for S390 arches */ + if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE && + ARCH_IS_S390(def->os.arch)) { + chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; + } + + return 0; +} static int qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, @@ -4096,13 +4109,6 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, } } - /* set the default console type for S390 arches */ - if (dev->type == VIR_DOMAIN_DEVICE_CHR && - dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && - dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE && - ARCH_IS_S390(def->os.arch)) - dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; - /* clear auto generated unix socket path for inactive definitions */ if ((parseFlags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && dev->type == VIR_DOMAIN_DEVICE_CHR) { @@ -4154,6 +4160,11 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, qemuDomainShmemDefPostParse(dev->data.shmem) < 0) goto cleanup; + if (dev->type == VIR_DOMAIN_DEVICE_CHR && + qemuDomainChrDefPostParse(dev->data.chr, def) < 0) { + goto cleanup; + } + ret = 0; cleanup: -- 2.13.6

On Wed, Nov 15, 2017 at 12:50:06PM +0100, Andrea Bolognani wrote:
Having a separate function for char device handling is better than adding even more code to qemuDomainDeviceDefPostParse().
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_domain.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cc7596bad..2d5eee01e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4036,6 +4036,19 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, return 0; }
+static int +qemuDomainChrDefPostParse(virDomainChrDefPtr chr, + const virDomainDef *def) +{ + /* set the default console type for S390 arches */ + if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE && + ARCH_IS_S390(def->os.arch)) { + chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; + } + + return 0; +}
static int qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, @@ -4096,13 +4109,6 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, } }
- /* set the default console type for S390 arches */ - if (dev->type == VIR_DOMAIN_DEVICE_CHR && - dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && - dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE && - ARCH_IS_S390(def->os.arch)) - dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; -
You've missed the following condition that clears auto generated unix socket path, it is also for char devices.
/* clear auto generated unix socket path for inactive definitions */ if ((parseFlags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && dev->type == VIR_DOMAIN_DEVICE_CHR) { @@ -4154,6 +4160,11 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, qemuDomainShmemDefPostParse(dev->data.shmem) < 0) goto cleanup;
+ if (dev->type == VIR_DOMAIN_DEVICE_CHR && + qemuDomainChrDefPostParse(dev->data.chr, def) < 0) { + goto cleanup; + } +
Is there any specific reason why did you move it to a different place? Pavel

On Thu, 2017-11-16 at 11:30 +0100, Pavel Hrdina wrote:
@@ -4096,13 +4109,6 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, } }
- /* set the default console type for S390 arches */ - if (dev->type == VIR_DOMAIN_DEVICE_CHR && - dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && - dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE && - ARCH_IS_S390(def->os.arch)) - dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; -
You've missed the following condition that clears auto generated unix socket path, it is also for char devices.
Right. I was focusing on the target type, but the stuff you mention should definitely go in qemuDomainChrDefPostParse() too.
@@ -4154,6 +4160,11 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, qemuDomainShmemDefPostParse(dev->data.shmem) < 0) goto cleanup;
+ if (dev->type == VIR_DOMAIN_DEVICE_CHR && + qemuDomainChrDefPostParse(dev->data.chr, def) < 0) { + goto cleanup; + } +
Is there any specific reason why did you move it to a different place?
All the ad-hoc code is before the sub-functions, and it looks nicer to have the sub-functions all together. There should be no functional impact with moving it. -- Andrea Bolognani / Red Hat / Virtualization

The devicePostParse() callback is invoked for all devices so that drivers have a chance to set their own specific values; however, virDomainDefAddImplicitDevices() runs *after* the devicePostParse() callbacks have been invoked and can add new devices, in which case the driver wouldn't have a chance to customize them. Work around the issue by invoking the devicePostParse() callback after virDomainDefAddImplicitDevices(), only for the first serial devices, which might have been added by it. The same was already happening for the first video device for the very same reason. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9da8dd646..bd3a23c0a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4937,6 +4937,18 @@ virDomainDefPostParseCommon(virDomainDefPtr def, return -1; } + if (def->nserials != 0) { + virDomainDeviceDef device = { + .type = VIR_DOMAIN_DEVICE_CHR, + .data.chr = def->serials[0], + }; + + /* serials[0] might have been added in AddImplicitDevices, after we've + * done the per-device post-parse */ + if (virDomainDefPostParseDeviceIterator(def, &device, NULL, data) < 0) + return -1; + } + /* clean up possibly duplicated metadata entries */ virXMLNodeSanitizeNamespaces(def->metadata); -- 2.13.6

On Wed, Nov 15, 2017 at 12:50:07PM +0100, Andrea Bolognani wrote:
The devicePostParse() callback is invoked for all devices so that drivers have a chance to set their own specific values; however, virDomainDefAddImplicitDevices() runs *after* the devicePostParse() callbacks have been invoked and can add new devices, in which case the driver wouldn't have a chance to customize them.
Work around the issue by invoking the devicePostParse() callback after virDomainDefAddImplicitDevices(), only for the first serial devices, which might have been added by it. The same was already happening for the first video device for the very same reason.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
So far this was not required but following patches will change how the "implicit" serial device is added. Might be nice to mention it in the commit message. Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

This is the first step in getting rid of the assumption that isa-serial is the default target type for serial devices. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 8 +++++--- src/conf/domain_conf.h | 3 ++- src/qemu/qemu_command.c | 13 +++++++++++++ src/qemu/qemu_domain.c | 21 +++++++++++++++++++++ src/qemu/qemu_domain_address.c | 1 + 5 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bd3a23c0a..23ae68b9a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -446,6 +446,7 @@ VIR_ENUM_IMPL(virDomainChrDeviceState, VIR_DOMAIN_CHR_DEVICE_STATE_LAST, VIR_ENUM_IMPL(virDomainChrSerialTarget, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST, + "none", "isa-serial", "usb-serial", "pci-serial") @@ -4014,7 +4015,7 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def) /* modify it to be a serial port */ def->serials[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; - def->serials[0]->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; + def->serials[0]->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE; def->serials[0]->target.port = 0; } else { /* if the console source doesn't match */ @@ -4038,7 +4039,8 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def) def->serials[0]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL) { switch ((virDomainChrSerialTargetType) def->serials[0]->targetType) { - case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: { + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: { /* Create a stub console to match the serial port. * console[0] either does not exist @@ -11479,7 +11481,7 @@ virDomainChrDefaultTargetType(int devtype) return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE; case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: - return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; + return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE; case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL: case VIR_DOMAIN_CHR_DEVICE_TYPE_LAST: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 41443a02c..645845dfc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1081,7 +1081,8 @@ typedef enum { } virDomainChrDeviceType; typedef enum { - VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA = 0, + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE = 0, + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 010c2992f..60bdcfb2a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9143,6 +9143,14 @@ qemuChrIsPlatformDevice(const virDomainDef *def, return true; } + /* If we got all the way here and we're still stuck with the default + * target type for a serial device, it means we have no clue what kind of + * device we're talking about and we must treat it as a platform device */ + if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE) { + return true; + } + return false; } @@ -10246,7 +10254,12 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, } break; + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: + /* Except from _LAST, which is just a guard value and will never + * be used, all of the above are platform devices, which means + * qemuBuildSerialCommandLine() will have taken the appropriate + * branch and we will not have ended up here */ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid target type for serial device")); goto error; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2d5eee01e..15ab51dbd 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4047,6 +4047,27 @@ qemuDomainChrDefPostParse(virDomainChrDefPtr chr, chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; } + /* Historically, isa-serial and the default matched, so in order to + * maintain backwards compatibility we map them here. The actual default + * will be picked below based on the architecture and machine type */ + if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA) { + chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE; + } + + /* Set the default serial type */ + if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE) { + if (ARCH_IS_X86(def->os.arch)) { + chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; + } else if (qemuDomainIsPSeries(def)) { + /* Setting TYPE_ISA here is just a temporary hack to reduce test + * suite churn. Later on we will have a proper serial type for + * pSeries and this line will be updated accordingly */ + chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; + } + } + return 0; } diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 7f4ac0f45..989c0e6c9 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -782,6 +782,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: return 0; } -- 2.13.6

On Wed, Nov 15, 2017 at 12:50:08PM +0100, Andrea Bolognani wrote:
This is the first step in getting rid of the assumption that isa-serial is the default target type for serial devices.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 8 +++++--- src/conf/domain_conf.h | 3 ++- src/qemu/qemu_command.c | 13 +++++++++++++ src/qemu/qemu_domain.c | 21 +++++++++++++++++++++ src/qemu/qemu_domain_address.c | 1 + 5 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bd3a23c0a..23ae68b9a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -446,6 +446,7 @@ VIR_ENUM_IMPL(virDomainChrDeviceState, VIR_DOMAIN_CHR_DEVICE_STATE_LAST,
VIR_ENUM_IMPL(virDomainChrSerialTarget, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST, + "none", "isa-serial", "usb-serial", "pci-serial") @@ -4014,7 +4015,7 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def)
/* modify it to be a serial port */ def->serials[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; - def->serials[0]->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; + def->serials[0]->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE; def->serials[0]->target.port = 0; } else { /* if the console source doesn't match */ @@ -4038,7 +4039,8 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def) def->serials[0]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL) {
switch ((virDomainChrSerialTargetType) def->serials[0]->targetType) { - case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: { + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: {
/* Create a stub console to match the serial port. * console[0] either does not exist @@ -11479,7 +11481,7 @@ virDomainChrDefaultTargetType(int devtype) return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE;
case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: - return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; + return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE;
case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL: case VIR_DOMAIN_CHR_DEVICE_TYPE_LAST: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 41443a02c..645845dfc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1081,7 +1081,8 @@ typedef enum { } virDomainChrDeviceType;
typedef enum { - VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA = 0, + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE = 0, + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 010c2992f..60bdcfb2a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9143,6 +9143,14 @@ qemuChrIsPlatformDevice(const virDomainDef *def, return true; }
+ /* If we got all the way here and we're still stuck with the default + * target type for a serial device, it means we have no clue what kind of + * device we're talking about and we must treat it as a platform device */
Missing full stop/period at the end of the sentence.
+ if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE) { + return true; + } + return false; }
@@ -10246,7 +10254,12 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, } break;
+ case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: + /* Except from _LAST, which is just a guard value and will never + * be used, all of the above are platform devices, which means + * qemuBuildSerialCommandLine() will have taken the appropriate + * branch and we will not have ended up here */
Missing full stop/period at the end of the sentence.
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid target type for serial device")); goto error; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2d5eee01e..15ab51dbd 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4047,6 +4047,27 @@ qemuDomainChrDefPostParse(virDomainChrDefPtr chr, chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; }
+ /* Historically, isa-serial and the default matched, so in order to + * maintain backwards compatibility we map them here. The actual default + * will be picked below based on the architecture and machine type */
Missing full stop/period at the end of the sentence.
+ if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA) { + chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE; + } + + /* Set the default serial type */ + if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE) { + if (ARCH_IS_X86(def->os.arch)) { + chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; + } else if (qemuDomainIsPSeries(def)) { + /* Setting TYPE_ISA here is just a temporary hack to reduce test + * suite churn. Later on we will have a proper serial type for + * pSeries and this line will be updated accordingly */
Missing full stop/period at the end of the sentence.
+ chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; + } + } + return 0; }
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 7f4ac0f45..989c0e6c9 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -782,6 +782,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: return 0; }
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

On Thu, 2017-11-16 at 12:30 +0100, Pavel Hrdina wrote:
@@ -9143,6 +9143,14 @@ qemuChrIsPlatformDevice(const virDomainDef *def, return true; }
+ /* If we got all the way here and we're still stuck with the default + * target type for a serial device, it means we have no clue what kind of + * device we're talking about and we must treat it as a platform device */
Missing full stop/period at the end of the sentence.
Is that a thing now? It seems like *not* having the full stop is way more common: $ git grep -E '\.($| \*/)' src/ | grep -iEv 'copyr|licen' | wc -l 8166 $ git grep -E '[^\.] \*/' src/ | wc -l 19772 with the former number being bigger than reality because of API documentation and such. -- Andrea Bolognani / Red Hat / Virtualization

On Thu, Nov 16, 2017 at 02:00:24PM +0100, Andrea Bolognani wrote:
On Thu, 2017-11-16 at 12:30 +0100, Pavel Hrdina wrote:
@@ -9143,6 +9143,14 @@ qemuChrIsPlatformDevice(const virDomainDef *def, return true; }
+ /* If we got all the way here and we're still stuck with the default + * target type for a serial device, it means we have no clue what kind of + * device we're talking about and we must treat it as a platform device */
Missing full stop/period at the end of the sentence.
Is that a thing now? It seems like *not* having the full stop is way more common:
$ git grep -E '\.($| \*/)' src/ | grep -iEv 'copyr|licen' | wc -l 8166 $ git grep -E '[^\.] \*/' src/ | wc -l 19772
with the former number being bigger than reality because of API documentation and such.
Since when something that is more common does mean it's right? :) To make the argument even more absurd, why did you use the period at the end of other sentences or even in commit messages? I hoped that you've just missed it and wanted to point it out, not to start this silly and wasteful conversation. Pavel

On Thu, 2017-11-16 at 14:08 +0100, Pavel Hrdina wrote:
On Thu, Nov 16, 2017 at 02:00:24PM +0100, Andrea Bolognani wrote:
On Thu, 2017-11-16 at 12:30 +0100, Pavel Hrdina wrote:
@@ -9143,6 +9143,14 @@ qemuChrIsPlatformDevice(const virDomainDef *def, return true; }
+ /* If we got all the way here and we're still stuck with the default + * target type for a serial device, it means we have no clue what kind of + * device we're talking about and we must treat it as a platform device */
Missing full stop/period at the end of the sentence.
Is that a thing now? It seems like *not* having the full stop is way more common:
$ git grep -E '\.($| \*/)' src/ | grep -iEv 'copyr|licen' | wc -l 8166 $ git grep -E '[^\.] \*/' src/ | wc -l 19772
with the former number being bigger than reality because of API documentation and such.
Since when something that is more common does mean it's right? :) To make the argument even more absurd, why did you use the period at the end of other sentences or even in commit messages?
Different contexts often call for different conventions. I would never willfully not capitalize the first word in a sentence or skip the full stop at the end when writing an e-mail, but I regularly do both when using IM. And I know for a fact that so do you ;)
I hoped that you've just missed it and wanted to point it out, not to start this silly and wasteful conversation.
Agreed. I'll add the missing full stops and pick up your R-b. -- Andrea Bolognani / Red Hat / Virtualization

This attribute was used to decide whether to format the type attribute of the <target> element, but the logic didn't take into account all possible cases and as such could lead to unexpected results. Moreover, it's one more thing to keep track of, and can easily fall out of sync with other attributes. Now that we have VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE, we can use that value to signal that no specific target type has been configured for the serial device and as such the attribute should not be formatted at all. All other values are now formatted. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 11 ++++------- src/conf/domain_conf.h | 1 - src/vz/vz_sdk.c | 3 +-- tests/qemuargv2xmldata/qemuargv2xml-console-compat.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-dev.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-file.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-many.xml | 4 ++-- tests/qemuargv2xmldata/qemuargv2xml-serial-pty.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-tcp-telnet.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-tcp.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-udp.xml | 4 ++-- tests/qemuargv2xmldata/qemuargv2xml-serial-unix.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-vc.xml | 2 +- .../qemuhotplug-console-compat-2-live+console-virtio.xml | 4 ++-- .../qemuhotplug-console-compat-2-live.xml | 4 ++-- .../qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml | 4 ++-- .../qemuxml2xmlout-bios-nvram-os-interleave.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-chardev-label.xml | 4 ++-- .../qemuxml2xmloutdata/qemuxml2xmlout-console-compat-auto.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat2.xml | 2 +- .../qemuxml2xmloutdata/qemuxml2xmlout-console-virtio-many.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-interface-driver.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-interface-server.xml | 4 ++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-net-bandwidth.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-net-bandwidth2.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-net-coalesce.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-net-mtu.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml | 2 +- .../qemuxml2xmlout-pseries-cpu-compat-power9.xml | 2 +- .../qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-exact.xml | 2 +- .../qemuxml2xmlout-pseries-panic-missing.xml | 2 +- .../qemuxml2xmlout-pseries-panic-no-address.xml | 2 +- .../qemuxml2xmlout-q35-virt-manager-basic.xml | 2 +- .../qemuxml2xmlout-serial-spiceport-nospice.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-spiceport.xml | 2 +- .../qemuxml2xmlout-serial-target-port-auto.xml | 6 +++--- .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 4 ++-- .../qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-vhost_queues.xml | 2 +- 43 files changed, 56 insertions(+), 61 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 23ae68b9a..0bcfd5537 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11493,8 +11493,7 @@ virDomainChrDefaultTargetType(int devtype) } static int -virDomainChrTargetTypeFromString(virDomainChrDefPtr def, - int devtype, +virDomainChrTargetTypeFromString(int devtype, const char *targetType) { int ret = -1; @@ -11522,8 +11521,6 @@ virDomainChrTargetTypeFromString(virDomainChrDefPtr def, break; } - def->targetTypeAttr = true; - return ret; } @@ -11540,7 +11537,7 @@ virDomainChrDefParseTargetXML(virDomainChrDefPtr def, char *stateStr = NULL; if ((def->targetType = - virDomainChrTargetTypeFromString(def, def->deviceType, + virDomainChrTargetTypeFromString(def->deviceType, targetType)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown target type '%s' specified for character device"), @@ -16460,7 +16457,7 @@ virDomainChrEquals(virDomainChrDefPtr src, break; case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: - if (src->targetTypeAttr != tgt->targetTypeAttr) + if (src->targetType != tgt->targetType) return false; ATTRIBUTE_FALLTHROUGH; @@ -24020,7 +24017,7 @@ virDomainChrDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: - if (def->targetTypeAttr) { + if (def->targetType) { virBufferAsprintf(buf, "<target type='%s' port='%d'/>\n", virDomainChrTargetTypeToString(def->deviceType, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 645845dfc..9856fbc10 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1207,7 +1207,6 @@ struct _virDomainChrSourceDef { struct _virDomainChrDef { int deviceType; /* enum virDomainChrDeviceType */ - bool targetTypeAttr; int targetType; /* enum virDomainChrConsoleTargetType || enum virDomainChrChannelTargetType || enum virDomainChrSerialTargetType according to deviceType */ diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 819b02b1e..c9f2a13e4 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1191,7 +1191,6 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort, virDomainChrDefPtr chr) int ret = -1; chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; - chr->targetTypeAttr = false; pret = PrlVmDev_GetIndex(serialPort, &serialPortIndex); prlsdkCheckRetGoto(pret, cleanup); chr->target.port = serialPortIndex; @@ -2864,7 +2863,7 @@ static int prlsdkCheckSerialUnsupportedParams(virDomainChrDefPtr chr) return -1; } - if (chr->targetTypeAttr) { + if (chr->targetType) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Specified character device target type is not " "supported by vz driver.")); diff --git a/tests/qemuargv2xmldata/qemuargv2xml-console-compat.xml b/tests/qemuargv2xmldata/qemuargv2xml-console-compat.xml index f51284442..7c106f145 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-console-compat.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-console-compat.xml @@ -28,7 +28,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-serial-dev.xml b/tests/qemuargv2xmldata/qemuargv2xml-serial-dev.xml index aac814d59..e76d0211d 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-serial-dev.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-serial-dev.xml @@ -29,7 +29,7 @@ </controller> <serial type='dev'> <source path='/dev/ttyS2'/> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='dev'> <source path='/dev/ttyS2'/> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-serial-file.xml b/tests/qemuargv2xmldata/qemuargv2xml-serial-file.xml index 64819a48a..ed67ada0d 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-serial-file.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-serial-file.xml @@ -29,7 +29,7 @@ </controller> <serial type='file'> <source path='/tmp/serial.log'/> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='file'> <source path='/tmp/serial.log'/> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-serial-many.xml b/tests/qemuargv2xmldata/qemuargv2xml-serial-many.xml index e00afe317..420771dc9 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-serial-many.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-serial-many.xml @@ -28,11 +28,11 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <serial type='file'> <source path='/tmp/serial.log'/> - <target port='1'/> + <target type='isa-serial' port='1'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-serial-pty.xml b/tests/qemuargv2xmldata/qemuargv2xml-serial-pty.xml index f51284442..7c106f145 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-serial-pty.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-serial-pty.xml @@ -28,7 +28,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-serial-tcp-telnet.xml b/tests/qemuargv2xmldata/qemuargv2xml-serial-tcp-telnet.xml index c35a4ca73..3fe61ffa0 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-serial-tcp-telnet.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-serial-tcp-telnet.xml @@ -30,7 +30,7 @@ <serial type='tcp'> <source mode='bind' host='127.0.0.1' service='9999'/> <protocol type='telnet'/> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='tcp'> <source mode='bind' host='127.0.0.1' service='9999'/> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-serial-tcp.xml b/tests/qemuargv2xmldata/qemuargv2xml-serial-tcp.xml index 0d218f548..3fc9fd39b 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-serial-tcp.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-serial-tcp.xml @@ -30,7 +30,7 @@ <serial type='tcp'> <source mode='connect' host='127.0.0.1' service='9999'/> <protocol type='raw'/> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='tcp'> <source mode='connect' host='127.0.0.1' service='9999'/> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-serial-udp.xml b/tests/qemuargv2xmldata/qemuargv2xml-serial-udp.xml index f7069d541..5b4af3fe9 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-serial-udp.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-serial-udp.xml @@ -30,11 +30,11 @@ <serial type='udp'> <source mode='bind' host='127.0.0.1' service='9999'/> <source mode='connect' host='127.0.0.1' service='9998'/> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <serial type='udp'> <source mode='connect' service='9999'/> - <target port='1'/> + <target type='isa-serial' port='1'/> </serial> <console type='udp'> <source mode='bind' host='127.0.0.1' service='9999'/> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-serial-unix.xml b/tests/qemuargv2xmldata/qemuargv2xml-serial-unix.xml index 14fc8fc7c..6bb291ff7 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-serial-unix.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-serial-unix.xml @@ -29,7 +29,7 @@ </controller> <serial type='unix'> <source mode='connect' path='/tmp/serial.sock'/> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='unix'> <source mode='connect' path='/tmp/serial.sock'/> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-serial-vc.xml b/tests/qemuargv2xmldata/qemuargv2xml-serial-vc.xml index 95aa1c7b9..41954fc85 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-serial-vc.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-serial-vc.xml @@ -28,7 +28,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <serial type='vc'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='vc'> <target type='serial' port='0'/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live+console-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live+console-virtio.xml index 4e1dd49c2..427f431cc 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live+console-virtio.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live+console-virtio.xml @@ -72,13 +72,13 @@ <alias name='serial0'/> </serial> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> <alias name='serial1'/> </serial> <serial type='tcp'> <source mode='bind' host='0.0.0.0' service='2445'/> <protocol type='raw'/> - <target port='1'/> + <target type='isa-serial' port='1'/> <alias name='serial2'/> </serial> <console type='pty'> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live.xml b/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live.xml index c56d13ef4..144f6eff7 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live.xml @@ -72,13 +72,13 @@ <alias name='serial0'/> </serial> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> <alias name='serial1'/> </serial> <serial type='tcp'> <source mode='bind' host='0.0.0.0' service='2445'/> <protocol type='raw'/> - <target port='1'/> + <target type='isa-serial' port='1'/> <alias name='serial2'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml index a95e29ad8..e6c4adb6f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml @@ -29,12 +29,12 @@ <serial type='udp'> <source mode='bind' host='127.0.0.1' service='1111'/> <source mode='connect' host='127.0.0.1' service='2222'/> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <serial type='tcp'> <source mode='connect' host='127.0.0.1' service='5555' tls='no'/> <protocol type='raw'/> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='udp'> <source mode='bind' host='127.0.0.1' service='1111'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml b/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml index d1cb8fea6..df0c71eb5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml @@ -108,10 +108,10 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> </interface> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <serial type='pty'> - <target port='1'/> + <target type='isa-serial' port='1'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-bios-nvram-os-interleave.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-bios-nvram-os-interleave.xml index 033e86d3a..5ee73b527 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-bios-nvram-os-interleave.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-bios-nvram-os-interleave.xml @@ -33,7 +33,7 @@ </controller> <controller type='pci' index='0' model='pci-root'/> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-chardev-label.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-chardev-label.xml index 840bf69f6..ad77f62d9 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-chardev-label.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-chardev-label.xml @@ -25,13 +25,13 @@ <source path='/tmp/serial.file'> <seclabel model='dac' relabel='no'/> </source> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <serial type='unix'> <source mode='connect' path='/tmp/serial.sock'> <seclabel model='dac' relabel='no'/> </source> - <target port='1'/> + <target type='isa-serial' port='1'/> </serial> <console type='file'> <source path='/tmp/serial.file'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat-auto.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat-auto.xml index e76f857ae..cd9d75c4b 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat-auto.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat-auto.xml @@ -27,7 +27,7 @@ </controller> <controller type='pci' index='0' model='pci-root'/> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat.xml index 8dc361dfc..0c0bd7b34 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat.xml @@ -28,7 +28,7 @@ </controller> <controller type='pci' index='0' model='pci-root'/> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat2.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat2.xml index 858b2c675..305c53eab 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat2.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat2.xml @@ -31,7 +31,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </controller> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio-many.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio-many.xml index f9f9abd2d..b38b3ce98 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio-many.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio-many.xml @@ -30,7 +30,7 @@ </controller> <controller type='pci' index='0' model='pci-root'/> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-interface-driver.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-interface-driver.xml index 1c5501767..06192fbb4 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-interface-driver.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-interface-driver.xml @@ -47,7 +47,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-interface-server.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-interface-server.xml index 95b6e2df1..a6eaa3807 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-interface-server.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-interface-server.xml @@ -104,10 +104,10 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> </interface> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <serial type='pty'> - <target port='1'/> + <target type='isa-serial' port='1'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-bandwidth.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-bandwidth.xml index 7fe69bd6c..e6ad23424 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-bandwidth.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-bandwidth.xml @@ -55,7 +55,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </interface> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-bandwidth2.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-bandwidth2.xml index b631e5b51..66448ec3d 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-bandwidth2.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-bandwidth2.xml @@ -44,7 +44,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </interface> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-coalesce.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-coalesce.xml index fd5fdbece..b1240b8ed 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-coalesce.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-coalesce.xml @@ -57,7 +57,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </interface> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-mtu.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-mtu.xml index 4571b6a82..4f7ad323e 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-mtu.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-mtu.xml @@ -54,7 +54,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </interface> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml index 7fb49feb0..a563b6ddd 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml @@ -22,7 +22,7 @@ <target index='0'/> </controller> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat-power9.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat-power9.xml index f02005621..59587b3c3 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat-power9.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat-power9.xml @@ -25,7 +25,7 @@ <target index='0'/> </controller> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat.xml index 3cbce9fe6..a39e1fd01 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat.xml @@ -25,7 +25,7 @@ <target index='0'/> </controller> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-exact.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-exact.xml index d69b38768..666eede1a 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-exact.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-exact.xml @@ -26,7 +26,7 @@ <target index='0'/> </controller> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml index 7fb49feb0..a563b6ddd 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml @@ -22,7 +22,7 @@ <target index='0'/> </controller> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml index 7fb49feb0..a563b6ddd 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml @@ -22,7 +22,7 @@ <target index='0'/> </controller> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virt-manager-basic.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virt-manager-basic.xml index c4ccd98aa..27baaa3f1 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virt-manager-basic.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virt-manager-basic.xml @@ -79,7 +79,7 @@ <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> </interface> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-spiceport-nospice.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-spiceport-nospice.xml index 79c4ebc73..63462e6f9 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-spiceport-nospice.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-spiceport-nospice.xml @@ -28,7 +28,7 @@ <controller type='pci' index='0' model='pci-root'/> <serial type='spiceport'> <source channel='org.qemu.console.serial.0'/> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='spiceport'> <source channel='org.qemu.console.serial.0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-spiceport.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-spiceport.xml index 9527b2d15..c90bbeb7d 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-spiceport.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-spiceport.xml @@ -28,7 +28,7 @@ <controller type='pci' index='0' model='pci-root'/> <serial type='spiceport'> <source channel='org.qemu.console.serial.0'/> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='spiceport'> <source channel='org.qemu.console.serial.0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-target-port-auto.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-target-port-auto.xml index 71516a31a..a8790b509 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-target-port-auto.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-target-port-auto.xml @@ -27,13 +27,13 @@ </controller> <controller type='pci' index='0' model='pci-root'/> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <serial type='null'> - <target port='1'/> + <target type='isa-serial' port='1'/> </serial> <serial type='stdio'> - <target port='2'/> + <target type='isa-serial' port='2'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml index 18f51e538..a8af87b53 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml @@ -29,12 +29,12 @@ <serial type='udp'> <source mode='bind' host='127.0.0.1' service='1111'/> <source mode='connect' host='127.0.0.1' service='2222'/> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <serial type='tcp'> <source mode='connect' host='127.0.0.1' service='5555'/> <protocol type='raw'/> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='udp'> <source mode='bind' host='127.0.0.1' service='1111'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml index 3a95b6088..6d847de3a 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml @@ -40,7 +40,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </interface> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost.xml index 759b84439..30989658f 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost.xml @@ -47,7 +47,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-vhost_queues.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-vhost_queues.xml index 8c27470dd..72cbcc4cb 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-vhost_queues.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-vhost_queues.xml @@ -46,7 +46,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> -- 2.13.6

On Wed, Nov 15, 2017 at 12:50:09PM +0100, Andrea Bolognani wrote:
This attribute was used to decide whether to format the type attribute of the <target> element, but the logic didn't take into account all possible cases and as such could lead to unexpected results. Moreover, it's one more thing to keep track of, and can easily fall out of sync with other attributes.
Now that we have VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE, we can use that value to signal that no specific target type has been configured for the serial device and as such the attribute should not be formatted at all. All other values are now formatted.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 11 ++++------- src/conf/domain_conf.h | 1 - src/vz/vz_sdk.c | 3 +-- tests/qemuargv2xmldata/qemuargv2xml-console-compat.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-dev.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-file.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-many.xml | 4 ++-- tests/qemuargv2xmldata/qemuargv2xml-serial-pty.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-tcp-telnet.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-tcp.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-udp.xml | 4 ++-- tests/qemuargv2xmldata/qemuargv2xml-serial-unix.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-vc.xml | 2 +- .../qemuhotplug-console-compat-2-live+console-virtio.xml | 4 ++-- .../qemuhotplug-console-compat-2-live.xml | 4 ++-- .../qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml | 4 ++-- .../qemuxml2xmlout-bios-nvram-os-interleave.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-chardev-label.xml | 4 ++-- .../qemuxml2xmloutdata/qemuxml2xmlout-console-compat-auto.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat2.xml | 2 +- .../qemuxml2xmloutdata/qemuxml2xmlout-console-virtio-many.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-interface-driver.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-interface-server.xml | 4 ++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-net-bandwidth.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-net-bandwidth2.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-net-coalesce.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-net-mtu.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml | 2 +- .../qemuxml2xmlout-pseries-cpu-compat-power9.xml | 2 +- .../qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-exact.xml | 2 +- .../qemuxml2xmlout-pseries-panic-missing.xml | 2 +- .../qemuxml2xmlout-pseries-panic-no-address.xml | 2 +- .../qemuxml2xmlout-q35-virt-manager-basic.xml | 2 +- .../qemuxml2xmlout-serial-spiceport-nospice.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-spiceport.xml | 2 +- .../qemuxml2xmlout-serial-target-port-auto.xml | 6 +++--- .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 4 ++-- .../qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-vhost_queues.xml | 2 +- 43 files changed, 56 insertions(+), 61 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 23ae68b9a..0bcfd5537 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11493,8 +11493,7 @@ virDomainChrDefaultTargetType(int devtype) }
static int -virDomainChrTargetTypeFromString(virDomainChrDefPtr def, - int devtype, +virDomainChrTargetTypeFromString(int devtype, const char *targetType) { int ret = -1; @@ -11522,8 +11521,6 @@ virDomainChrTargetTypeFromString(virDomainChrDefPtr def, break; }
- def->targetTypeAttr = true; - return ret; }
@@ -11540,7 +11537,7 @@ virDomainChrDefParseTargetXML(virDomainChrDefPtr def, char *stateStr = NULL;
if ((def->targetType = - virDomainChrTargetTypeFromString(def, def->deviceType, + virDomainChrTargetTypeFromString(def->deviceType, targetType)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown target type '%s' specified for character device"), @@ -16460,7 +16457,7 @@ virDomainChrEquals(virDomainChrDefPtr src, break;
case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: - if (src->targetTypeAttr != tgt->targetTypeAttr) + if (src->targetType != tgt->targetType) return false;
ATTRIBUTE_FALLTHROUGH; @@ -24020,7 +24017,7 @@ virDomainChrDefFormat(virBufferPtr buf, break;
case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: - if (def->targetTypeAttr) { + if (def->targetType) {
I would prefer explicit check if it's equal to VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE. It's not a bool variable.
virBufferAsprintf(buf, "<target type='%s' port='%d'/>\n", virDomainChrTargetTypeToString(def->deviceType, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 645845dfc..9856fbc10 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1207,7 +1207,6 @@ struct _virDomainChrSourceDef { struct _virDomainChrDef { int deviceType; /* enum virDomainChrDeviceType */
- bool targetTypeAttr; int targetType; /* enum virDomainChrConsoleTargetType || enum virDomainChrChannelTargetType || enum virDomainChrSerialTargetType according to deviceType */ diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 819b02b1e..c9f2a13e4 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1191,7 +1191,6 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort, virDomainChrDefPtr chr) int ret = -1;
chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; - chr->targetTypeAttr = false; pret = PrlVmDev_GetIndex(serialPort, &serialPortIndex); prlsdkCheckRetGoto(pret, cleanup); chr->target.port = serialPortIndex; @@ -2864,7 +2863,7 @@ static int prlsdkCheckSerialUnsupportedParams(virDomainChrDefPtr chr) return -1; }
- if (chr->targetTypeAttr) { + if (chr->targetType) {
Same here.
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Specified character device target type is not " "supported by vz driver."));
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

We can finally introduce a specific target type for the spapr-vty device used by pSeries guests, which means isa-serial will no longer show up to confuse users. We make sure migration works in both directions by interpreting the isa-serial target type, or the lack of target type, appropriately when parsing the guest XML, and skipping the newly-introduced type when formatting if for migration. We also verify that spapr-vty is not used for non-pSeries guests and add a bunch of test cases. This commit is best viewed with 'git diff -w'. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511421 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/formatdomain.html.in | 11 +- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 5 +- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 115 +++++++++++---------- src/qemu/qemu_domain.c | 38 ++++++- src/qemu/qemu_domain_address.c | 1 + .../qemuxml2argv-pseries-basic.args | 2 +- .../qemuxml2argv-pseries-console-native.args | 1 + .../qemuxml2argv-pseries-console-native.xml | 17 +++ ...gs => qemuxml2argv-pseries-console-virtio.args} | 10 +- .../qemuxml2argv-pseries-console-virtio.xml | 19 ++++ .../qemuxml2argv-pseries-cpu-compat-power9.args | 2 +- .../qemuxml2argv-pseries-cpu-compat.args | 2 +- .../qemuxml2argv-pseries-cpu-exact.args | 2 +- .../qemuxml2argv-pseries-cpu-le.args | 2 +- .../qemuxml2argv-pseries-panic-missing.args | 2 +- .../qemuxml2argv-pseries-panic-no-address.args | 2 +- ...qemuxml2argv-pseries-serial+console-native.args | 1 + .../qemuxml2argv-pseries-serial+console-native.xml | 18 ++++ .../qemuxml2argv-pseries-serial-compat.args | 1 + .../qemuxml2argv-pseries-serial-compat.xml | 19 ++++ ...qemuxml2argv-pseries-serial-invalid-machine.xml | 19 ++++ ...rgs => qemuxml2argv-pseries-serial-native.args} | 7 +- .../qemuxml2argv-pseries-serial-native.xml | 16 +++ .../qemuxml2argv-pseries-usb-default.args | 2 +- .../qemuxml2argv-pseries-usb-kbd.args | 2 +- .../qemuxml2argv-pseries-usb-multi.args | 2 +- .../qemuxml2argv-pseries-vio-user-assigned.args | 4 +- .../qemuxml2argvdata/qemuxml2argv-pseries-vio.args | 4 +- tests/qemuxml2argvtest.c | 16 +++ .../qemuxml2xmlout-panic-pseries.xml | 2 +- .../qemuxml2xmlout-pseries-console-native.xml | 1 + ...l => qemuxml2xmlout-pseries-console-virtio.xml} | 16 ++- .../qemuxml2xmlout-pseries-cpu-compat-power9.xml | 2 +- .../qemuxml2xmlout-pseries-cpu-compat.xml | 2 +- .../qemuxml2xmlout-pseries-cpu-exact.xml | 2 +- .../qemuxml2xmlout-pseries-panic-missing.xml | 2 +- .../qemuxml2xmlout-pseries-panic-no-address.xml | 2 +- ...emuxml2xmlout-pseries-serial+console-native.xml | 1 + .../qemuxml2xmlout-pseries-serial-compat.xml | 1 + ...ml => qemuxml2xmlout-pseries-serial-native.xml} | 8 +- tests/qemuxml2xmltest.c | 15 +++ 43 files changed, 288 insertions(+), 110 deletions(-) create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-pseries-console-native.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-console-native.xml copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-basic.args => qemuxml2argv-pseries-console-virtio.args} (59%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-console-virtio.xml create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial+console-native.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial+console-native.xml create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-compat.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-compat.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-invalid-machine.xml copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-basic.args => qemuxml2argv-pseries-serial-native.args} (70%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-native.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-console-native.xml copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-panic-pseries.xml => qemuxml2xmlout-pseries-console-virtio.xml} (74%) create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial+console-native.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-compat.xml copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-panic-pseries.xml => qemuxml2xmlout-pseries-serial-native.xml} (81%) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8dbea6af7..7ff097d65 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6526,10 +6526,13 @@ qemu-kvm -net nic,model=? /dev/null specifies the port number. Ports are numbered starting from 0. There are usually 0, 1 or 2 serial ports. There is also an optional <code>type</code> attribute <span class="since">since 1.0.2</span> - which has three choices for its value, one is <code>isa-serial</code>, - then <code>usb-serial</code> and last one is <code>pci-serial</code>. - If <code>type</code> is missing, <code>isa-serial</code> will be used by - default. For <code>usb-serial</code> an optional sub-element + which can be used to pick between <code>isa-serial</code>, + <code>usb-serial</code>, <code>pci-serial</code> and, + <span class="since">since 3.10.0</span>, <code>spapr-vty</code>. + Some values are not compatible with all architecture and machine types; + if the value is missing altogether, libvirt will try to pick an + appropriate default. + For <code>usb-serial</code> an optional sub-element <code><address/></code> with <code>type='usb'</code> can tie the device to a particular controller, <a href="#elementsAddress">documented above</a>. Similarly, <code>pci-serial</code> can be used to attach the device to diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 82fdfd5f7..8f6d035de 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3585,6 +3585,7 @@ <value>isa-serial</value> <value>usb-serial</value> <value>pci-serial</value> + <value>spapr-vty</value> </choice> </attribute> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0bcfd5537..f7faa1a35 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -449,7 +449,9 @@ VIR_ENUM_IMPL(virDomainChrSerialTarget, "none", "isa-serial", "usb-serial", - "pci-serial") + "pci-serial", + "spapr-vty", +); VIR_ENUM_IMPL(virDomainChrChannelTarget, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST, @@ -4040,6 +4042,7 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def) switch ((virDomainChrSerialTargetType) def->serials[0]->targetType) { case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: { /* Create a stub console to match the serial port. diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9856fbc10..6dd97a0a3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1085,6 +1085,7 @@ typedef enum { VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI, + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST } virDomainChrSerialTargetType; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 60bdcfb2a..7b6961bf2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10201,75 +10201,76 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, { virBuffer cmd = VIR_BUFFER_INITIALIZER; - if (qemuDomainIsPSeries(def)) { - if (serial->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && - serial->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_VTY)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spapr-vty not supported in this QEMU binary")); - goto error; - } + switch ((virDomainChrSerialTargetType) serial->targetType) { + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_SERIAL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("usb-serial is not supported in this QEMU binary")); + goto error; + } - virBufferAsprintf(&cmd, "spapr-vty,chardev=char%s", - serial->info.alias); + if (serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("usb-serial requires address of usb type")); + goto error; } - } else { - switch ((virDomainChrSerialTargetType) serial->targetType) { - case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_SERIAL)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("usb-serial is not supported in this QEMU binary")); - goto error; - } + break; - if (serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && - serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("usb-serial requires address of usb type")); - goto error; - } - break; + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: + if (serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("isa-serial requires address of isa type")); + goto error; + } + break; - case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: - if (serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && - serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("isa-serial requires address of isa type")); - goto error; - } - break; + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_SERIAL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("pci-serial is not supported with this QEMU binary")); + goto error; + } - case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_SERIAL)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("pci-serial is not supported with this QEMU binary")); - goto error; - } + if (serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("pci-serial requires address of pci type")); + goto error; + } + break; - if (serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && - serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("pci-serial requires address of pci type")); - goto error; - } - break; + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_VTY)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spapr-vty not supported in this QEMU binary")); + goto error; + } - case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: - case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: - /* Except from _LAST, which is just a guard value and will never - * be used, all of the above are platform devices, which means - * qemuBuildSerialCommandLine() will have taken the appropriate - * branch and we will not have ended up here */ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Invalid target type for serial device")); + if (serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spapr-vty requires address of spapr-vio type")); goto error; } + break; - virBufferAsprintf(&cmd, "%s,chardev=char%s,id=%s", - virDomainChrSerialTargetTypeToString(serial->targetType), - serial->info.alias, serial->info.alias); + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: + /* Except from _LAST, which is just a guard value and will never + * be used, all of the above are platform devices, which means + * qemuBuildSerialCommandLine() will have taken the appropriate + * branch and we will not have ended up here */ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Invalid target type for serial device")); + goto error; } + virBufferAsprintf(&cmd, "%s,chardev=char%s,id=%s", + virDomainChrSerialTargetTypeToString(serial->targetType), + serial->info.alias, serial->info.alias); + if (qemuBuildDeviceAddressStr(&cmd, def, &serial->info, qemuCaps) < 0) goto error; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 15ab51dbd..9cafbdd27 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3504,6 +3504,15 @@ qemuDomainChrDefValidate(const virDomainChrDef *dev, return -1; } + if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + dev->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR && + !qemuDomainIsPSeries(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spapr-vty serial devices are only supported on " + "pSeries guests")); + return -1; + } + return 0; } @@ -4061,10 +4070,7 @@ qemuDomainChrDefPostParse(virDomainChrDefPtr chr, if (ARCH_IS_X86(def->os.arch)) { chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; } else if (qemuDomainIsPSeries(def)) { - /* Setting TYPE_ISA here is just a temporary hack to reduce test - * suite churn. Later on we will have a proper serial type for - * pSeries and this line will be updated accordingly */ - chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; + chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR; } } @@ -4968,6 +4974,30 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver, goto cleanup; } + for (i = 0; i < def->nserials; i++) { + virDomainChrDefPtr serial = def->serials[i]; + + /* Historically, the native console type for some machine types + * was not set at all, which means it defaulted to ISA even + * though that was not even remotely accurate. To ensure migration + * towards older libvirt versions works for such guests, we switch + * it back to the default here */ + if (flags & VIR_DOMAIN_XML_MIGRATABLE) { + switch ((virDomainChrSerialTargetType) serial->targetType) { + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR: + serial->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE; + break; + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: + /* Nothing to do */ + break; + } + } + } + /* Replace the CPU definition updated according to QEMU with the one * used for starting the domain. The updated def will be sent * separately for backward compatibility. diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 989c0e6c9..46e91f9fe 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -782,6 +782,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: return 0; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args index 97a7057ba..789d9f679 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args @@ -20,4 +20,4 @@ server,nowait \ -boot c \ -usb \ -chardev pty,id=charserial0 \ --device spapr-vty,chardev=charserial0,reg=0x30000000 +-device spapr-vty,chardev=charserial0,id=serial0,reg=0x30000000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-console-native.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-console-native.args new file mode 120000 index 000000000..d6c830ecd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-console-native.args @@ -0,0 +1 @@ +qemuxml2argv-pseries-serial-native.args \ No newline at end of file diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-console-native.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-console-native.xml new file mode 100644 index 000000000..9f37bf0de --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-console-native.xml @@ -0,0 +1,17 @@ +<domain type='qemu'> + <name>guest</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> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' model='none'/> + <!-- The <console> element being present should result in a matching + <serial> element being created --> + <console type='pty'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-console-virtio.args similarity index 59% copy from tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args copy to tests/qemuxml2argvdata/qemuxml2argv-pseries-console-virtio.args index 97a7057ba..343018fb3 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-console-virtio.args @@ -5,7 +5,7 @@ USER=test \ LOGNAME=test \ QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-ppc64 \ --name QEMUGuest1 \ +-name guest \ -S \ -M pseries \ -m 512 \ @@ -14,10 +14,10 @@ QEMU_AUDIO_DRV=none \ -nographic \ -nodefconfig \ -nodefaults \ --chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\ server,nowait \ -mon chardev=charmonitor,id=monitor,mode=readline \ -boot c \ --usb \ --chardev pty,id=charserial0 \ --device spapr-vty,chardev=charserial0,reg=0x30000000 +-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x1 \ +-chardev pty,id=charconsole0 \ +-device virtconsole,chardev=charconsole0,id=console0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-console-virtio.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-console-virtio.xml new file mode 100644 index 000000000..0190ab63a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-console-virtio.xml @@ -0,0 +1,19 @@ +<domain type='qemu'> + <name>guest</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> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' model='none'/> + <!-- The <console> element being present should *not* result in a + matching <serial> element being created --> + <console type='pty'> + <target type='virtio'/> + </console> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args index af93d63dc..9bb375aeb 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args @@ -21,4 +21,4 @@ server,nowait \ -boot c \ -usb \ -chardev pty,id=charserial0 \ --device spapr-vty,chardev=charserial0,reg=0x30000000 +-device spapr-vty,chardev=charserial0,id=serial0,reg=0x30000000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args index 7740e2f5a..5174aa760 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args @@ -21,4 +21,4 @@ server,nowait \ -boot c \ -usb \ -chardev pty,id=charserial0 \ --device spapr-vty,chardev=charserial0,reg=0x30000000 +-device spapr-vty,chardev=charserial0,id=serial0,reg=0x30000000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-exact.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-exact.args index d2c99a7fa..3790deca8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-exact.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-exact.args @@ -21,4 +21,4 @@ server,nowait \ -boot c \ -usb \ -chardev pty,id=charserial0 \ --device spapr-vty,chardev=charserial0,reg=0x30000000 +-device spapr-vty,chardev=charserial0,id=serial0,reg=0x30000000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args index 97a7057ba..789d9f679 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args @@ -20,4 +20,4 @@ server,nowait \ -boot c \ -usb \ -chardev pty,id=charserial0 \ --device spapr-vty,chardev=charserial0,reg=0x30000000 +-device spapr-vty,chardev=charserial0,id=serial0,reg=0x30000000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.args index 97a7057ba..789d9f679 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.args @@ -20,4 +20,4 @@ server,nowait \ -boot c \ -usb \ -chardev pty,id=charserial0 \ --device spapr-vty,chardev=charserial0,reg=0x30000000 +-device spapr-vty,chardev=charserial0,id=serial0,reg=0x30000000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.args index 97a7057ba..789d9f679 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.args @@ -20,4 +20,4 @@ server,nowait \ -boot c \ -usb \ -chardev pty,id=charserial0 \ --device spapr-vty,chardev=charserial0,reg=0x30000000 +-device spapr-vty,chardev=charserial0,id=serial0,reg=0x30000000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial+console-native.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial+console-native.args new file mode 120000 index 000000000..d6c830ecd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial+console-native.args @@ -0,0 +1 @@ +qemuxml2argv-pseries-serial-native.args \ No newline at end of file diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial+console-native.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial+console-native.xml new file mode 100644 index 000000000..2733baa98 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial+console-native.xml @@ -0,0 +1,18 @@ +<domain type='qemu'> + <name>guest</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> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' model='none'/> + <!-- When both the <serial> and <console> elements are present, they will + be matched and end up representing the same native serial console --> + <serial type='pty'/> + <console type='pty'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-compat.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-compat.args new file mode 120000 index 000000000..d6c830ecd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-compat.args @@ -0,0 +1 @@ +qemuxml2argv-pseries-serial-native.args \ No newline at end of file diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-compat.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-compat.xml new file mode 100644 index 000000000..568686dbc --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-compat.xml @@ -0,0 +1,19 @@ +<domain type='qemu'> + <name>guest</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> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' model='none'/> + <!-- isa-serial has to be accepted for backwards compatibility reasons, + but should get converted to the proper type (spapr-vty) --> + <serial type='pty'> + <target type='isa-serial'/> + </serial> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-invalid-machine.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-invalid-machine.xml new file mode 100644 index 000000000..fd44dca6f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-invalid-machine.xml @@ -0,0 +1,19 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' model='none'/> + <!-- The spapr-vty serial console can only be used for pSeries guests, + so this should be rejected --> + <serial type='pty'> + <target type='spapr-vty'/> + </serial> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-native.args similarity index 70% copy from tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args copy to tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-native.args index 97a7057ba..f72b8b625 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-native.args @@ -5,7 +5,7 @@ USER=test \ LOGNAME=test \ QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-ppc64 \ --name QEMUGuest1 \ +-name guest \ -S \ -M pseries \ -m 512 \ @@ -14,10 +14,9 @@ QEMU_AUDIO_DRV=none \ -nographic \ -nodefconfig \ -nodefaults \ --chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\ server,nowait \ -mon chardev=charmonitor,id=monitor,mode=readline \ -boot c \ --usb \ -chardev pty,id=charserial0 \ --device spapr-vty,chardev=charserial0,reg=0x30000000 +-device spapr-vty,chardev=charserial0,id=serial0,reg=0x30000000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-native.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-native.xml new file mode 100644 index 000000000..b5fabcdf7 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-native.xml @@ -0,0 +1,16 @@ +<domain type='qemu'> + <name>guest</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> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' model='none'/> + <!-- This will use the spapr-vty target type --> + <serial type='pty'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-default.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-default.args index a92b1e01b..37c059403 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-default.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-default.args @@ -20,4 +20,4 @@ server,nowait \ -boot c \ -device pci-ohci,id=usb,bus=pci.0,addr=0x1 \ -chardev pty,id=charserial0 \ --device spapr-vty,chardev=charserial0,reg=0x30000000 +-device spapr-vty,chardev=charserial0,id=serial0,reg=0x30000000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args index caaccdbb8..838b80453 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args @@ -20,5 +20,5 @@ server,nowait \ -boot c \ -device pci-ohci,id=usb,bus=pci.0,addr=0x1 \ -chardev pty,id=charserial0 \ --device spapr-vty,chardev=charserial0,reg=0x30000000 \ +-device spapr-vty,chardev=charserial0,id=serial0,reg=0x30000000 \ -device usb-kbd,id=input0,bus=usb.0,port=1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-multi.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-multi.args index b9bd905a5..56bc1d67e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-multi.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-multi.args @@ -21,4 +21,4 @@ server,nowait \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1 \ -device pci-ohci,id=usb1,bus=pci.0,addr=0x2 \ -chardev pty,id=charserial0 \ --device spapr-vty,chardev=charserial0,reg=0x30000000 +-device spapr-vty,chardev=charserial0,id=serial0,reg=0x30000000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.args index 63cf3c183..0fcfbe379 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.args @@ -25,6 +25,6 @@ server,nowait \ -device scsi-disk,bus=scsi1.0,channel=0,scsi-id=0,lun=0,\ drive=drive-scsi1-0-0-0,id=scsi1-0-0-0 \ -chardev pty,id=charserial0 \ --device spapr-vty,chardev=charserial0,reg=0x20000000 \ +-device spapr-vty,chardev=charserial0,id=serial0,reg=0x20000000 \ -chardev pty,id=charserial1 \ --device spapr-vty,chardev=charserial1,reg=0x30001000 +-device spapr-vty,chardev=charserial1,id=serial1,reg=0x30001000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.args index 0294067bc..8a9bdcc4c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.args @@ -25,6 +25,6 @@ server,nowait \ -device scsi-disk,bus=scsi1.0,channel=0,scsi-id=0,lun=0,\ drive=drive-scsi1-0-0-0,id=scsi1-0-0-0 \ -chardev pty,id=charserial0 \ --device spapr-vty,chardev=charserial0,reg=0x30000000 \ +-device spapr-vty,chardev=charserial0,id=serial0,reg=0x30000000 \ -chardev pty,id=charserial1 \ --device spapr-vty,chardev=charserial1,reg=0x30001000 +-device spapr-vty,chardev=charserial1,id=serial1,reg=0x30001000 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 632c59b7b..843171cfa 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1872,6 +1872,22 @@ mymain(void) QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT); + DO_TEST("pseries-serial-native", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_SPAPR_VTY); + DO_TEST("pseries-serial+console-native", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_SPAPR_VTY); + DO_TEST("pseries-serial-compat", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_SPAPR_VTY); + DO_TEST("pseries-console-native", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_SPAPR_VTY); + DO_TEST("pseries-console-virtio", + QEMU_CAPS_NODEFCONFIG); + DO_TEST_PARSE_ERROR("pseries-serial-invalid-machine", NONE); + DO_TEST("disk-ide-drive-split", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml index a563b6ddd..ec04b5a3c 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml @@ -22,7 +22,7 @@ <target index='0'/> </controller> <serial type='pty'> - <target type='isa-serial' port='0'/> + <target type='spapr-vty' port='0'/> <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-console-native.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-console-native.xml new file mode 120000 index 000000000..b0e645fc0 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-console-native.xml @@ -0,0 +1 @@ +qemuxml2xmlout-pseries-serial-native.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-console-virtio.xml similarity index 74% copy from tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml copy to tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-console-virtio.xml index a563b6ddd..48760f282 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-console-virtio.xml @@ -1,5 +1,5 @@ <domain type='qemu'> - <name>QEMUGuest1</name> + <name>guest</name> <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> <memory unit='KiB'>524288</memory> <currentMemory unit='KiB'>524288</currentMemory> @@ -14,20 +14,16 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-ppc64</emulator> - <controller type='usb' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> - </controller> + <controller type='usb' index='0' model='none'/> <controller type='pci' index='0' model='pci-root'> <model name='spapr-pci-host-bridge'/> <target index='0'/> </controller> - <serial type='pty'> - <target type='isa-serial' port='0'/> - <address type='spapr-vio' reg='0x30000000'/> - </serial> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> <console type='pty'> - <target type='serial' port='0'/> - <address type='spapr-vio' reg='0x30000000'/> + <target type='virtio' port='0'/> </console> <memballoon model='none'/> <panic model='pseries'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat-power9.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat-power9.xml index 59587b3c3..88fb2dd61 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat-power9.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat-power9.xml @@ -25,7 +25,7 @@ <target index='0'/> </controller> <serial type='pty'> - <target type='isa-serial' port='0'/> + <target type='spapr-vty' port='0'/> <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat.xml index a39e1fd01..f5176b1d6 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat.xml @@ -25,7 +25,7 @@ <target index='0'/> </controller> <serial type='pty'> - <target type='isa-serial' port='0'/> + <target type='spapr-vty' port='0'/> <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-exact.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-exact.xml index 666eede1a..ec972327a 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-exact.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-exact.xml @@ -26,7 +26,7 @@ <target index='0'/> </controller> <serial type='pty'> - <target type='isa-serial' port='0'/> + <target type='spapr-vty' port='0'/> <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml index a563b6ddd..ec04b5a3c 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml @@ -22,7 +22,7 @@ <target index='0'/> </controller> <serial type='pty'> - <target type='isa-serial' port='0'/> + <target type='spapr-vty' port='0'/> <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml index a563b6ddd..ec04b5a3c 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml @@ -22,7 +22,7 @@ <target index='0'/> </controller> <serial type='pty'> - <target type='isa-serial' port='0'/> + <target type='spapr-vty' port='0'/> <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial+console-native.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial+console-native.xml new file mode 120000 index 000000000..b0e645fc0 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial+console-native.xml @@ -0,0 +1 @@ +qemuxml2xmlout-pseries-serial-native.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-compat.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-compat.xml new file mode 120000 index 000000000..b0e645fc0 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-compat.xml @@ -0,0 +1 @@ +qemuxml2xmlout-pseries-serial-native.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-native.xml similarity index 81% copy from tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml copy to tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-native.xml index a563b6ddd..6538dc3d8 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-native.xml @@ -1,5 +1,5 @@ <domain type='qemu'> - <name>QEMUGuest1</name> + <name>guest</name> <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> <memory unit='KiB'>524288</memory> <currentMemory unit='KiB'>524288</currentMemory> @@ -14,15 +14,13 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-ppc64</emulator> - <controller type='usb' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> - </controller> + <controller type='usb' index='0' model='none'/> <controller type='pci' index='0' model='pci-root'> <model name='spapr-pci-host-bridge'/> <target index='0'/> </controller> <serial type='pty'> - <target type='isa-serial' port='0'/> + <target type='spapr-vty' port='0'/> <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 1afd0d271..493d82eb4 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -766,6 +766,21 @@ mymain(void) QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT); + DO_TEST("pseries-serial-native", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_SPAPR_VTY); + DO_TEST("pseries-serial+console-native", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_SPAPR_VTY); + DO_TEST("pseries-serial-compat", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_SPAPR_VTY); + DO_TEST("pseries-console-native", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_SPAPR_VTY); + DO_TEST("pseries-console-virtio", + QEMU_CAPS_NODEFCONFIG); + DO_TEST("balloon-device-auto", NONE); DO_TEST("balloon-device-period", NONE); DO_TEST("channel-virtio-auto", NONE); -- 2.13.6

On Wed, Nov 15, 2017 at 12:50:10PM +0100, Andrea Bolognani wrote:
We can finally introduce a specific target type for the spapr-vty device used by pSeries guests, which means isa-serial will no longer show up to confuse users.
We make sure migration works in both directions by interpreting the isa-serial target type, or the lack of target type, appropriately when parsing the guest XML, and skipping the newly-introduced type when formatting if for migration. We also verify that spapr-vty is not used for non-pSeries guests and add a bunch of test cases.
This commit is best viewed with 'git diff -w'.
It would be probably good idea to split it into two patches, one that changes all the if-else conditions to switch and second where the actual new code is introduced.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511421
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/formatdomain.html.in | 11 +- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 5 +- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 115 +++++++++++---------- src/qemu/qemu_domain.c | 38 ++++++- src/qemu/qemu_domain_address.c | 1 + .../qemuxml2argv-pseries-basic.args | 2 +- .../qemuxml2argv-pseries-console-native.args | 1 + .../qemuxml2argv-pseries-console-native.xml | 17 +++ ...gs => qemuxml2argv-pseries-console-virtio.args} | 10 +- .../qemuxml2argv-pseries-console-virtio.xml | 19 ++++ .../qemuxml2argv-pseries-cpu-compat-power9.args | 2 +- .../qemuxml2argv-pseries-cpu-compat.args | 2 +- .../qemuxml2argv-pseries-cpu-exact.args | 2 +- .../qemuxml2argv-pseries-cpu-le.args | 2 +- .../qemuxml2argv-pseries-panic-missing.args | 2 +- .../qemuxml2argv-pseries-panic-no-address.args | 2 +- ...qemuxml2argv-pseries-serial+console-native.args | 1 + .../qemuxml2argv-pseries-serial+console-native.xml | 18 ++++ .../qemuxml2argv-pseries-serial-compat.args | 1 + .../qemuxml2argv-pseries-serial-compat.xml | 19 ++++ ...qemuxml2argv-pseries-serial-invalid-machine.xml | 19 ++++ ...rgs => qemuxml2argv-pseries-serial-native.args} | 7 +- .../qemuxml2argv-pseries-serial-native.xml | 16 +++ .../qemuxml2argv-pseries-usb-default.args | 2 +- .../qemuxml2argv-pseries-usb-kbd.args | 2 +- .../qemuxml2argv-pseries-usb-multi.args | 2 +- .../qemuxml2argv-pseries-vio-user-assigned.args | 4 +- .../qemuxml2argvdata/qemuxml2argv-pseries-vio.args | 4 +- tests/qemuxml2argvtest.c | 16 +++ .../qemuxml2xmlout-panic-pseries.xml | 2 +- .../qemuxml2xmlout-pseries-console-native.xml | 1 + ...l => qemuxml2xmlout-pseries-console-virtio.xml} | 16 ++- .../qemuxml2xmlout-pseries-cpu-compat-power9.xml | 2 +- .../qemuxml2xmlout-pseries-cpu-compat.xml | 2 +- .../qemuxml2xmlout-pseries-cpu-exact.xml | 2 +- .../qemuxml2xmlout-pseries-panic-missing.xml | 2 +- .../qemuxml2xmlout-pseries-panic-no-address.xml | 2 +- ...emuxml2xmlout-pseries-serial+console-native.xml | 1 + .../qemuxml2xmlout-pseries-serial-compat.xml | 1 + ...ml => qemuxml2xmlout-pseries-serial-native.xml} | 8 +- tests/qemuxml2xmltest.c | 15 +++ 43 files changed, 288 insertions(+), 110 deletions(-) create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-pseries-console-native.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-console-native.xml copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-basic.args => qemuxml2argv-pseries-console-virtio.args} (59%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-console-virtio.xml create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial+console-native.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial+console-native.xml create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-compat.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-compat.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-invalid-machine.xml copy tests/qemuxml2argvdata/{qemuxml2argv-pseries-basic.args => qemuxml2argv-pseries-serial-native.args} (70%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-native.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-console-native.xml copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-panic-pseries.xml => qemuxml2xmlout-pseries-console-virtio.xml} (74%) create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial+console-native.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-compat.xml copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-panic-pseries.xml => qemuxml2xmlout-pseries-serial-native.xml} (81%)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8dbea6af7..7ff097d65 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6526,10 +6526,13 @@ qemu-kvm -net nic,model=? /dev/null specifies the port number. Ports are numbered starting from 0. There are usually 0, 1 or 2 serial ports. There is also an optional <code>type</code> attribute <span class="since">since 1.0.2</span> - which has three choices for its value, one is <code>isa-serial</code>, - then <code>usb-serial</code> and last one is <code>pci-serial</code>. - If <code>type</code> is missing, <code>isa-serial</code> will be used by - default. For <code>usb-serial</code> an optional sub-element + which can be used to pick between <code>isa-serial</code>, + <code>usb-serial</code>, <code>pci-serial</code> and, + <span class="since">since 3.10.0</span>, <code>spapr-vty</code>. + Some values are not compatible with all architecture and machine types; + if the value is missing altogether, libvirt will try to pick an + appropriate default. + For <code>usb-serial</code> an optional sub-element <code><address/></code> with <code>type='usb'</code> can tie the device to a particular controller, <a href="#elementsAddress">documented above</a>. Similarly, <code>pci-serial</code> can be used to attach the device to diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 82fdfd5f7..8f6d035de 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3585,6 +3585,7 @@ <value>isa-serial</value> <value>usb-serial</value> <value>pci-serial</value> + <value>spapr-vty</value>
This name doesn't feel right. Previous names are based on the BUS that the serial device is connected with "-serial" appended to the BUS name. Since sPAPR is specification that defines a set of para-virtualized devices, there is no actual BUS, but I think that in this case we can consider spapr as a BUS name and use "spapr-serial". It would be better than just copying the device name from QEMU. Pavel

On Thu, 2017-11-16 at 14:22 +0100, Pavel Hrdina wrote:
On Wed, Nov 15, 2017 at 12:50:10PM +0100, Andrea Bolognani wrote:
We can finally introduce a specific target type for the spapr-vty device used by pSeries guests, which means isa-serial will no longer show up to confuse users.
We make sure migration works in both directions by interpreting the isa-serial target type, or the lack of target type, appropriately when parsing the guest XML, and skipping the newly-introduced type when formatting if for migration. We also verify that spapr-vty is not used for non-pSeries guests and add a bunch of test cases.
This commit is best viewed with 'git diff -w'.
It would be probably good idea to split it into two patches, one that changes all the if-else conditions to switch and second where the actual new code is introduced.
I'm not changing any if-else to switch, I'm just folding a special case that was outside of the existing all-encompassing switch back into it and getting rid of the if-else that only existed to support that special case at the same time. I can't really think of a good way to split the change right now, plus I think what's happening is pretty clear if you use '-w'.
@@ -3585,6 +3585,7 @@ <value>isa-serial</value> <value>usb-serial</value> <value>pci-serial</value> + <value>spapr-vty</value>
This name doesn't feel right. Previous names are based on the BUS that the serial device is connected with "-serial" appended to the BUS name. Since sPAPR is specification that defines a set of para-virtualized devices, there is no actual BUS, but I think that in this case we can consider spapr as a BUS name and use "spapr-serial". It would be better than just copying the device name from QEMU.
I'm not sure that's how it went: it looks to me like all the *-serial names have been adopted merely because that's what the QEMU folks had chosen for the corresponding device. We could abstract this further but that would mean adding another layer of translation, since at the moment we're basically passing it through to QEMU and that would no longer fly. I'm not opposed to doing that on principle, but both for pSeries and for other non-x86 architecture, as you noted in response to other commits, obvious candidates for the name don't necessarily exist in the same way they do for ISA, USB and PCI. So I wonder whether it would be worth adding more machinery when the proposed names, while maybe not pretty, do not cause any ambiguity and can be matched to the corresponding platform just as easily. -- Andrea Bolognani / Red Hat / Virtualization

On Thu, Nov 16, 2017 at 03:44:57PM +0100, Andrea Bolognani wrote:
On Thu, 2017-11-16 at 14:22 +0100, Pavel Hrdina wrote:
On Wed, Nov 15, 2017 at 12:50:10PM +0100, Andrea Bolognani wrote:
We can finally introduce a specific target type for the spapr-vty device used by pSeries guests, which means isa-serial will no longer show up to confuse users.
We make sure migration works in both directions by interpreting the isa-serial target type, or the lack of target type, appropriately when parsing the guest XML, and skipping the newly-introduced type when formatting if for migration. We also verify that spapr-vty is not used for non-pSeries guests and add a bunch of test cases.
This commit is best viewed with 'git diff -w'.
It would be probably good idea to split it into two patches, one that changes all the if-else conditions to switch and second where the actual new code is introduced.
I'm not changing any if-else to switch, I'm just folding a special case that was outside of the existing all-encompassing switch back into it and getting rid of the if-else that only existed to support that special case at the same time.
Right, you are folding the if-else into an existing switch.
I can't really think of a good way to split the change right now, plus I think what's happening is pretty clear if you use '-w'.
Well, it wasn't that clear to me, obviously, even if I used '-w'. Now I can see that the folding isn't possible without introducing the new spapr type. So ignore this comment :).
@@ -3585,6 +3585,7 @@ <value>isa-serial</value> <value>usb-serial</value> <value>pci-serial</value> + <value>spapr-vty</value>
This name doesn't feel right. Previous names are based on the BUS that the serial device is connected with "-serial" appended to the BUS name. Since sPAPR is specification that defines a set of para-virtualized devices, there is no actual BUS, but I think that in this case we can consider spapr as a BUS name and use "spapr-serial". It would be better than just copying the device name from QEMU.
I'm not sure that's how it went: it looks to me like all the *-serial names have been adopted merely because that's what the QEMU folks had chosen for the corresponding device.
I believe that it was like you are describing :).
We could abstract this further but that would mean adding another layer of translation, since at the moment we're basically passing it through to QEMU and that would no longer fly.
That's not an issue that we would not pass it directly to QEMU, and sometimes it's even better to abstract it a little bit.
I'm not opposed to doing that on principle, but both for pSeries and for other non-x86 architecture, as you noted in response to other commits, obvious candidates for the name don't necessarily exist in the same way they do for ISA, USB and PCI. So I wonder whether it would be worth adding more machinery when the proposed names, while maybe not pretty, do not cause any ambiguity and can be matched to the corresponding platform just as easily.
I would like to make it right and not to use names that will backfire later. Pavel

On Wed, Nov 15, 2017 at 12:50 PM +0100, Andrea Bolognani <abologna@redhat.com> wrote:
We can finally introduce a specific target type for the spapr-vty device used by pSeries guests, which means isa-serial will no longer show up to confuse users.
We make sure migration works in both directions by interpreting the isa-serial target type, or the lack of target type, appropriately when parsing the guest XML, and skipping the newly-introduced type when formatting if for migration. We also verify that spapr-vty is not used for non-pSeries guests and add a bunch of test cases.
This commit is best viewed with 'git diff -w'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511421
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/formatdomain.html.in | 11 +- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 5 +-
+ for (i = 0; i < def->nserials; i++) { + virDomainChrDefPtr serial = def->serials[i]; + + /* Historically, the native console type for some machine types + * was not set at all, which means it defaulted to ISA even + * though that was not even remotely accurate. To ensure migration + * towards older libvirt versions works for such guests, we switch + * it back to the default here */ + if (flags & VIR_DOMAIN_XML_MIGRATABLE) { ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This condition is unneeded here as we already checked this ~100 lines before.
+ switch ((virDomainChrSerialTargetType) serial->targetType) { + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR: + serial->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE; + break;
[...snip] -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On Fri, 2017-11-17 at 11:34 +0100, Marc Hartmayer wrote:
On Wed, Nov 15, 2017 at 12:50 PM +0100, Andrea Bolognani <abologna@redhat.com> wrote:
+ for (i = 0; i < def->nserials; i++) { + virDomainChrDefPtr serial = def->serials[i]; + + /* Historically, the native console type for some machine types + * was not set at all, which means it defaulted to ISA even + * though that was not even remotely accurate. To ensure migration + * towards older libvirt versions works for such guests, we switch + * it back to the default here */ + if (flags & VIR_DOMAIN_XML_MIGRATABLE) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This condition is unneeded here as we already checked this ~100 lines before.
Excellent point, thanks for spotting that :) -- Andrea Bolognani / Red Hat / Virtualization

The existing implementation set the address type for all serial devices to spapr-vio, which made it impossible to use other devices such as usb-serial and pci-serial; moreover, some decisions were made based on the address type rather than the device type. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1512934 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_command.c | 9 ------- src/qemu/qemu_domain_address.c | 3 ++- .../qemuxml2argv-pseries-serial-pci.args | 22 ++++++++++++++++ .../qemuxml2argv-pseries-serial-pci.xml | 18 +++++++++++++ .../qemuxml2argv-pseries-serial-usb.args | 23 +++++++++++++++++ .../qemuxml2argv-pseries-serial-usb.xml | 21 +++++++++++++++ tests/qemuxml2argvtest.c | 7 +++++ .../qemuxml2xmlout-pseries-serial-pci.xml | 29 +++++++++++++++++++++ .../qemuxml2xmlout-pseries-serial-usb.xml | 30 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 7 +++++ 10 files changed, 159 insertions(+), 10 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-pci.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-pci.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-usb.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-usb.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-pci.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-usb.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7b6961bf2..c38b9ca29 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9127,15 +9127,6 @@ static bool qemuChrIsPlatformDevice(const virDomainDef *def, virDomainChrDefPtr chr) { - if ((def->os.arch == VIR_ARCH_PPC) || ARCH_IS_PPC64(def->os.arch)) { - if (!qemuDomainIsPSeries(def)) - return true; - /* only pseries need -device spapr-vty with -chardev */ - if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && - chr->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) - return true; - } - if (def->os.arch == VIR_ARCH_ARMV7L || def->os.arch == VIR_ARCH_AARCH64) { /* TARGET_TYPE_ISA here really means 'the default platform device' */ if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 46e91f9fe..27ec5b6c4 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -246,8 +246,9 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, for (i = 0; i < def->nserials; i++) { if (def->serials[i]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && - qemuDomainIsPSeries(def)) + def->serials[i]->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR) { def->serials[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; + } if (qemuDomainAssignSpaprVIOAddress(def, &def->serials[i]->info, VIO_ADDR_SERIAL) < 0) goto cleanup; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-pci.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-pci.args new file mode 100644 index 000000000..eb2a9bf0e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-pci.args @@ -0,0 +1,22 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-ppc64 \ +-name guest \ +-S \ +-M pseries \ +-m 512 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-boot c \ +-chardev pty,id=charserial0 \ +-device pci-serial,chardev=charserial0,id=serial0,bus=pci.0,addr=0x1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-pci.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-pci.xml new file mode 100644 index 000000000..2c2534b4c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-pci.xml @@ -0,0 +1,18 @@ +<domain type='qemu'> + <name>guest</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> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' model='none'/> + <!-- This will be assigned a PCI address --> + <serial type='pty'> + <target type='pci-serial'/> + </serial> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-usb.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-usb.args new file mode 100644 index 000000000..0403985dc --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-usb.args @@ -0,0 +1,23 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-ppc64 \ +-name guest \ +-S \ +-M pseries \ +-m 512 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-boot c \ +-device qemu-xhci,id=usb,bus=pci.0,addr=0x1 \ +-chardev pty,id=charserial0 \ +-device usb-serial,chardev=charserial0,id=serial0,bus=usb.0,port=1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-usb.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-usb.xml new file mode 100644 index 000000000..734c90c66 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-usb.xml @@ -0,0 +1,21 @@ +<domain type='qemu'> + <name>guest</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> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' model='qemu-xhci'/> + <!-- This should be assigned a USB address. You'll not be able to find it + in the file generated by qemuxml2xmltest due to limitations in the + test suite, but it will be there when actually running libvirt; + moreover, the USB address will be present in the .args file --> + <serial type='pty'> + <target type='usb-serial'/> + </serial> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 843171cfa..4dcfe4f63 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1881,6 +1881,13 @@ mymain(void) DO_TEST("pseries-serial-compat", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DEVICE_SPAPR_VTY); + DO_TEST("pseries-serial-pci", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_PCI_SERIAL); + DO_TEST("pseries-serial-usb", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_QEMU_XHCI, + QEMU_CAPS_DEVICE_USB_SERIAL); DO_TEST("pseries-console-native", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DEVICE_SPAPR_VTY); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-pci.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-pci.xml new file mode 100644 index 000000000..3c2ddb456 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-pci.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> + <serial type='pty'> + <target type='pci-serial' port='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </serial> + <memballoon model='none'/> + <panic model='pseries'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-usb.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-usb.xml new file mode 100644 index 000000000..ce654e00d --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-usb.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' index='0' model='qemu-xhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> + <serial type='pty'> + <target type='usb-serial' port='0'/> + </serial> + <memballoon model='none'/> + <panic model='pseries'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 493d82eb4..f590fa937 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -775,6 +775,13 @@ mymain(void) DO_TEST("pseries-serial-compat", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DEVICE_SPAPR_VTY); + DO_TEST("pseries-serial-pci", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_PCI_SERIAL); + DO_TEST("pseries-serial-usb", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_QEMU_XHCI, + QEMU_CAPS_DEVICE_USB_SERIAL); DO_TEST("pseries-console-native", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DEVICE_SPAPR_VTY); -- 2.13.6

On Wed, Nov 15, 2017 at 12:50:11PM +0100, Andrea Bolognani wrote:
The existing implementation set the address type for all serial devices to spapr-vio, which made it impossible to use other devices such as usb-serial and pci-serial; moreover, some decisions were made based on the address type rather than the device type.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1512934
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_command.c | 9 ------- src/qemu/qemu_domain_address.c | 3 ++- .../qemuxml2argv-pseries-serial-pci.args | 22 ++++++++++++++++ .../qemuxml2argv-pseries-serial-pci.xml | 18 +++++++++++++ .../qemuxml2argv-pseries-serial-usb.args | 23 +++++++++++++++++ .../qemuxml2argv-pseries-serial-usb.xml | 21 +++++++++++++++ tests/qemuxml2argvtest.c | 7 +++++ .../qemuxml2xmlout-pseries-serial-pci.xml | 29 +++++++++++++++++++++ .../qemuxml2xmlout-pseries-serial-usb.xml | 30 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 7 +++++ 10 files changed, 159 insertions(+), 10 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-pci.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-pci.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-usb.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-serial-usb.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-pci.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-serial-usb.xml
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

We can finally introduce a specific target type for the pl011 device used by mach-virt guests, which means isa-serial will no longer show up to confuse users. We make sure migration works in both directions by interpreting the isa-serial target type, or the lack of target type, appropriately when parsing the guest XML, and skipping the newly-introduced type when formatting if for migration. We also verify that pl011 is not used for non-mach-virt guests and add a bunch of test cases. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=151292 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/formatdomain.html.in | 3 +- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 2 ++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 7 ++-- src/qemu/qemu_domain.c | 12 +++++++ src/qemu/qemu_domain_address.c | 1 + .../qemuxml2argv-mach-virt-console-native.args | 1 + .../qemuxml2argv-mach-virt-console-native.xml | 17 +++++++++ .../qemuxml2argv-mach-virt-console-virtio.args | 24 +++++++++++++ .../qemuxml2argv-mach-virt-console-virtio.xml | 19 ++++++++++ ...muxml2argv-mach-virt-serial+console-native.args | 1 + ...emuxml2argv-mach-virt-serial+console-native.xml | 18 ++++++++++ .../qemuxml2argv-mach-virt-serial-compat.args | 1 + .../qemuxml2argv-mach-virt-serial-compat.xml | 19 ++++++++++ ...muxml2argv-mach-virt-serial-invalid-machine.xml | 19 ++++++++++ .../qemuxml2argv-mach-virt-serial-native.args | 23 ++++++++++++ .../qemuxml2argv-mach-virt-serial-native.xml | 16 +++++++++ .../qemuxml2argv-mach-virt-serial-pci.args | 26 ++++++++++++++ .../qemuxml2argv-mach-virt-serial-pci.xml | 18 ++++++++++ .../qemuxml2argv-mach-virt-serial-usb.args | 27 ++++++++++++++ .../qemuxml2argv-mach-virt-serial-usb.xml | 21 +++++++++++ tests/qemuxml2argvtest.c | 27 ++++++++++++++ .../qemuxml2xmlout-aarch64-virtio-pci-default.xml | 2 +- .../qemuxml2xmlout-mach-virt-console-native.xml | 1 + .../qemuxml2xmlout-mach-virt-console-virtio.xml | 27 ++++++++++++++ ...uxml2xmlout-mach-virt-serial+console-native.xml | 1 + .../qemuxml2xmlout-mach-virt-serial-compat.xml | 29 +++++++++++++++ .../qemuxml2xmlout-mach-virt-serial-native.xml | 1 + .../qemuxml2xmlout-mach-virt-serial-pci.xml | 42 ++++++++++++++++++++++ .../qemuxml2xmlout-mach-virt-serial-usb.xml | 39 ++++++++++++++++++++ tests/qemuxml2xmltest.c | 26 ++++++++++++++ 32 files changed, 468 insertions(+), 4 deletions(-) create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-native.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-native.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-virtio.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-virtio.xml create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial+console-native.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial+console-native.xml create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-compat.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-compat.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-invalid-machine.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-native.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-native.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-pci.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-pci.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-usb.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-usb.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-console-native.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-console-virtio.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial+console-native.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-compat.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-native.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-pci.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-usb.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7ff097d65..9f46b411a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6528,7 +6528,8 @@ qemu-kvm -net nic,model=? /dev/null <code>type</code> attribute <span class="since">since 1.0.2</span> which can be used to pick between <code>isa-serial</code>, <code>usb-serial</code>, <code>pci-serial</code> and, - <span class="since">since 3.10.0</span>, <code>spapr-vty</code>. + <span class="since">since 3.10.0</span>, <code>spapr-vty</code> and + <code>pl011</code>. Some values are not compatible with all architecture and machine types; if the value is missing altogether, libvirt will try to pick an appropriate default. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 8f6d035de..6dca83a62 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3586,6 +3586,7 @@ <value>usb-serial</value> <value>pci-serial</value> <value>spapr-vty</value> + <value>pl011</value> </choice> </attribute> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f7faa1a35..aadbfc0cd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -451,6 +451,7 @@ VIR_ENUM_IMPL(virDomainChrSerialTarget, "usb-serial", "pci-serial", "spapr-vty", + "pl011", ); VIR_ENUM_IMPL(virDomainChrChannelTarget, @@ -4043,6 +4044,7 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def) switch ((virDomainChrSerialTargetType) def->serials[0]->targetType) { case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PL011: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: { /* Create a stub console to match the serial port. diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6dd97a0a3..ae239b09c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1086,6 +1086,7 @@ typedef enum { VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR, + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PL011, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST } virDomainChrSerialTargetType; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c38b9ca29..fb6f69123 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9128,10 +9128,12 @@ qemuChrIsPlatformDevice(const virDomainDef *def, virDomainChrDefPtr chr) { if (def->os.arch == VIR_ARCH_ARMV7L || def->os.arch == VIR_ARCH_AARCH64) { - /* TARGET_TYPE_ISA here really means 'the default platform device' */ + + /* pl011 (used on mach-virt) is a platform device */ if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && - chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA) + chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PL011) { return true; + } } /* If we got all the way here and we're still stuck with the default @@ -10247,6 +10249,7 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, } break; + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PL011: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: /* Except from _LAST, which is just a guard value and will never diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9cafbdd27..b506fedd0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3513,6 +3513,15 @@ qemuDomainChrDefValidate(const virDomainChrDef *dev, return -1; } + if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + dev->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PL011 && + !qemuDomainIsVirt(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("pl011 serial devices are only supported on " + "mach-virt guests")); + return -1; + } + return 0; } @@ -4071,6 +4080,8 @@ qemuDomainChrDefPostParse(virDomainChrDefPtr chr, chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; } else if (qemuDomainIsPSeries(def)) { chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR; + } else if (qemuDomainIsVirt(def)) { + chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PL011; } } @@ -4985,6 +4996,7 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver, if (flags & VIR_DOMAIN_XML_MIGRATABLE) { switch ((virDomainChrSerialTargetType) serial->targetType) { case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PL011: serial->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE; break; case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 27ec5b6c4..4a7854740 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -784,6 +784,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PL011: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: return 0; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-native.args b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-native.args new file mode 120000 index 000000000..1a90484d3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-native.args @@ -0,0 +1 @@ +qemuxml2argv-mach-virt-serial-native.args \ No newline at end of file diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-native.xml b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-native.xml new file mode 100644 index 000000000..6aba864d0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-native.xml @@ -0,0 +1,17 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' model='none'/> + <!-- The <console> element being present should result in a matching + <serial> element being created --> + <console type='pty'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-virtio.args b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-virtio.args new file mode 100644 index 000000000..2a862bff1 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-virtio.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-aarch64 \ +-name guest \ +-S \ +-M virt \ +-m 512 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-device virtio-serial,id=virtio-serial0 \ +-chardev pty,id=charconsole0 \ +-device virtconsole,chardev=charconsole0,id=console0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-virtio.xml b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-virtio.xml new file mode 100644 index 000000000..92704504c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-console-virtio.xml @@ -0,0 +1,19 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' model='none'/> + <!-- The <console> element being present should *not* result in a + matching <serial> element being created --> + <console type='pty'> + <target type='virtio'/> + </console> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial+console-native.args b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial+console-native.args new file mode 120000 index 000000000..1a90484d3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial+console-native.args @@ -0,0 +1 @@ +qemuxml2argv-mach-virt-serial-native.args \ No newline at end of file diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial+console-native.xml b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial+console-native.xml new file mode 100644 index 000000000..549b764e9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial+console-native.xml @@ -0,0 +1,18 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' model='none'/> + <!-- When both the <serial> and <console> elements are present, they will + be matched and end up representing the same native serial console --> + <serial type='pty'/> + <console type='pty'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-compat.args b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-compat.args new file mode 120000 index 000000000..1a90484d3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-compat.args @@ -0,0 +1 @@ +qemuxml2argv-mach-virt-serial-native.args \ No newline at end of file diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-compat.xml b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-compat.xml new file mode 100644 index 000000000..9e6be3ffe --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-compat.xml @@ -0,0 +1,19 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' model='none'/> + <!-- isa-serial has to be accepted for backwards compatibility reasons, + but should get converted to the proper type (pl011) --> + <serial type='pty'> + <target type='isa-serial'/> + </serial> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-invalid-machine.xml b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-invalid-machine.xml new file mode 100644 index 000000000..a874a026b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-invalid-machine.xml @@ -0,0 +1,19 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' model='none'/> + <!-- The pl011 serial console can only be used for mach-virt guests, + so this should be rejected --> + <serial type='pty'> + <target type='pl011'/> + </serial> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-native.args b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-native.args new file mode 100644 index 000000000..f4bfce376 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-native.args @@ -0,0 +1,23 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-aarch64 \ +-name guest \ +-S \ +-M virt \ +-m 512 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-chardev pty,id=charserial0 \ +-serial chardev:charserial0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-native.xml b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-native.xml new file mode 100644 index 000000000..817f606ee --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-native.xml @@ -0,0 +1,16 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' model='none'/> + <!-- This will use the pl011 target type --> + <serial type='pty'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-pci.args b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-pci.args new file mode 100644 index 000000000..334194efe --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-pci.args @@ -0,0 +1,26 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-aarch64 \ +-name guest \ +-S \ +-M virt \ +-m 512 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1 \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device pcie-root-port,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ +-chardev pty,id=charserial0 \ +-device pci-serial,chardev=charserial0,id=serial0,bus=pci.2,addr=0x1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-pci.xml b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-pci.xml new file mode 100644 index 000000000..29aa7664b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-pci.xml @@ -0,0 +1,18 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' model='none'/> + <!-- This will be assigned a PCI address --> + <serial type='pty'> + <target type='pci-serial'/> + </serial> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-usb.args b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-usb.args new file mode 100644 index 000000000..44c4027ac --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-usb.args @@ -0,0 +1,27 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-aarch64 \ +-name guest \ +-S \ +-M virt \ +-m 512 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-device pcie-root-port,port=0x8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,\ +addr=0x1 \ +-device pcie-root-port,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 \ +-device qemu-xhci,id=usb,bus=pci.1,addr=0x0 \ +-chardev pty,id=charserial0 \ +-device usb-serial,chardev=charserial0,id=serial0,bus=usb.0,port=1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-usb.xml b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-usb.xml new file mode 100644 index 000000000..35f192a3e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-mach-virt-serial-usb.xml @@ -0,0 +1,21 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' model='qemu-xhci'/> + <!-- This should be assigned a USB address. You'll not be able to find it + in the file generated by qemuxml2xmltest due to limitations in the + test suite, but it will be there when actually running libvirt; + moreover, the USB address will be present in the .args file --> + <serial type='pty'> + <target type='usb-serial'/> + </serial> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4dcfe4f63..24662cc09 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1895,6 +1895,33 @@ mymain(void) QEMU_CAPS_NODEFCONFIG); DO_TEST_PARSE_ERROR("pseries-serial-invalid-machine", NONE); + DO_TEST("mach-virt-serial-native", + QEMU_CAPS_NODEFCONFIG); + DO_TEST("mach-virt-serial+console-native", + QEMU_CAPS_NODEFCONFIG); + DO_TEST("mach-virt-serial-compat", + QEMU_CAPS_NODEFCONFIG); + DO_TEST("mach-virt-serial-pci", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_SERIAL); + DO_TEST("mach-virt-serial-usb", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_DEVICE_QEMU_XHCI, + QEMU_CAPS_DEVICE_USB_SERIAL); + DO_TEST("mach-virt-console-native", + QEMU_CAPS_NODEFCONFIG); + DO_TEST("mach-virt-console-virtio", + QEMU_CAPS_NODEFCONFIG); + DO_TEST_PARSE_ERROR("mach-virt-serial-invalid-machine", NONE); + DO_TEST("disk-ide-drive-split", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml index e5496424b..af192b1e5 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml @@ -71,7 +71,7 @@ <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> </interface> <serial type='pty'> - <target port='0'/> + <target type='pl011' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-console-native.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-console-native.xml new file mode 120000 index 000000000..a4768fcf8 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-console-native.xml @@ -0,0 +1 @@ +qemuxml2xmlout-mach-virt-serial-compat.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-console-virtio.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-console-virtio.xml new file mode 100644 index 000000000..3e46cd201 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-console-virtio.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <gic version='2'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='virtio-serial' index='0'/> + <console type='pty'> + <target type='virtio' port='0'/> + </console> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial+console-native.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial+console-native.xml new file mode 120000 index 000000000..a4768fcf8 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial+console-native.xml @@ -0,0 +1 @@ +qemuxml2xmlout-mach-virt-serial-compat.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-compat.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-compat.xml new file mode 100644 index 000000000..b648858ee --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-compat.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <gic version='2'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' index='0' model='none'/> + <serial type='pty'> + <target type='pl011' port='0'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + </console> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-native.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-native.xml new file mode 120000 index 000000000..a4768fcf8 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-native.xml @@ -0,0 +1 @@ +qemuxml2xmlout-mach-virt-serial-compat.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-pci.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-pci.xml new file mode 100644 index 000000000..955f593c9 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-pci.xml @@ -0,0 +1,42 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <gic version='2'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='3' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <serial type='pty'> + <target type='pci-serial' port='0'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </serial> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-usb.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-usb.xml new file mode 100644 index 000000000..8829e7b6f --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-mach-virt-serial-usb.xml @@ -0,0 +1,39 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <gic version='2'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' index='0' model='qemu-xhci'> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='1' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='2' port='0x9'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <serial type='pty'> + <target type='usb-serial' port='0'/> + </serial> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f590fa937..9fb09316b 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -788,6 +788,32 @@ mymain(void) DO_TEST("pseries-console-virtio", QEMU_CAPS_NODEFCONFIG); + DO_TEST("mach-virt-serial-native", + QEMU_CAPS_NODEFCONFIG); + DO_TEST("mach-virt-serial+console-native", + QEMU_CAPS_NODEFCONFIG); + DO_TEST("mach-virt-serial-compat", + QEMU_CAPS_NODEFCONFIG); + DO_TEST("mach-virt-serial-pci", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_SERIAL); + DO_TEST("mach-virt-serial-usb", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_DEVICE_QEMU_XHCI, + QEMU_CAPS_DEVICE_USB_SERIAL); + DO_TEST("mach-virt-console-native", + QEMU_CAPS_NODEFCONFIG); + DO_TEST("mach-virt-console-virtio", + QEMU_CAPS_NODEFCONFIG); + DO_TEST("balloon-device-auto", NONE); DO_TEST("balloon-device-period", NONE); DO_TEST("channel-virtio-auto", NONE); -- 2.13.6

On Wed, Nov 15, 2017 at 12:50:12PM +0100, Andrea Bolognani wrote:
We can finally introduce a specific target type for the pl011 device used by mach-virt guests, which means isa-serial will no longer show up to confuse users.
We make sure migration works in both directions by interpreting the isa-serial target type, or the lack of target type, appropriately when parsing the guest XML, and skipping the newly-introduced type when formatting if for migration. We also verify that pl011 is not used for non-mach-virt guests and add a bunch of test cases.
Here the name "pl011" is even worse than "spapr-vty". It's a device name and there is also "pl022" (probably not supported by QEMU). The bus name is APB (Advanced Peripheral Bus). [1] [2] How about we introduce another attribute/element that would specify the exact model of the serial device and it would be optional, libvirt would be able to choose the model if none is specified. I was no able to find anything about s390 and its sclp/sclplm consoles but it's the same case. I would expect that both devices are connected to the same BUS, we just need to find the BUS name to use it $BUS-serial and have sclp/sclplm as models. Other than the naming the patch looks good. Pavel [1] <http://infocenter.arm.com/help/topic/com.arm.doc.ddi0183f/DDI0183.pdf> [2] <http://infocenter.arm.com/help/topic/com.arm.doc.ddi0194g/DDI0194G_ssp_pl022_r1p3_trm.pdf>

On Thu, 2017-11-16 at 14:56 +0100, Pavel Hrdina wrote:
Here the name "pl011" is even worse than "spapr-vty". It's a device name and there is also "pl022" (probably not supported by QEMU). The bus name is APB (Advanced Peripheral Bus). [1] [2]
QEMU has pl031 and pl061 devices in addition to pl011, but no pl022 as far as I can tell.
How about we introduce another attribute/element that would specify the exact model of the serial device and it would be optional, libvirt would be able to choose the model if none is specified.
That would have precedents in eg. PCI controllers, where the <model> subelement contains the hypervisor-specific device name whereas the 'model' attribute of the <controller> element contains a more generic name... You might have just convinced me :) Of course that would lead to duplicated information in the existing cases of ISA, USB and PCI, but I guess we can live with that.
I was no able to find anything about s390 and its sclp/sclplm consoles but it's the same case. I would expect that both devices are connected to the same BUS, we just need to find the BUS name to use it $BUS-serial and have sclp/sclplm as models.
Pino, any ideas about this? -- Andrea Bolognani / Red Hat / Virtualization

On Thu, Nov 16, 2017 at 03:53:42PM +0100, Andrea Bolognani wrote:
On Thu, 2017-11-16 at 14:56 +0100, Pavel Hrdina wrote:
Here the name "pl011" is even worse than "spapr-vty". It's a device name and there is also "pl022" (probably not supported by QEMU). The bus name is APB (Advanced Peripheral Bus). [1] [2]
QEMU has pl031 and pl061 devices in addition to pl011, but no pl022 as far as I can tell.
How about we introduce another attribute/element that would specify the exact model of the serial device and it would be optional, libvirt would be able to choose the model if none is specified.
That would have precedents in eg. PCI controllers, where the <model> subelement contains the hypervisor-specific device name whereas the 'model' attribute of the <controller> element contains a more generic name...
You might have just convinced me :)
good :)
Of course that would lead to duplicated information in the existing cases of ISA, USB and PCI, but I guess we can live with that.
Or the duplication can be solved by documenting, that the model is valid only for specific types where the model makes sense.
I was no able to find anything about s390 and its sclp/sclplm consoles but it's the same case. I would expect that both devices are connected to the same BUS, we just need to find the BUS name to use it $BUS-serial and have sclp/sclplm as models.
Pino, any ideas about this?
-- Andrea Bolognani / Red Hat / Virtualization

On Thursday, 16 November 2017 14:56:32 CET Pavel Hrdina wrote:
How about we introduce another attribute/element that would specify the exact model of the serial device and it would be optional, libvirt would be able to choose the model if none is specified.
Can you please provide an example of how it would look like?
I was no able to find anything about s390 and its sclp/sclplm consoles but it's the same case. I would expect that both devices are connected to the same BUS, we just need to find the BUS name to use it $BUS-serial and have sclp/sclplm as models.
There is no real bus for sclp/sclplm -- there is only an internal "bus" in QEMU, but it should not be exposed in upper layers (as it is only an implementation detail). -- Pino Toscano

On Thu, 2017-11-16 at 16:17 +0100, Pino Toscano wrote:
On Thursday, 16 November 2017 14:56:32 CET Pavel Hrdina wrote:
How about we introduce another attribute/element that would specify the exact model of the serial device and it would be optional, libvirt would be able to choose the model if none is specified.
Can you please provide an example of how it would look like?
<serial type='pty'> <target type='xxx-serial> <model name='splc'/> </target> </serial>
I was no able to find anything about s390 and its sclp/sclplm consoles but it's the same case. I would expect that both devices are connected to the same BUS, we just need to find the BUS name to use it $BUS-serial and have sclp/sclplm as models.
There is no real bus for sclp/sclplm -- there is only an internal "bus" in QEMU, but it should not be exposed in upper layers (as it is only an implementation detail).
There's probably some name, in some spec, somewhere :) Or we could use 's390-serial', but that would be suboptimal. -- Andrea Bolognani / Red Hat / Virtualization

On Thursday, 16 November 2017 16:44:24 CET Andrea Bolognani wrote:
On Thu, 2017-11-16 at 16:17 +0100, Pino Toscano wrote:
On Thursday, 16 November 2017 14:56:32 CET Pavel Hrdina wrote:
How about we introduce another attribute/element that would specify the exact model of the serial device and it would be optional, libvirt would be able to choose the model if none is specified.
Can you please provide an example of how it would look like?
<serial type='pty'> <target type='xxx-serial> <model name='splc'/> </target> </serial>
IMHO that looks a bit more convoluted than necessary.
I was no able to find anything about s390 and its sclp/sclplm consoles but it's the same case. I would expect that both devices are connected to the same BUS, we just need to find the BUS name to use it $BUS-serial and have sclp/sclplm as models.
There is no real bus for sclp/sclplm -- there is only an internal "bus" in QEMU, but it should not be exposed in upper layers (as it is only an implementation detail).
There's probably some name, in some spec, somewhere :)
Nope.
Or we could use 's390-serial', but that would be suboptimal.
That is way too generic, and representing something which does not exist (unlike the current -serial ones). I still do not see the advantage of this, though. -- Pino Toscano

On Thu, Nov 16, 2017 at 04:44:24PM +0100, Andrea Bolognani wrote:
On Thu, 2017-11-16 at 16:17 +0100, Pino Toscano wrote:
On Thursday, 16 November 2017 14:56:32 CET Pavel Hrdina wrote:
How about we introduce another attribute/element that would specify the exact model of the serial device and it would be optional, libvirt would be able to choose the model if none is specified.
Can you please provide an example of how it would look like?
<serial type='pty'> <target type='xxx-serial> <model name='splc'/> </target> </serial>
I've done some more digging and the SCLP probably stands for "Service-Call Logical Processor" so we could probably use it as "sclp-serial" and there would be two models "sclpconsole" and "sclplmconsole".
I was no able to find anything about s390 and its sclp/sclplm consoles but it's the same case. I would expect that both devices are connected to the same BUS, we just need to find the BUS name to use it $BUS-serial and have sclp/sclplm as models.
There is no real bus for sclp/sclplm -- there is only an internal "bus" in QEMU, but it should not be exposed in upper layers (as it is only an implementation detail).
There's probably some name, in some spec, somewhere :)
Or we could use 's390-serial', but that would be suboptimal.
-- Andrea Bolognani / Red Hat / Virtualization

From: Pino Toscano <ptoscano@redhat.com> Introduce specific target types for the two console devices (sclp and sclplm) used in s390 and s390x guests, so isa-serial is no more used for them. This makes <serial> usable on s390 and s390x guests, with at most only a single sclpconsole and one sclplmconsole devices usable in a single guest (due to limitations in QEMU, which will enforce already at runtime). Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1449265 Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- docs/formatdomain.html.in | 5 ++-- docs/schemas/domaincommon.rng | 2 ++ src/conf/domain_conf.c | 4 +++ src/conf/domain_conf.h | 2 ++ src/qemu/qemu_command.c | 9 ++++++ src/qemu/qemu_domain.c | 14 ++++++++++ src/qemu/qemu_domain_address.c | 2 ++ .../qemuxml2argv-s390-serial-2.args | 24 ++++++++++++++++ .../qemuxml2argv-s390-serial-2.xml | 17 ++++++++++++ .../qemuxml2argv-s390-serial-console.args | 25 +++++++++++++++++ .../qemuxml2argv-s390-serial-console.xml | 15 ++++++++++ .../qemuxml2argvdata/qemuxml2argv-s390-serial.args | 22 +++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-s390-serial.xml | 14 ++++++++++ tests/qemuxml2argvtest.c | 15 ++++++++++ .../qemuxml2xmlout-s390-serial-2.xml | 29 ++++++++++++++++++++ .../qemuxml2xmlout-s390-serial-console.xml | 32 ++++++++++++++++++++++ .../qemuxml2xmlout-s390-serial.xml | 26 ++++++++++++++++++ tests/qemuxml2xmltest.c | 6 ++++ 18 files changed, 261 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-serial-2.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-serial-2.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-serial.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-serial.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-2.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9f46b411a..7c819d8bf 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6528,8 +6528,9 @@ qemu-kvm -net nic,model=? /dev/null <code>type</code> attribute <span class="since">since 1.0.2</span> which can be used to pick between <code>isa-serial</code>, <code>usb-serial</code>, <code>pci-serial</code> and, - <span class="since">since 3.10.0</span>, <code>spapr-vty</code> and - <code>pl011</code>. + <span class="since">since 3.10.0</span>, <code>spapr-vty</code>, + <code>pl011</code>, <code>sclpconsole</code> and + <code>sclplmconsole</code>. Some values are not compatible with all architecture and machine types; if the value is missing altogether, libvirt will try to pick an appropriate default. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6dca83a62..26e4ee7fd 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3585,6 +3585,8 @@ <value>isa-serial</value> <value>usb-serial</value> <value>pci-serial</value> + <value>sclpconsole</value> + <value>sclplmconsole</value> <value>spapr-vty</value> <value>pl011</value> </choice> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index aadbfc0cd..ccd81ef97 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -452,6 +452,8 @@ VIR_ENUM_IMPL(virDomainChrSerialTarget, "pci-serial", "spapr-vty", "pl011", + "sclpconsole", + "sclplmconsole", ); VIR_ENUM_IMPL(virDomainChrChannelTarget, @@ -4045,6 +4047,8 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def) case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PL011: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLP: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLPLM: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: { /* Create a stub console to match the serial port. diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ae239b09c..93a326018 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1087,6 +1087,8 @@ typedef enum { VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PL011, + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLP, + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLPLM, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST } virDomainChrSerialTargetType; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fb6f69123..4897d0895 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10249,6 +10249,15 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, } break; + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLP: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLPLM: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCLP_S390)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sclp/sclplm console requires QEMU to support s390-sclp")); + return -1; + } + break; + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PL011: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b506fedd0..a6467ba4b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3522,6 +3522,16 @@ qemuDomainChrDefValidate(const virDomainChrDef *dev, return -1; } + if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + (dev->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLP || + dev->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLPLM) && + !ARCH_IS_S390(def->os.arch)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sclp/sclplm serial devices are only supported on " + "s390 and s390x guests")); + return -1; + } + return 0; } @@ -4082,6 +4092,8 @@ qemuDomainChrDefPostParse(virDomainChrDefPtr chr, chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR; } else if (qemuDomainIsVirt(def)) { chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PL011; + } else if (ARCH_IS_S390(def->os.arch)) { + chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLP; } } @@ -5002,6 +5014,8 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver, case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLP: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLPLM: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: /* Nothing to do */ diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 4a7854740..374249e0a 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -785,6 +785,8 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PL011: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLP: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLPLM: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: return 0; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-2.args b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-2.args new file mode 100644 index 000000000..346dcd16b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-2.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-M s390-ccw-virtio \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-boot c \ +-chardev pty,id=charserial0 \ +-device sclpconsole,chardev=charserial0,id=serial0 \ +-chardev pty,id=charserial1 \ +-device sclplmconsole,chardev=charserial1,id=serial1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-2.xml b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-2.xml new file mode 100644 index 000000000..b5371372e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-2.xml @@ -0,0 +1,17 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch="s390x" machine="s390-ccw-virtio">hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <serial type='pty'/> + <serial type='pty'> + <target type='sclplmconsole'/> + </serial> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args new file mode 100644 index 000000000..c405fb59e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-M s390-ccw-virtio \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-boot c \ +-device virtio-serial-ccw,id=virtio-serial0,devno=fe.0.0000 \ +-chardev pty,id=charserial0 \ +-device sclpconsole,chardev=charserial0,id=serial0 \ +-chardev pty,id=charconsole1 \ +-device virtconsole,chardev=charconsole1,id=console1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.xml b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.xml new file mode 100644 index 000000000..c841f1f24 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.xml @@ -0,0 +1,15 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch="s390x" machine="s390-ccw-virtio">hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <serial type='pty'/> + <console type='pty'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-serial.args b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial.args new file mode 100644 index 000000000..20968f794 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial.args @@ -0,0 +1,22 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-M s390-ccw-virtio \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-boot c \ +-chardev pty,id=charserial0 \ +-device sclpconsole,chardev=charserial0,id=serial0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-serial.xml b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial.xml new file mode 100644 index 000000000..55b45bac0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial.xml @@ -0,0 +1,14 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch="s390x" machine="s390-ccw-virtio">hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <serial type='pty'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 24662cc09..a280a8a85 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2046,6 +2046,21 @@ mymain(void) QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST("s390-serial", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_VIRTIO_S390, + QEMU_CAPS_SCLP_S390); + DO_TEST("s390-serial-2", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_VIRTIO_S390, + QEMU_CAPS_SCLP_S390); + DO_TEST("s390-serial-console", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_VIRTIO_S390, + QEMU_CAPS_SCLP_S390); DO_TEST("ppc-dtb", QEMU_CAPS_KVM, diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-2.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-2.xml new file mode 100644 index 000000000..5debc3fad --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-2.xml @@ -0,0 +1,29 @@ +<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'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <serial type='pty'> + <target type='sclpconsole' port='0'/> + </serial> + <serial type='pty'> + <target type='sclplmconsole' port='1'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + </console> + <memballoon model='none'/> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml new file mode 100644 index 000000000..f3364fa23 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml @@ -0,0 +1,32 @@ +<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'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <controller type='virtio-serial' index='0'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> + </controller> + <serial type='pty'> + <target type='sclpconsole' port='0'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + </console> + <console type='pty'> + <target type='virtio' port='0'/> + </console> + <memballoon model='none'/> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial.xml new file mode 100644 index 000000000..201156445 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial.xml @@ -0,0 +1,26 @@ +<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'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <serial type='pty'> + <target type='sclpconsole' port='0'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + </console> + <memballoon model='none'/> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 9fb09316b..bf76d5986 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1155,6 +1155,12 @@ mymain(void) QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); DO_TEST("s390-panic-no-address", QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST("s390-serial", + QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST("s390-serial-2", + QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST("s390-serial-console", + QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); DO_TEST("pcihole64", NONE); DO_TEST("pcihole64-gib", NONE); -- 2.13.6

On Wed, 2017-11-15 at 12:50 +0100, Andrea Bolognani wrote:
@@ -10249,6 +10249,15 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, } break;
+ case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLP: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLPLM: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCLP_S390)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sclp/sclplm console requires QEMU to support s390-sclp")); + return -1; + } + break;
It's existing code, but a single capability should really not be used for both devices. QEMU_CAPS_SCLP_S390 is set whenever sclpconsole is available, so it should be renamed to QEMU_CAPS_DEVICE_SCLPCONSOLE and a new QEMU_CAPS_DEVICE_SCLPLMCONSOLE should be introduced to signal the availability of sclplmconsole separately. With that fixed, and assuming the series gets ACKed up until here, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

From: Pino Toscano <ptoscano@redhat.com> Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/conf/domain_conf.c | 10 ++++++---- src/conf/domain_conf.h | 3 ++- src/vz/vz_sdk.c | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ccd81ef97..9a3cdc777 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3967,7 +3967,8 @@ virDomainDefPostParseMemory(virDomainDefPtr def, static int -virDomainDefAddConsoleCompat(virDomainDefPtr def) +virDomainDefAddConsoleCompat(virDomainDefPtr def, + unsigned int parseFlags ATTRIBUTE_UNUSED) { size_t i; @@ -4929,7 +4930,7 @@ virDomainDefPostParseCommon(virDomainDefPtr def, if (virDomainDefPostParseTimer(def) < 0) return -1; - if (virDomainDefAddImplicitDevices(def) < 0) + if (virDomainDefAddImplicitDevices(def, data->parseFlags) < 0) return -1; if (def->nvideos != 0) { @@ -21945,9 +21946,10 @@ virDomainDefAddImplicitVideo(virDomainDefPtr def) } int -virDomainDefAddImplicitDevices(virDomainDefPtr def) +virDomainDefAddImplicitDevices(virDomainDefPtr def, + unsigned int parseFlags) { - if (virDomainDefAddConsoleCompat(def) < 0) + if (virDomainDefAddConsoleCompat(def, parseFlags) < 0) return -1; if (virDomainDefAddImplicitControllers(def) < 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 93a326018..bab9680b1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2932,7 +2932,8 @@ bool virDomainDefCheckABIStabilityFlags(virDomainDefPtr src, virDomainXMLOptionPtr xmlopt, unsigned int flags); -int virDomainDefAddImplicitDevices(virDomainDefPtr def); +int virDomainDefAddImplicitDevices(virDomainDefPtr def, + unsigned int parseFlags); virDomainIOThreadIDDefPtr virDomainIOThreadIDFind(const virDomainDef *def, unsigned int iothread_id); diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index c9f2a13e4..2e3a3d0f8 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1932,7 +1932,7 @@ prlsdkLoadDomain(vzDriverPtr driver, if (prlsdkGetDomainState(dom, sdkdom, &domainState) < 0) goto error; - if (!IS_CT(def) && virDomainDefAddImplicitDevices(def) < 0) + if (!IS_CT(def) && virDomainDefAddImplicitDevices(def, 0) < 0) goto error; if (def->ngraphics > 0) { -- 2.13.6

On Wed, 2017-11-15 at 12:50 +0100, Andrea Bolognani wrote:
From: Pino Toscano <ptoscano@redhat.com>
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/conf/domain_conf.c | 10 ++++++---- src/conf/domain_conf.h | 3 ++- src/vz/vz_sdk.c | 2 +-
Assuming the series gets ACKed up until here, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

From: Pino Toscano <ptoscano@redhat.com> In case we are allowed to break the ABI of a s390/s390x guest, then convert the first sclp/sclplm console from <console> to <serial>, just like it is done on other architectures. Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/conf/domain_conf.c | 39 ++++++++++++++++++++-- .../qemuxml2argv-s390-console2serial.args | 22 ++++++++++++ .../qemuxml2argv-s390-console2serial.xml | 19 +++++++++++ tests/qemuxml2argvtest.c | 6 ++++ 4 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9a3cdc777..14b611602 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3966,9 +3966,35 @@ virDomainDefPostParseMemory(virDomainDefPtr def, } +static virDomainChrSerialTargetType +virDomainChrConsoleTargetTypeToSerial(virDomainChrConsoleTargetType type) +{ + switch (type) { + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP: + return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLP; + + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM: + return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLPLM; + + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST: + /* Return the default at the end, to please strict compilers. */ + break; + } + + return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE; +} + + static int virDomainDefAddConsoleCompat(virDomainDefPtr def, - unsigned int parseFlags ATTRIBUTE_UNUSED) + unsigned int parseFlags) { size_t i; @@ -3985,6 +4011,10 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def, * * We then fill def->consoles[0] with a stub just so we get sequencing * correct for consoles > 0 + * + * sclp/sclplm consoles (in s390 and s390x guests) are converted to serial + * only when we can update the ABI of the guest, to avoid breaking + * migrations to old libvirt. */ /* Only the first console (if there are any) can be of type serial, @@ -4001,7 +4031,10 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def, } if (def->nconsoles > 0 && def->os.type == VIR_DOMAIN_OSTYPE_HVM && (def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL || - def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE)) { + def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE || + ((def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP || + def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM) && + (parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE)))) { /* If there isn't a corresponding serial port: * - create one and set, the console to be an alias for it @@ -4021,7 +4054,7 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def, /* modify it to be a serial port */ def->serials[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; - def->serials[0]->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE; + def->serials[0]->targetType = virDomainChrConsoleTargetTypeToSerial(def->serials[0]->targetType); def->serials[0]->target.port = 0; } else { /* if the console source doesn't match */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.args b/tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.args new file mode 100644 index 000000000..20968f794 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.args @@ -0,0 +1,22 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-M s390-ccw-virtio \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-boot c \ +-chardev pty,id=charserial0 \ +-device sclpconsole,chardev=charserial0,id=serial0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.xml b/tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.xml new file mode 100644 index 000000000..5f02ec8a0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.xml @@ -0,0 +1,19 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch="s390x" machine="s390-ccw-virtio">hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <!-- Since this test is run with the flag VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, + then it will result in a <serial> element created for it, and thus + a serial chardev for QEMU. --> + <console type='pty'> + <target type='sclp'/> + </console> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a280a8a85..6cc7731a7 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2061,6 +2061,12 @@ mymain(void) QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390, QEMU_CAPS_SCLP_S390); + DO_TEST_FULL("s390-console2serial", NULL, -1, 0, + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, GIC_NONE, + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_VIRTIO_S390, + QEMU_CAPS_SCLP_S390); DO_TEST("ppc-dtb", QEMU_CAPS_KVM, -- 2.13.6

On Wed, 2017-11-15 at 12:50 +0100, Andrea Bolognani wrote:
From: Pino Toscano <ptoscano@redhat.com>
In case we are allowed to break the ABI of a s390/s390x guest, then convert the first sclp/sclplm console from <console> to <serial>, just like it is done on other architectures.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/conf/domain_conf.c | 39 ++++++++++++++++++++-- .../qemuxml2argv-s390-console2serial.args | 22 ++++++++++++ .../qemuxml2argv-s390-console2serial.xml | 19 +++++++++++ tests/qemuxml2argvtest.c | 6 ++++ 4 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.xml
Assuming the series gets ACKed up until here, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

Andrea Bolognani <abologna@redhat.com> [2017-11-15, 12:50PM +0100]:
From: Pino Toscano <ptoscano@redhat.com>
In case we are allowed to break the ABI of a s390/s390x guest, then convert the first sclp/sclplm console from <console> to <serial>, just like it is done on other architectures.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/conf/domain_conf.c | 39 ++++++++++++++++++++-- .../qemuxml2argv-s390-console2serial.args | 22 ++++++++++++ .../qemuxml2argv-s390-console2serial.xml | 19 +++++++++++ tests/qemuxml2argvtest.c | 6 ++++ 4 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9a3cdc777..14b611602 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3966,9 +3966,35 @@ virDomainDefPostParseMemory(virDomainDefPtr def, }
+static virDomainChrSerialTargetType +virDomainChrConsoleTargetTypeToSerial(virDomainChrConsoleTargetType type) +{ + switch (type) { + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP: + return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLP; + + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM: + return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLPLM; + + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST: + /* Return the default at the end, to please strict compilers. */ + break; + } + + return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE; +} + + static int virDomainDefAddConsoleCompat(virDomainDefPtr def, - unsigned int parseFlags ATTRIBUTE_UNUSED) + unsigned int parseFlags) { size_t i;
@@ -3985,6 +4011,10 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def, * * We then fill def->consoles[0] with a stub just so we get sequencing * correct for consoles > 0 + * + * sclp/sclplm consoles (in s390 and s390x guests) are converted to serial + * only when we can update the ABI of the guest, to avoid breaking + * migrations to old libvirt. */
/* Only the first console (if there are any) can be of type serial, @@ -4001,7 +4031,10 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def, } if (def->nconsoles > 0 && def->os.type == VIR_DOMAIN_OSTYPE_HVM && (def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL || - def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE)) { + def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE || + ((def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP || + def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM) && + (parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE)))) {
This gets a bit hard to parse, maybe we can extract the conditional?
/* If there isn't a corresponding serial port: * - create one and set, the console to be an alias for it @@ -4021,7 +4054,7 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def,
/* modify it to be a serial port */ def->serials[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; - def->serials[0]->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE; + def->serials[0]->targetType = virDomainChrConsoleTargetTypeToSerial(def->serials[0]->targetType); def->serials[0]->target.port = 0; } else { /* if the console source doesn't match */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.args b/tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.args new file mode 100644 index 000000000..20968f794 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.args @@ -0,0 +1,22 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-M s390-ccw-virtio \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-boot c \ +-chardev pty,id=charserial0 \ +-device sclpconsole,chardev=charserial0,id=serial0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.xml b/tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.xml new file mode 100644 index 000000000..5f02ec8a0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-console2serial.xml @@ -0,0 +1,19 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch="s390x" machine="s390-ccw-virtio">hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <!-- Since this test is run with the flag VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, + then it will result in a <serial> element created for it, and thus + a serial chardev for QEMU. --> + <console type='pty'> + <target type='sclp'/> + </console> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a280a8a85..6cc7731a7 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2061,6 +2061,12 @@ mymain(void) QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390, QEMU_CAPS_SCLP_S390); + DO_TEST_FULL("s390-console2serial", NULL, -1, 0, + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, GIC_NONE, + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_VIRTIO_S390, + QEMU_CAPS_SCLP_S390);
DO_TEST("ppc-dtb", QEMU_CAPS_KVM, -- 2.13.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Rest looks fine and initial smoke testing was all good. Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> -- IBM Systems Linux on z Systems & Virtualization Development ------------------------------------------------------------------------ IBM Deutschland Schönaicher Str. 220 71032 Böblingen Phone: +49 7031 16 1819 E-Mail: bwalk@de.ibm.com ------------------------------------------------------------------------ IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

From: Pino Toscano <ptoscano@redhat.com> Now that <serial> and <console> on s390/s390x behave a bit more like the other architectures, remove this extra differentation, and use sclp console by default for new guests. New virtio consoles can still be added, and it is actually needed because of the limited number of instances for sclp and sclplm. This reverts commit b1c88c14764e0b043a269d454a83a6ac7af34eac, whose reasons are not totally clear. Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/qemu/qemu_domain.c | 7 ------- tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args | 5 +---- tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml | 6 ++++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml | 6 ------ 4 files changed, 5 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a6467ba4b..08467dc7e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4068,13 +4068,6 @@ static int qemuDomainChrDefPostParse(virDomainChrDefPtr chr, const virDomainDef *def) { - /* set the default console type for S390 arches */ - if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && - chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE && - ARCH_IS_S390(def->os.arch)) { - chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; - } - /* Historically, isa-serial and the default matched, so in order to * maintain backwards compatibility we map them here. The actual default * will be picked below based on the architecture and machine type */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args index c405fb59e..20968f794 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args @@ -18,8 +18,5 @@ QEMU_AUDIO_DRV=none \ server,nowait \ -mon chardev=charmonitor,id=monitor,mode=readline \ -boot c \ --device virtio-serial-ccw,id=virtio-serial0,devno=fe.0.0000 \ -chardev pty,id=charserial0 \ --device sclpconsole,chardev=charserial0,id=serial0 \ --chardev pty,id=charconsole1 \ --device virtconsole,chardev=charconsole1,id=console1 +-device sclpconsole,chardev=charserial0,id=serial0 diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml index 7eb1a765a..931e255c5 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml @@ -14,9 +14,11 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-s390x</emulator> - <controller type='virtio-serial' index='0'/> + <serial type='pty'> + <target type='sclpconsole' port='0'/> + </serial> <console type='pty'> - <target type='virtio' port='0'/> + <target type='serial' port='0'/> </console> <memballoon model='none'/> <panic model='s390'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml index f3364fa23..201156445 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml @@ -14,18 +14,12 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-s390x</emulator> - <controller type='virtio-serial' index='0'> - <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> - </controller> <serial type='pty'> <target type='sclpconsole' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> </console> - <console type='pty'> - <target type='virtio' port='0'/> - </console> <memballoon model='none'/> <panic model='s390'/> </devices> -- 2.13.6

On Wed, 2017-11-15 at 12:50 +0100, Andrea Bolognani wrote:
From: Pino Toscano <ptoscano@redhat.com>
Now that <serial> and <console> on s390/s390x behave a bit more like the other architectures, remove this extra differentation, and use sclp console by default for new guests. New virtio consoles can still be added, and it is actually needed because of the limited number of instances for sclp and sclplm.
This reverts commit b1c88c14764e0b043a269d454a83a6ac7af34eac, whose reasons are not totally clear.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/qemu/qemu_domain.c | 7 ------- tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args | 5 +---- tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml | 6 ++++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml | 6 ------ 4 files changed, 5 insertions(+), 19 deletions(-)
Assuming the series gets ACKed up until here, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

Andrea Bolognani <abologna@redhat.com> [2017-11-15, 12:50PM +0100]:
From: Pino Toscano <ptoscano@redhat.com>
Now that <serial> and <console> on s390/s390x behave a bit more like the other architectures, remove this extra differentation, and use sclp console by default for new guests. New virtio consoles can still be added, and it is actually needed because of the limited number of instances for sclp and sclplm.
This reverts commit b1c88c14764e0b043a269d454a83a6ac7af34eac, whose reasons are not totally clear.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/qemu/qemu_domain.c | 7 ------- tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args | 5 +---- tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml | 6 ++++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml | 6 ------ 4 files changed, 5 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a6467ba4b..08467dc7e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4068,13 +4068,6 @@ static int qemuDomainChrDefPostParse(virDomainChrDefPtr chr, const virDomainDef *def) { - /* set the default console type for S390 arches */ - if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && - chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE && - ARCH_IS_S390(def->os.arch)) { - chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; - } - /* Historically, isa-serial and the default matched, so in order to * maintain backwards compatibility we map them here. The actual default * will be picked below based on the architecture and machine type */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args index c405fb59e..20968f794 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-serial-console.args @@ -18,8 +18,5 @@ QEMU_AUDIO_DRV=none \ server,nowait \ -mon chardev=charmonitor,id=monitor,mode=readline \ -boot c \ --device virtio-serial-ccw,id=virtio-serial0,devno=fe.0.0000 \ -chardev pty,id=charserial0 \ --device sclpconsole,chardev=charserial0,id=serial0 \ --chardev pty,id=charconsole1 \ --device virtconsole,chardev=charconsole1,id=console1 +-device sclpconsole,chardev=charserial0,id=serial0 diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml index 7eb1a765a..931e255c5 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml @@ -14,9 +14,11 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-s390x</emulator> - <controller type='virtio-serial' index='0'/> + <serial type='pty'> + <target type='sclpconsole' port='0'/> + </serial> <console type='pty'> - <target type='virtio' port='0'/> + <target type='serial' port='0'/> </console> <memballoon model='none'/> <panic model='s390'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml index f3364fa23..201156445 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-serial-console.xml @@ -14,18 +14,12 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-s390x</emulator> - <controller type='virtio-serial' index='0'> - <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> - </controller> <serial type='pty'> <target type='sclpconsole' port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> </console> - <console type='pty'> - <target type='virtio' port='0'/> - </console> <memballoon model='none'/> <panic model='s390'/> </devices> -- 2.13.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Looks good to me. Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> -- IBM Systems Linux on z Systems & Virtualization Development ------------------------------------------------------------------------ IBM Deutschland Schönaicher Str. 220 71032 Böblingen Phone: +49 7031 16 1819 E-Mail: bwalk@de.ibm.com ------------------------------------------------------------------------ IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/news.xml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index efde4c8b7..7d2b20ecf 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -124,6 +124,18 @@ can prepare the files if they need to do so. </description> </change> + <change> + <summary> + Improve serial console behavior on non-x86 architectures + </summary> + <description> + ppc64, aarch64 and s390x guests were treating the <serial> + and <console> elements differently from x86, in some cases + presenting misleading information to the user. The behavior is now + consistent across all architectures and the information reported + is always accurate. + </description> + </change> </section> <section title="Bug fixes"> <change> -- 2.13.6

Our current documentation is missing some information and doesn't do a great job at explaining how the <serial> and <console> elements are connected. Let's try to fix that. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/formatdomain.html.in | 212 +++++++++++++++++++++++++++++++++------------- 1 file changed, 152 insertions(+), 60 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7c819d8bf..4630743c8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6514,6 +6514,7 @@ qemu-kvm -net nic,model=? /dev/null <pre> ... <devices> + <!-- Serial port --> <serial type='pty'> <source path='/dev/pts/3'/> <target port='0'/> @@ -6521,98 +6522,189 @@ qemu-kvm -net nic,model=? /dev/null </devices> ...</pre> +<pre> +... +<devices> + <!-- USB serial port --> + <serial type='pty'> + <target type='usb' port='0'/> + <address type='usb' bus='0' port='1'/> + </serial> +</devices> +...</pre> + <p> - <code>target</code> can have a <code>port</code> attribute, which - specifies the port number. Ports are numbered starting from 0. There are - usually 0, 1 or 2 serial ports. There is also an optional - <code>type</code> attribute <span class="since">since 1.0.2</span> - which can be used to pick between <code>isa-serial</code>, - <code>usb-serial</code>, <code>pci-serial</code> and, - <span class="since">since 3.10.0</span>, <code>spapr-vty</code>, + The <code>target</code> target element can have an optional + <code>port</code> attribute, which specifies the port number (starting + from 0), and an optional <code>type</code> attribute which can be used + to pick the guest-visible device: values available ever + <span class="since">since 1.0.2</span> are <code>isa-serial</code>, + <code>usb-serial</code> and <code>pci-serial</code>; + <span class="since">since 3.10.0</span> <code>spapr-vty</code>, <code>pl011</code>, <code>sclpconsole</code> and - <code>sclplmconsole</code>. - Some values are not compatible with all architecture and machine types; - if the value is missing altogether, libvirt will try to pick an - appropriate default. - For <code>usb-serial</code> an optional sub-element - <code><address/></code> with <code>type='usb'</code> can tie the - device to a particular controller, <a href="#elementsAddress">documented above</a>. - Similarly, <code>pci-serial</code> can be used to attach the device to - the pci bus (<span class="since">since 1.2.16</span>). Again, it has - optional sub-element <code><address/></code> with - <code>type='pci'</code> to select desired location on the PCI bus. + <code>sclplmconsole</code> can be used as well. Some values are not + compatible with all architecture and machine types, and if the value is + missing altogether, libvirt will try to pick an appropriate default. + Some of the types support configuring the guest-visible device + address as <a href="#elementsAddress">documented above</a>. + </p> + + <p> + For the relationship between serial ports and consoles, + <a href="#elementCharSerialAndConsole">see below</a>. </p> <h6><a id="elementCharConsole">Console</a></h6> +<pre> +... +<devices> + <!-- Serial console --> + <console type='pty'> + <source path='/dev/pts/2'/> + <target type='serial' port='0'/> + </console> +</devices> +...</pre> + +<pre> +... + <!-- KVM virtio console --> + <console type='pty'> + <source path='/dev/pts/5'/> + <target type='virtio' port='0'/> + </console> +</devices> +...</pre> + + <p> + The <code>console</code> element is used to represent interactive + serial consoles. Depending on the type of guest in use and the specifics + of the configuration, the <code>console</code> element might represent + the same device as an existing <code>serial</code> element or a separate + device. + </p> + + <p> + A <code>target</code> subelement is supported and works the same way + as with the <code>serial</code> element + (<a href="#elementCharSerial">see above</a> for details); valid values + for the <code>type</code> attribute are <code>serial</code>, + <code>virtio</code>, <code>xen</code>, <code>lxc</code>, + <code>uml</code> and <code>openvz</code>. The <code>sclp</code> and + <code>sclplm</code> values are supported for compatibility reasons but + should not be used for new guests: use the <code>sclpconsole</code> + and <code>sclplmconsole</code> values, respectively, with the + <code>serial</code> element instead. + </p> + + <p> + Of the target types listed above, <code>serial</code> is special in + that it doesn't represents a separate device, but rather the same + device as the first <code>serial</code> element. Due to this, there can + only be a single <code>console</code> element with target type + <code>serial</code> per guest. + </p> + + <p> + Virtio consoles are usually accessible as <code>/dev/hvc[0-7]</code> + from inside the guest; for more information, see + <a href="http://fedoraproject.org/wiki/Features/VirtioSerial">http://fedoraproject.org/wiki/Features/VirtioSerial</a>. + <span class="since">Since 0.8.3</span> + </p> + + <p> + For the relationship between serial ports and consoles, + <a href="#elementCharSerialAndConsole">see below</a>. + </p> + + <h6><a id="elementCharSerialAndConsole">Relationship between serial ports and consoles</a></h6> + + <p> + Due to hystorical reasons, the <code>serial</code> and + <code>console</code> elements have partially overlapping scopes. + </p> + + <p> + In general, both elements are used to configure one or more serial + consoles to be used for interacting with the guest. The main difference + between the two is that <code>serial</code> is used for emulated, + usually native, serial consoles, whereas <code>console</code> is used + for paravirtualized ones. + </p> + <p> - The console element is used to represent interactive consoles. Depending - on the type of guest in use, the consoles might be paravirtualized devices, - or they might be a clone of a serial device, according to the following - rules: + Both emulated and paravirtualized serial consoles have advantages and + disadvantages: </p> <ul> - <li>If no <code>targetType</code> attribute is set, then the default - device type is according to the hypervisor's rules. The default - type will be added when re-querying the XML fed into libvirt. - For fully virtualized guests, the default device type will usually - be a serial port.</li> - <li>If the <code>targetType</code> attribute is <code>serial</code>, - then if no <code><serial></code> element exists, the console - element will be copied to the serial element. If a <code><serial></code> - element does already exist, the console element will be ignored.</li> - <li>If the <code>targetType</code> attribute is not <code>serial</code>, - it will be treated normally.</li> - <li>Only the first <code>console</code> element may use a <code>targetType</code> - of <code>serial</code>. Secondary consoles must all be paravirtualized. + <li> + emulated serial consoles are usually initialized much earlier than + paravirtualized ones, so they can be used to control the bootloader + and display both firmware and early boot messages; </li> - <li>On S390, the <code>console</code> element may use a - <code>targetType</code> of <code>sclp</code> or <code>sclplm</code> - (line mode). SCLP is the native console type for S390. There's no - controller associated to SCLP consoles. - <span class="since">Since 1.0.2</span> + <li> + on several platforms, there can only be a single emulated serial + console per guest but paravirtualized consoles don't suffer from the + same limitation. </li> </ul> <p> - A virtio console device is exposed in the - guest as /dev/hvc[0-7] (for more information, see - <a href="http://fedoraproject.org/wiki/Features/VirtioSerial">http://fedoraproject.org/wiki/Features/VirtioSerial</a>) - <span class="since">Since 0.8.3</span> + A configuration such as: </p> <pre> ... -<devices> +</devices> <console type='pty'> - <source path='/dev/pts/4'/> - <target port='0'/> + <target type='serial'/> </console> - - <!-- KVM virtio console --> <console type='pty'> - <source path='/dev/pts/5'/> - <target type='virtio' port='0'/> + <target type='virtio'/> </console> </devices> ...</pre> + <p> + will work on any platform and will result in one emulated serial console + for early boot logging / interactive / recovery use, and one + paravirtualized serial console to be used eg. as a side channel. Most + people will be fine with having just the first <code>console</code> + element in their configuration. + </p> + + <p> + Note that, due to the compatibility concerns mentioned earlier, all the + following configurations: + </p> + <pre> ... -<devices> - <!-- KVM S390 sclp console --> - <console type='pty'> - <source path='/dev/pts/1'/> - <target type='sclp' port='0'/> - </console> +</devices> + <serial type='pty'/> +</devices> +...</pre> + +<pre> +... +</devices> + <console type='pty'/> +</devices> +...</pre> + +<pre> +... +</devices> + <serial type='pty'/> + <console type='pty'/> </devices> ...</pre> <p> - If the console is presented as a serial port, the <code>target</code> - element has the same attributes as for a serial port. There is usually - only 1 console. + will be treated the same and will result in a single emulated serial + console being available to the guest. </p> <h6><a id="elementCharChannel">Channel</a></h6> -- 2.13.6
participants (5)
-
Andrea Bolognani
-
Bjoern Walk
-
Marc Hartmayer
-
Pavel Hrdina
-
Pino Toscano