[libvirt] [PATCH 07/14] conf: Add VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR

Andrea Bolognani posted 14 patches 7 years, 6 months ago
There is a newer version of this series
[libvirt] [PATCH 07/14] conf: Add VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR
Posted by Andrea Bolognani 7 years, 6 months ago
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>&lt;address/&gt;</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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/14] conf: Add VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR
Posted by Pavel Hrdina 7 years, 5 months ago
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>&lt;address/&gt;</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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/14] conf: Add VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR
Posted by Andrea Bolognani 7 years, 5 months ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/14] conf: Add VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR
Posted by Pavel Hrdina 7 years, 5 months ago
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/14] conf: Add VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR
Posted by Marc Hartmayer 7 years, 5 months ago
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


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/14] conf: Add VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR
Posted by Andrea Bolognani 7 years, 5 months ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list