Re: [libvirt] [PATCH 2/2] Add QEMU support for virtio channel

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.

> + > + 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);

> + } > + > + 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; > } > } >