[libvirt] [PATCH 2/2] Add QEMU support for virtio channel
Matthew Booth
mbooth at redhat.com
Thu Feb 18 15:29:51 UTC 2010
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
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0002-Add-QEMU-support-for-virtio-channel.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100218/aa3d35d2/attachment-0001.ksh>
More information about the libvir-list
mailing list