https://bugzilla.redhat.com/show_bug.cgi?id=998813 Like usb-serial, the pci-serial device allows a serial device to be attached to PCI bus. An example XML looks like this: <serial type='dev'> <source path='/dev/ttyS2'/> <target type='pci-serial' port='0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </serial> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- docs/formatdomain.html.in | 16 ++++++---- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 + .../qemuxml2argv-pci-serial-dev-chardev.xml | 37 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 6 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pci-serial-dev-chardev.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e0b6ba7..b61798a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5107,12 +5107,16 @@ 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 two choices for its value, one is <code>isa-serial</code>, - the other is <code>usb-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 <code>&lt;address&gt;</code> with - <code>type='usb'</code> can tie the device to a particular controller, - <a href="#elementsAddress">documented above</a>. + 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 + <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 + the pci bus (<span class="since">since 1.2.16</span>). Again, it has + optional sub-element <code>&lt;address/&gt;</code> with + <code>type='pci'</code> to select desired location on the PCI bus. </p> <h6><a name="elementCharConsole">Console</a></h6> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c151e92..f3cb37a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3055,6 +3055,7 @@ <choice> <value>isa-serial</value> <value>usb-serial</value> + <value>pci-serial</value> </choice> </attribute> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index add857c..103d4d4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -407,7 +407,8 @@ VIR_ENUM_IMPL(virDomainChrDeviceState, VIR_DOMAIN_CHR_DEVICE_STATE_LAST, VIR_ENUM_IMPL(virDomainChrSerialTarget, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST, "isa-serial", - "usb-serial") + "usb-serial", + "pci-serial") VIR_ENUM_IMPL(virDomainChrChannelTarget, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2cd105a7..fa36e59 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1076,6 +1076,7 @@ typedef enum { typedef enum { VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA = 0, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB, + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI, VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST } virDomainChrSerialTargetType; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pci-serial-dev-chardev.xml b/tests/qemuxml2argvdata/qemuxml2argv-pci-serial-dev-chardev.xml new file mode 100644 index 0000000..a058e38 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pci-serial-dev-chardev.xml @@ -0,0 +1,37 @@ +<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='i686' machine='pc'>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</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <serial type='dev'> + <source path='/dev/ttyS2'/> + <target type='pci-serial' port='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </serial> + <console type='dev'> + <source path='/dev/ttyS2'/> + <target type='serial' port='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </console> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 2c53d7c..53bcc9f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -481,6 +481,7 @@ mymain(void) DO_TEST("hostdev-pci-address"); DO_TEST("hostdev-vfio"); DO_TEST("pci-rom"); + DO_TEST("pci-serial-dev-chardev"); DO_TEST("encrypted-disk"); DO_TEST_DIFFERENT("memtune"); -- 2.3.6

