On 18/02/10 14:11, Daniel P. Berrange wrote:
> On Thu, Feb 18, 2010 at 01:23:57PM +0000, Matthew Booth wrote:
>> On 18/02/10 12:39, Daniel P. Berrange wrote:
>>> On Wed, Feb 17, 2010 at 05:11:01PM +0000, Matthew Booth wrote:
>>>> Support virtio-serial controller and virtio channel in QEMU backend. Will output
>>>> the following for virtio-serial controller:
>>>>
>>>> -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4,max_ports=16,vectors=4
>>>>
>>>> and the following for a virtio channel:
>>>>
>>>> -chardev pty,id=channel0 \
>>>> -device virtserialport,bus=virtio-serial0.0,chardev=channel0,name=org.linux-kvm.port.0
>>>>
>>>> * src/qemu/qemu_conf.c: Add argument output for virtio
>>>> * tests/qemuxml2argvtest.c
>>>> tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args
>>>> : Add test for QEMU command line generation
>>>> ---
>>>> src/qemu/qemu_conf.c | 91 +++++++++++++++++++-
>>>> .../qemuxml2argv-channel-virtio.args | 1 +
>>>> tests/qemuxml2argvtest.c | 1 +
>>>> 3 files changed, 92 insertions(+), 1 deletions(-)
>>>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args
>>>>
>>>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>>>> index 456ee34..110409d 100644
>>>> --- a/src/qemu/qemu_conf.c
>>>> +++ b/src/qemu/qemu_conf.c
>>>> @@ -2168,7 +2168,6 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs)
>>>> }
>>>> for (i = 0; i < def->nchannels ; i++) {
>>>> /* Nada - none are PCI based (yet) */
>>>> - /* XXX virtio-serial will need one */
>>>> }
>>>> if (def->watchdog &&
>>>> def->watchdog->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
>>>> @@ -2465,6 +2464,23 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr def)
>>>> virBufferVSprintf(&buf, ",id=scsi%d", def->idx);
>>>> break;
>>>>
>>>> + case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
>>>> + if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
>>>> + virBufferAddLit(&buf, "virtio-serial-pci");
>>>> + } else {
>>>> + virBufferAddLit(&buf, "virtio-serial");
>>>> + }
>>>> + virBufferVSprintf(&buf, ",id=virtio-serial%d", def->idx);
>>>> + if (def->opts.vioserial.max_ports != -1) {
>>>> + virBufferVSprintf(&buf, ",max_ports=%d",
>>>> + def->opts.vioserial.max_ports);
>>>> + }
>>>> + if (def->opts.vioserial.vectors != -1) {
>>>> + virBufferVSprintf(&buf, ",vectors=%d",
>>>> + def->opts.vioserial.vectors);
>>>> + }
>>>> + break;
>>>> +
>>>> /* We always get an IDE controller, whether we want it or not. */
>>>> case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
>>>> default:
>>>> @@ -3830,6 +3846,79 @@ int qemudBuildCommandLine(virConnectPtr conn,
>>>> }
>>>> VIR_FREE(addr);
>>>> ADD_ARG(devstr);
>>>> + break;
>>>> +
>>>> + case VIR_DOMAIN_CHR_TARGET_TYPE_VIRTIO:
>>>> + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
>>>> + qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
>>>> + _("virtio channel requires QEMU to support -device"));
>>>> + goto error;
>>>> + }
>>>> +
>>>> + ADD_ARG_LIT("-chardev");
>>>> + if (!(devstr = qemuBuildChrChardevStr(channel)))
>>>> + goto error;
>>>> + ADD_ARG(devstr);
>>>
>>> It would be desirable to put the stuff that follows this point, into
>>> a qemuBuildVirtioSerialPortDevStr() method, because the main
>>> qemudBuildCommandLine() method is far too large & unwieldly these days.
>>
>> Would this comment still stand if the code below was simplified to
>> hard-code alias?
>
> Yep, because you'll want this method to exist if you ever add the
> hot-plug support, since that uses the exact same syntax as the
> main -device arg these days. Which is nice :-)
>
>>
>>>> +
>>>> + virBuffer virtiodev = VIR_BUFFER_INITIALIZER;
>>>> + virBufferAddLit(&virtiodev, "virtserialport");
>>>> +
>>>> + if (channel->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
>>>> + /* Check it's a virtio-serial address */
>>>> + if (channel->info.type !=
>>>> + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL)
>>>> + {
>>>> + qemuReportError(VIR_ERR_INTERNAL_ERROR,
>>>> + _("virtio serial device has invalid "
>>>> + "address type"));
>>>> + virBufferFreeAndReset(&virtiodev);
>>>> + goto error;
>>>> + }
>>>> +
>>>> + /* Look for the virtio-serial controller */
>>>> + const char *vsalias = NULL;
>>>> + int vs = 0;
>>>> + int c;
>>>> + for (c = 0; c < def->ncontrollers; c++) {
>>>> + if (def->controllers[c]->type !=
>>>> + VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL)
>>>> + {
>>>> + continue;
>>>> + }
>>>> +
>>>> + if (vs == channel->info.addr.vioserial.controller) {
>>>> + vsalias = def->controllers[c]->info.alias;
>>>> + break;
>>>> + }
>>>> +
>>>> + vs++;
>>>> + }
>>>> +
>>>> + if (!vsalias) {
>>>> + qemuReportError(VIR_ERR_INTERNAL_ERROR,
>>>> + _("virtio serial device address controller "
>>>> + "does not exist"));
>>>> + virBufferFreeAndReset(&virtiodev);
>>>> + goto error;
>>>> + }
>>>
>>> We don't really need this loop / check there, since the XML parser routine
>>> has already guarenteed that we have a controller associated with the port.
>>> Just
>>>
>>>> +
>>>> + virBufferVSprintf(&virtiodev, ",bus=%s.%d",
>>>> + vsalias, channel->info.addr.vioserial.bus);
>>>
>>> This can just be
>>>
>>> virBufferVSprintf(&virtiodev, ",bus=virtio-serial%d.%d",
>>> channel->info.addr.vioserial.controller,
>>> channel->info.addr.vioserial.bus);
>>
>> Actually, this code is really to look for the controller's alias. Is it
>> safe to hard-code it?
>
> Yep, we do for disks. If you want though, you could add more #defines to
> src/qemu/qemu_conf.h for that, alongside the existing
>
> #define QEMU_DRIVE_HOST_PREFIX "drive-"
>
> eg,
>
> #define QEMU_VIRTIO_SERIAL_PREFIX "virtio-serial"
>
> and reference that constant in both places
>
>>
>>>> + }
>>>> +
>>>> + virBufferVSprintf(&virtiodev, ",chardev=%s", channel->info.alias);
>>>> + if (channel->target.name) {
>>>> + virBufferVSprintf(&virtiodev, ",name=%s", channel->target.name);
>>>> + }
>>>> + if (virBufferError(&virtiodev)) {
>>>> + virReportOOMError();
>>>> + goto error;
>>>> + }
>>>> +
>>>> + ADD_ARG_LIT("-device");
>>>> + ADD_ARG(virBufferContentAndReset(&virtiodev));
>>>> +
>>>> + break;
>>>> }
>>>> }
>>>>
>
Update patch attached.
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
>From 9602171c257f038758fb624c0018c469c8c7f5e0 Mon Sep 17 00:00:00 2001
From: Matthew Booth <mbooth@xxxxxxxxxx>
Date: Thu, 12 Nov 2009 17:47:15 +0000
Subject: [PATCH 2/2] Add QEMU support for virtio channel
Support virtio-serial controller and virtio channel in QEMU backend. Will output
the following for virtio-serial controller:
-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4,max_ports=16,vectors=4
and the following for a virtio channel:
-chardev pty,id=channel0 \
-device virtserialport,bus=virtio-serial0.0,chardev=channel0,name=org.linux-kvm.port.0
* src/qemu/qemu_conf.c: Add argument output for virtio
* tests/qemuxml2argvtest.c
tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args
: Add test for QEMU command line generation
---
src/qemu/qemu_conf.c | 77 +++++++++++++++++++-
src/qemu/qemu_conf.h | 3 +
.../qemuxml2argv-channel-virtio.args | 1 +
tests/qemuxml2argvtest.c | 1 +
4 files changed, 81 insertions(+), 1 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 456ee34..7e96102 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -2168,7 +2168,6 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs)
}
for (i = 0; i < def->nchannels ; i++) {
/* Nada - none are PCI based (yet) */
- /* XXX virtio-serial will need one */
}
if (def->watchdog &&
def->watchdog->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
@@ -2465,6 +2464,24 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr def)
virBufferVSprintf(&buf, ",id=scsi%d", def->idx);
break;
+ case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
+ if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+ virBufferAddLit(&buf, "virtio-serial-pci");
+ } else {
+ virBufferAddLit(&buf, "virtio-serial");
+ }
+ virBufferVSprintf(&buf, ",id=" QEMU_VIRTIO_SERIAL_PREFIX "%d",
+ def->idx);
+ if (def->opts.vioserial.ports != -1) {
+ virBufferVSprintf(&buf, ",max_ports=%d",
+ def->opts.vioserial.ports);
+ }
+ if (def->opts.vioserial.vectors != -1) {
+ virBufferVSprintf(&buf, ",vectors=%d",
+ def->opts.vioserial.vectors);
+ }
+ break;
+
/* We always get an IDE controller, whether we want it or not. */
case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
default:
@@ -2969,6 +2986,45 @@ error:
}
+char *
+qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ virBufferAddLit(&buf, "virtserialport");
+
+ if (dev->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ /* Check it's a virtio-serial address */
+ if (dev->info.type !=
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL)
+ {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ _("virtio serial device has invalid address type"));
+ goto error;
+ }
+
+ virBufferVSprintf(&buf,
+ ",bus=" QEMU_VIRTIO_SERIAL_PREFIX "%d.%d",
+ dev->info.addr.vioserial.controller,
+ dev->info.addr.vioserial.bus);
+ }
+
+ virBufferVSprintf(&buf, ",chardev=%s", dev->info.alias);
+ if (dev->target.name) {
+ virBufferVSprintf(&buf, ",name=%s", dev->target.name);
+ }
+ if (virBufferError(&buf)) {
+ virReportOOMError();
+ goto error;
+ }
+
+ return virBufferContentAndReset(&buf);
+
+error:
+ virBufferFreeAndReset(&buf);
+ return NULL;
+}
+
+
static int
qemuBuildCpuArgStr(const struct qemud_driver *driver,
const virDomainDefPtr def,
@@ -3830,6 +3886,25 @@ int qemudBuildCommandLine(virConnectPtr conn,
}
VIR_FREE(addr);
ADD_ARG(devstr);
+ break;
+
+ case VIR_DOMAIN_CHR_TARGET_TYPE_VIRTIO:
+ if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
+ qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+ _("virtio channel requires QEMU to support -device"));
+ goto error;
+ }
+
+ ADD_ARG_LIT("-chardev");
+ if (!(devstr = qemuBuildChrChardevStr(channel)))
+ goto error;
+ ADD_ARG(devstr);
+
+ ADD_ARG_LIT("-device");
+ if (!(devstr = qemuBuildVirtioSerialPortDevStr(channel)))
+ goto error;
+ ADD_ARG(devstr);
+ break;
}
}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 7041489..4f35a9c 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -158,6 +158,7 @@ typedef qemuDomainPCIAddressSet *qemuDomainPCIAddressSetPtr;
#define QEMU_CONFIG_FORMAT_ARGV "qemu-argv"
#define QEMU_DRIVE_HOST_PREFIX "drive-"
+#define QEMU_VIRTIO_SERIAL_PREFIX "virtio-serial"
#define qemuReportError(code, fmt...) \
virReportErrorHelper(NULL, VIR_FROM_QEMU, code, __FILE__, \
@@ -234,6 +235,8 @@ char * qemuBuildChrChardevStr(virDomainChrDefPtr dev);
/* Legacy, pre device support */
char * qemuBuildChrArgStr(virDomainChrDefPtr dev, const char *prefix);
+char * qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev);
+
/* Legacy, pre device support */
char * qemuBuildUSBHostdevUsbDevStr(virDomainHostdevDefPtr dev);
/* Current, best practice */
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args
new file mode 100644
index 0000000..4097065
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args
@@ -0,0 +1 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device virtio-serial-pci,id=virtio-serial0,max_ports=16,vectors=4,bus=pci.0,addr=0x4 -device virtio-serial-pci,id=virtio-serial1,bus=pci.0,addr=0xa -hda /dev/HostVG/QEMUGuest1 -chardev pty,id=channel0 -device virtserialport,chardev=channel0,name=org.linux-kvm.port.0 -chardev pty,id=channel1 -device virtserialport,bus=virtio-serial1.0,chardev=channel1,name=org.linux-kvm.port.1 -usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index aa42f99..a8c9ed3 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -318,6 +318,7 @@ mymain(int argc, char **argv)
DO_TEST("console-compat-chardev", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE);
DO_TEST("channel-guestfwd", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE);
+ DO_TEST("channel-virtio", QEMUD_CMD_FLAG_DEVICE);
DO_TEST("watchdog", 0);
DO_TEST("watchdog-device", QEMUD_CMD_FLAG_DEVICE);
--
1.6.6
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list