https://bugzilla.redhat.com/show_bug.cgi?id=998813 Implementation is pretty straight-forward. Of course, not all qemus out there supports the device, so new capability is introduced and checked prior each use of the device. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 30 +++++++++++++++++++++- tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + .../qemuxml2argv-pci-serial-dev-chardev.args | 7 +++++ tests/qemuxml2argvtest.c | 3 +++ 11 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pci-serial-dev-chardev.args diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 25c15bf..d7bb443 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -281,6 +281,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "pc-dimm", "machine-vmport-opt", /* 185 */ + "pci-serial", ); @@ -1537,6 +1538,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "iothread", QEMU_CAPS_OBJECT_IOTHREAD}, { "ivshmem", QEMU_CAPS_DEVICE_IVSHMEM }, { "pc-dimm", QEMU_CAPS_DEVICE_PC_DIMM }, + { "pci-serial", QEMU_CAPS_DEVICE_PCI_SERIAL }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 81557b7..a2edf82 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -225,6 +225,7 @@ typedef enum { QEMU_CAPS_QXL_VGA_VGAMEM = 183, /* -device qxl-vga.vgamem_mb */ QEMU_CAPS_DEVICE_PC_DIMM = 184, /* pc-dimm device */ QEMU_CAPS_MACHINE_VMPORT_OPT = 185, /* -machine xxx,vmport=on/off/auto */ + QEMU_CAPS_DEVICE_PCI_SERIAL = 186, /* -device pci-serial */ 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 5d0a167..2fe01bb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2281,6 +2281,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, * - VirtIO balloon * - Host device passthrough * - Watchdog (not IB700) + * - pci serial devices * * Prior to this function being invoked, qemuCollectPCIAddress() will have * added all existing PCI addresses from the 'def' to 'addrs'. Thus this @@ -2553,7 +2554,16 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, /* Nada - none are PCI based (yet) */ } for (i = 0; i < def->nserials; i++) { - /* Nada - none are PCI based (yet) */ + virDomainChrDefPtr chr = def->serials[i]; + + if (chr->targetType != VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) + continue; + + if (chr->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + continue; + + if (virDomainPCIAddressReserveNextSlot(addrs, &chr->info, flags) < 0) + goto error; } for (i = 0; i < def->nchannels; i++) { /* Nada - none are PCI based (yet) */ @@ -10855,6 +10865,24 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, 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; + } + + 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; + } + + if (qemuBuildDeviceAddressStr(&cmd, def, &serial->info, qemuCaps) < 0) + goto error; + break; } } diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps index 68bed9f..ea3d850 100644 --- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps @@ -134,4 +134,5 @@ <flag name='vmware-svga.vgamem_mb'/> <flag name='qxl.vgamem_mb'/> <flag name='qxl-vga.vgamem_mb'/> + <flag name='pci-serial'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps index baf2e77..2c19ddc 100644 --- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps @@ -135,4 +135,5 @@ <flag name='vmware-svga.vgamem_mb'/> <flag name='qxl.vgamem_mb'/> <flag name='qxl-vga.vgamem_mb'/> + <flag name='pci-serial'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index 496f305..aadccd5 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -144,4 +144,5 @@ <flag name='vmware-svga.vgamem_mb'/> <flag name='qxl.vgamem_mb'/> <flag name='qxl-vga.vgamem_mb'/> + <flag name='pci-serial'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 38333a6..3e81cbf 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -150,4 +150,5 @@ <flag name='vmware-svga.vgamem_mb'/> <flag name='qxl.vgamem_mb'/> <flag name='qxl-vga.vgamem_mb'/> + <flag name='pci-serial'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index b093e08..84c357f 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -150,4 +150,5 @@ <flag name='vmware-svga.vgamem_mb'/> <flag name='qxl.vgamem_mb'/> <flag name='qxl-vga.vgamem_mb'/> + <flag name='pci-serial'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps index 5637edb..b1ee8df 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps @@ -166,4 +166,5 @@ <flag name='qxl.vgamem_mb'/> <flag name='qxl-vga.vgamem_mb'/> <flag name='pc-dimm'/> + <flag name='pci-serial'/> </qemuCaps> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pci-serial-dev-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-pci-serial-dev-chardev.args new file mode 100644 index 0000000..36cb067 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pci-serial-dev-chardev.args @@ -0,0 +1,7 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -usb \ +-hda /dev/HostVG/QEMUGuest1 -chardev tty,id=charserial0,path=/dev/ttyS2 \ +-device pci-serial,chardev=charserial0,id=serial0,bus=pci.0,addr=0x4 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e67d909..e70cd17 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1060,6 +1060,9 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("console-compat-chardev", QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("pci-serial-dev-chardev", + QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_PCI_SERIAL); DO_TEST("channel-guestfwd", QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); -- 2.3.6

Not every chardev is plugged onto virtio-serial bus. However, the code introduced in 89e991a2aa36b04 assumes that. Incorrectly. With previous patches we have three options where a chardev can be plugged: virtio-serial, USB and PCI. This commit fixes the attach part. However, since we are not auto allocating USB addresses yet, I'm just marking the place where appropriate code should go. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index fc45de1..a7fac98 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1556,11 +1556,18 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) allowZero = true; - if (virDomainVirtioSerialAddrAutoAssign(NULL, - priv->vioserialaddrs, - &chr->info, - allowZero) < 0) - goto cleanup; + if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) { + if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &chr->info) < 0) + goto cleanup; + } else if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) { + /* XXX */ + } else { + if (virDomainVirtioSerialAddrAutoAssign(NULL, + priv->vioserialaddrs, + &chr->info, + allowZero) < 0) + goto cleanup; + } need_release = true; if (qemuBuildChrDeviceStr(&devstr, vm->def, chr, priv->qemuCaps) < 0) @@ -1594,8 +1601,15 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, cleanup: if (ret < 0 && virDomainObjIsActive(vm)) qemuDomainChrInsertPreAllocCleanup(vm->def, chr); - if (ret < 0 && need_release) - virDomainVirtioSerialAddrRelease(priv->vioserialaddrs, &chr->info); + if (ret < 0 && need_release) { + if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) { + qemuDomainReleaseDeviceAddress(vm, &chr->info, NULL); + } else if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) { + /* XXX */ + } else { + virDomainVirtioSerialAddrRelease(priv->vioserialaddrs, &chr->info); + } + } VIR_FREE(charAlias); VIR_FREE(devstr); return ret; -- 2.3.6

Not every chardev is plugged onto virtio-serial bus. However, the code introduced in 89e991a2aa36b04 assumes that. Incorrectly. With previous patches we have three options where a chardev can be plugged: virtio-serial, USB and PCI. This commit fixes the detach part. However, since we are not auto allocating USB addresses yet, I'm just marking the place where appropriate code should go. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a7fac98..6ee4078 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4164,7 +4164,13 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver, rc = qemuDomainWaitForDeviceRemoval(vm); if (rc == 0 || rc == 1) { - virDomainVirtioSerialAddrRelease(priv->vioserialaddrs, &tmpChr->info); + if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) { + qemuDomainReleaseDeviceAddress(vm, &tmpChr->info, NULL); + } else if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) { + /* XXX */ + } else { + virDomainVirtioSerialAddrRelease(priv->vioserialaddrs, &tmpChr->info); + } ret = qemuDomainRemoveChrDevice(driver, vm, tmpChr); } else { ret = 0; -- 2.3.6