[libvirt] [PATCH v2] qemu: introduce spiceport serial backend
Adding a new backend that makes the chardev available to be backed up by a port in spice connection (different to spicevmc). This can be used (as well as other backends) for any chardev libvirt supports. Apart from spicevmc, spiceport-backed chardev will not be formatted into the command-line if there is no spice to use (with test for that as well). For this I moved the def->graphics caounting to the start of the function so its results can be used in rest of the code even in the future. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- Notes: v2: - No longer applies on top of "qemu: minor cleanups", but is now based purely on master - Addressed Christophe's comments docs/formatdomain.html.in | 18 ++++ docs/schemas/domaincommon.rng | 4 + src/conf/domain_audit.c | 3 +- src/conf/domain_conf.c | 39 +++++++- src/conf/domain_conf.h | 6 +- src/qemu/qemu_capabilities.c | 12 ++- src/qemu/qemu_capabilities.h | 3 +- src/qemu/qemu_command.c | 100 ++++++++++++--------- src/qemu/qemu_monitor_json.c | 3 +- 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 + .../qemuxml2argv-serial-spiceport-nospice.args | 6 ++ .../qemuxml2argv-serial-spiceport-nospice.xml | 40 +++++++++ .../qemuxml2argv-serial-spiceport.args | 13 +++ .../qemuxml2argv-serial-spiceport.xml | 43 +++++++++ tests/qemuxml2argvtest.c | 7 ++ tests/qemuxml2xmltest.c | 2 + 18 files changed, 255 insertions(+), 47 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport-nospice.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport-nospice.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fd02864..117f64d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4718,6 +4718,24 @@ qemu-kvm -net nic,model=? /dev/null </devices> ...</pre> + <h6><a name="elementsCharSpiceport">Spice channel</a></h6> + + <p> + The character device is accessible through spice connection + under a channel name specified in the <code>channel</code> + attribute. <span class="since">Since 1.2.2</span> + </p> + +<pre> + ... + <devices> + <serial type="spiceport"> + <source channel="org.qemu.console.serial.0"/> + <target port="1"/> + </serial> + </devices> + ...</pre> + <h4><a name="elementsSound">Sound devices</a></h4> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7f55f24..3063d5a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2876,6 +2876,7 @@ <value>vc</value> <value>pty</value> <value>spicevmc</value> + <value>spiceport</value> </choice> </define> @@ -2946,6 +2947,9 @@ <attribute name="wiremode"/> </optional> <optional> + <attribute name="channel"/> + </optional> + <optional> <ref name='devSeclabel'/> </optional> </element> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 11cf5c8..b6564c2 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -1,7 +1,7 @@ /* * domain_audit.c: Domain audit management * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -81,6 +81,7 @@ virDomainAuditChardevPath(virDomainChrSourceDefPtr chr) case VIR_DOMAIN_CHR_TYPE_VC: case VIR_DOMAIN_CHR_TYPE_STDIO: case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: case VIR_DOMAIN_CHR_TYPE_LAST: return NULL; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 512fe51..c7f5345 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -437,7 +437,8 @@ VIR_ENUM_IMPL(virDomainChr, VIR_DOMAIN_CHR_TYPE_LAST, "udp", "tcp", "unix", - "spicevmc") + "spicevmc", + "spiceport") VIR_ENUM_IMPL(virDomainChrTcpProtocol, VIR_DOMAIN_CHR_TCP_PROTOCOL_LAST, "raw", @@ -1583,6 +1584,11 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src, STREQ_NULLABLE(src->data.nix.path, tgt->data.nix.path); break; + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + return STREQ_NULLABLE(src->data.spiceport.channel, + tgt->data.spiceport.channel); + break; + case VIR_DOMAIN_CHR_TYPE_VC: case VIR_DOMAIN_CHR_TYPE_STDIO: case VIR_DOMAIN_CHR_TYPE_SPICEVMC: @@ -7084,6 +7090,9 @@ error: return ret; } +#define SERIAL_CHANNEL_NAME_CHARS \ + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-." + /* Parse the source half of the XML definition for a character device, * where node is the first element of node->children of the parent * element. def->type must already be valid. Return -1 on failure, @@ -7104,6 +7113,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *path = NULL; char *mode = NULL; char *protocol = NULL; + char *channel = NULL; int remaining = 0; while (cur != NULL) { @@ -7148,6 +7158,11 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, VIR_FREE(mode); break; + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + if (!channel) + channel = virXMLPropString(cur, "channel"); + break; + case VIR_DOMAIN_CHR_TYPE_LAST: case VIR_DOMAIN_CHR_TYPE_NULL: case VIR_DOMAIN_CHR_TYPE_VC: @@ -7287,6 +7302,21 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, def->data.nix.path = path; path = NULL; break; + + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + if (!channel) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing source channel attribute for char device")); + goto error; + } + if (strspn(channel, SERIAL_CHANNEL_NAME_CHARS) < strlen(channel)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Invalid character in source channel for char device")); + goto error; + } + def->data.spiceport.channel = channel; + channel = NULL; + break; } cleanup: @@ -7297,6 +7327,7 @@ cleanup: VIR_FREE(connectHost); VIR_FREE(connectService); VIR_FREE(path); + VIR_FREE(channel); return remaining; @@ -15629,6 +15660,12 @@ virDomainChrSourceDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " path='%s'", def->data.nix.path); virBufferAddLit(buf, "/>\n"); break; + + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + virBufferAsprintf(buf, " <source channel='%s'/>\n", + def->data.spiceport.channel); + break; + } return 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9acb105..7b213b8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1,7 +1,7 @@ /* * domain_conf.h: domain XML processing * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1104,6 +1104,7 @@ enum virDomainChrType { VIR_DOMAIN_CHR_TYPE_TCP, VIR_DOMAIN_CHR_TYPE_UNIX, VIR_DOMAIN_CHR_TYPE_SPICEVMC, + VIR_DOMAIN_CHR_TYPE_SPICEPORT, VIR_DOMAIN_CHR_TYPE_LAST }; @@ -1152,6 +1153,9 @@ struct _virDomainChrSourceDef { bool listen; } nix; int spicevmc; + struct { + char *channel; + } spiceport; } data; }; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 8420047..51b4e02 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1,7 +1,7 @@ /* * qemu_capabilities.c: QEMU capabilities generation * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -247,7 +247,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "boot-strict", /* 160 */ "pvpanic", "enable-fips", - "spice-file-xfer-disable" + "spice-file-xfer-disable", + "spiceport", ); struct _virQEMUCaps { @@ -1012,6 +1013,8 @@ virQEMUCapsComputeCmdFlags(const char *help, virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV); if (strstr(help, "-chardev spicevmc")) virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEVMC); + if (strstr(help, "-chardev spiceport")) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEPORT); } if (strstr(help, "-balloon")) virQEMUCapsSet(qemuCaps, QEMU_CAPS_BALLOON); @@ -2567,6 +2570,11 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 1003001) virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET); + /* -chardev spiceport is supported from 1.4.0, + * but it's in qapi only since 1.5.0 */ + if (qemuCaps->version >= 1005000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEPORT); + if (qemuCaps->version >= 1006000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 23dccce..a4eecb6 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -1,7 +1,7 @@ /* * qemu_capabilities.h: QEMU capabilities generation * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -202,6 +202,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_PANIC = 161, /* -device pvpanic */ QEMU_CAPS_ENABLE_FIPS = 162, /* -enable-fips */ QEMU_CAPS_SPICE_FILE_XFER_DISABLE = 163, /* -spice disable-agent-file-xfer */ + QEMU_CAPS_CHARDEV_SPICEPORT = 164, /* -chardev spiceport */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6cc32f9..37d38f1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1,7 +1,7 @@ /* * qemu_command.c: QEMU command generation * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -5977,6 +5977,16 @@ qemuBuildChrChardevStr(virDomainChrSourceDefPtr dev, const char *alias, virDomainChrSpicevmcTypeToString(dev->data.spicevmc)); break; + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEPORT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spiceport not supported in this QEMU binary")); + goto error; + } + virBufferAsprintf(&buf, "spiceport,id=char%s,name=%s", alias, + dev->data.spiceport.channel); + break; + default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported chardev '%s'"), @@ -6072,6 +6082,10 @@ qemuBuildChrArgStr(virDomainChrSourceDefPtr dev, const char *prefix) dev->data.nix.path, dev->data.nix.listen ? ",server,nowait" : ""); break; + + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_LAST: + break; } if (virBufferError(&buf)) { @@ -7707,6 +7721,7 @@ qemuBuildCommandLine(virConnectPtr conn, int vnc = 0; int spice = 0; int usbcontroller = 0; + int actualSerials = 0; bool usblegacy = false; bool mlock = false; int contOrder[] = { @@ -7736,6 +7751,20 @@ qemuBuildCommandLine(virConnectPtr conn, emulator = def->emulator; + for (i = 0; i < def->ngraphics; ++i) { + switch (def->graphics[i]->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + ++sdl; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + ++vnc; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + ++spice; + break; + } + } + /* * do not use boot=on for drives when not using KVM since this * is not supported at all in upstream QEmu. @@ -8802,35 +8831,39 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgBuffer(cmd, &opt); } - if (!def->nserials) { - /* If we have -device, then we set -nodefault already */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) - virCommandAddArgList(cmd, "-serial", "none", NULL); - } else { - for (i = 0; i < def->nserials; i++) { - virDomainChrDefPtr serial = def->serials[i]; - char *devstr; + for (i = 0; i < def->nserials; i++) { + virDomainChrDefPtr serial = def->serials[i]; + char *devstr; - /* Use -chardev with -device if they are available */ - if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) { - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&serial->source, - serial->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); + if (serial->source.type == VIR_DOMAIN_CHR_TYPE_SPICEPORT && !spice) + continue; - if (qemuBuildChrDeviceCommandLine(cmd, def, serial, qemuCaps) < 0) - goto error; - } else { - virCommandAddArg(cmd, "-serial"); - if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } + /* Use -chardev with -device if they are available */ + if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) { + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&serial->source, + serial->info.alias, + qemuCaps))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (qemuBuildChrDeviceCommandLine(cmd, def, serial, qemuCaps) < 0) + goto error; + } else { + virCommandAddArg(cmd, "-serial"); + if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } + actualSerials++; + } + + if (!actualSerials) { + /* If we have -device, then we set -nodefault already */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + virCommandAddArgList(cmd, "-serial", "none", NULL); } if (!def->nparallels) { @@ -9017,19 +9050,6 @@ qemuBuildCommandLine(virConnectPtr conn, } } - for (i = 0; i < def->ngraphics; ++i) { - switch (def->graphics[i]->type) { - case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - ++sdl; - break; - case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - ++vnc; - break; - case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - ++spice; - break; - } - } if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_0_10) && sdl + vnc + spice > 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("only 1 graphics device is supported")); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ec3b958..5e825ac 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1,7 +1,7 @@ /* * qemu_monitor_json.c: interaction with QEMU monitor console * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -5318,6 +5318,7 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID, break; case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: case VIR_DOMAIN_CHR_TYPE_PIPE: case VIR_DOMAIN_CHR_TYPE_STDIO: case VIR_DOMAIN_CHR_TYPE_LAST: diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index 1e5bb74..adf8531 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -134,4 +134,5 @@ <flag name='boot-strict'/> <flag name='pvpanic'/> <flag name='reboot-timeout'/> + <flag name='spiceport'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 61542a8..e6b2f76 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -140,4 +140,5 @@ <flag name='reboot-timeout'/> <flag name='enable-fips'/> <flag name='spice-file-xfer-disable'/> + <flag name='spiceport'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index 8ce17aa..e6b8117 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -138,4 +138,5 @@ <flag name='pvpanic'/> <flag name='reboot-timeout'/> <flag name='spice-file-xfer-disable'/> + <flag name='spiceport'/> </qemuCaps> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport-nospice.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport-nospice.args new file mode 100644 index 0000000..1e27394 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport-nospice.args @@ -0,0 +1,6 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 -S -M pc -m 214 -smp 1 -nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-usb -hda /dev/HostVG/QEMUGuest1 -net none -serial \ +none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport-nospice.xml b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport-nospice.xml new file mode 100644 index 0000000..06a99a3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport-nospice.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <title>A description of the test machine.</title> + <description> + A test of qemu's minimal configuration. + This test also tests the description and title elements. + </description> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static' cpuset='1-4,8-20,525'>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='spiceport'> + <source channel='org.qemu.console.serial.0'/> + <target port='0'/> + </serial> + <console type='spiceport'> + <source channel='org.qemu.console.serial.0'/> + <target type='serial' port='0'/> + </console> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args new file mode 100644 index 0000000..8c631b1 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args @@ -0,0 +1,13 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=spice \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -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 spiceport,id=charserial0,name=org.qemu.console.serial.0 \ +-device isa-serial,chardev=charserial0,id=serial0 \ +-device usb-tablet,id=input0 \ +-spice port=5903,tls-port=5904,addr=127.0.0.1,x509-dir=/etc/pki/libvirt-spice \ +-device \ +qxl-vga,id=video0,ram_size=67107840,vram_size=67107840,bus=pci.0,addr=0x2 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.xml b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.xml new file mode 100644 index 0000000..1e42ee6 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.xml @@ -0,0 +1,43 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static' cpuset='1-4,8-20,525'>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='spiceport'> + <source channel='org.qemu.console.serial.0'/> + <target port='0'/> + </serial> + <console type='spiceport'> + <source channel='org.qemu.console.serial.0'/> + <target type='serial' port='0'/> + </console> + <input type='tablet' bus='usb'/> + <input type='mouse' bus='ps2'/> + <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='qxl' ram='65535' vram='65535' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index dae08d5..ec03aa2 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -947,6 +947,13 @@ mymain(void) DO_TEST("serial-udp", NONE); DO_TEST("serial-tcp-telnet", NONE); DO_TEST("serial-many", NONE); + DO_TEST("serial-spiceport", + QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, + QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_DEVICE_QXL_VGA, + QEMU_CAPS_SPICE, QEMU_CAPS_CHARDEV_SPICEPORT); + DO_TEST("serial-spiceport-nospice", QEMU_CAPS_NAME); + DO_TEST("parallel-tcp", NONE); DO_TEST("console-compat", NONE); DO_TEST("console-compat-auto", NONE); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 41d1904..c57d7af 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -239,6 +239,8 @@ mymain(void) DO_TEST("serial-udp"); DO_TEST("serial-tcp-telnet"); DO_TEST("serial-many"); + DO_TEST("serial-spiceport"); + DO_TEST("serial-spiceport-nospice"); DO_TEST("parallel-tcp"); DO_TEST("console-compat"); DO_TEST("console-virtio-many"); -- 1.8.5.4
On 02/07/2014 04:37 AM, Martin Kletzander wrote:
Adding a new backend that makes the chardev available to be backed up by a port in spice connection (different to spicevmc). This can be used (as well as other backends) for any chardev libvirt supports.
Apart from spicevmc, spiceport-backed chardev will not be formatted into the command-line if there is no spice to use (with test for that as well). For this I moved the def->graphics caounting to the start
s/caounting/counting/
of the function so its results can be used in rest of the code even in the future.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> ---
--- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c
Hmm, I might have split this into two patches - one for the XML and docs, and one for the qemu implementation.
@@ -1012,6 +1013,8 @@ virQEMUCapsComputeCmdFlags(const char *help, virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV); if (strstr(help, "-chardev spicevmc")) virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEVMC); + if (strstr(help, "-chardev spiceport")) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEPORT);
Why are we parsing -help output? Does qemu < 1.2 support spiceport?
} if (strstr(help, "-balloon")) virQEMUCapsSet(qemuCaps, QEMU_CAPS_BALLOON); @@ -2567,6 +2570,11 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 1003001) virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET);
+ /* -chardev spiceport is supported from 1.4.0,
Especially given this comment, we should NOT parse -help output.
+ * but it's in qapi only since 1.5.0 */ + if (qemuCaps->version >= 1005000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEPORT);
So why are we hard-coding a version number instead of querying qapi? This is all the more reason to split the commit - the XML and doc and src/conf changes are fine, but the qemu side needs work or more comments explaining why we are using version numbers instead of feature probing.
@@ -8802,35 +8831,39 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgBuffer(cmd, &opt); }
- if (!def->nserials) { - /* If we have -device, then we set -nodefault already */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) - virCommandAddArgList(cmd, "-serial", "none", NULL); - } else { - for (i = 0; i < def->nserials; i++) { - virDomainChrDefPtr serial = def->serials[i]; - char *devstr; + for (i = 0; i < def->nserials; i++) { + virDomainChrDefPtr serial = def->serials[i]; + char *devstr;
This hunk was a bit hard to follow; in cases like this, I will sometimes split into two patches - one that changes the logic but leaves indentation off (or adds {}) so that the just the logic change is shown, then the second that fixes indentation. The combined diff is the same, but it is much easier to review the two halves.
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport-nospice.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <title>A description of the test machine.</title> + <description> + A test of qemu's minimal configuration. + This test also tests the description and title elements.
Copied and pasted from another test :) Not a problem, but also not minimal for testing this particular XML. Thanks for the tests. Probably worth a v3, or at least a better explanation of how the qemu capability setting is done, before I feel good with ack. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Feb 07, 2014 at 01:10:42PM -0700, Eric Blake wrote:
On 02/07/2014 04:37 AM, Martin Kletzander wrote:
Adding a new backend that makes the chardev available to be backed up by a port in spice connection (different to spicevmc). This can be used (as well as other backends) for any chardev libvirt supports.
Apart from spicevmc, spiceport-backed chardev will not be formatted into the command-line if there is no spice to use (with test for that as well). For this I moved the def->graphics caounting to the start
s/caounting/counting/
of the function so its results can be used in rest of the code even in the future.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> ---
--- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c
Hmm, I might have split this into two patches - one for the XML and docs, and one for the qemu implementation.
@@ -1012,6 +1013,8 @@ virQEMUCapsComputeCmdFlags(const char *help, virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV); if (strstr(help, "-chardev spicevmc")) virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEVMC); + if (strstr(help, "-chardev spiceport")) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEPORT);
Why are we parsing -help output? Does qemu < 1.2 support spiceport?
Adding the parsing capability doesn't change anything for qemu >= 1.2 or for qemu < 1.2, but if somebody backports spiceport into qemu < 1.2, this will make it work.
} if (strstr(help, "-balloon")) virQEMUCapsSet(qemuCaps, QEMU_CAPS_BALLOON); @@ -2567,6 +2570,11 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 1003001) virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET);
+ /* -chardev spiceport is supported from 1.4.0,
Especially given this comment, we should NOT parse -help output.
+ * but it's in qapi only since 1.5.0 */ + if (qemuCaps->version >= 1005000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEPORT);
So why are we hard-coding a version number instead of querying qapi?
This is all the more reason to split the commit - the XML and doc and src/conf changes are fine, but the qemu side needs work or more comments explaining why we are using version numbers instead of feature probing.
You're right, I should've added more comments about that. The thing is that it is wired into QAPI in a sense that it can be queried as an object in running domain or hot(un)plugged, but it cannot be queried for as a capability (hence my patch for qemu that lists chardev backends.
@@ -8802,35 +8831,39 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgBuffer(cmd, &opt); }
- if (!def->nserials) { - /* If we have -device, then we set -nodefault already */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) - virCommandAddArgList(cmd, "-serial", "none", NULL); - } else { - for (i = 0; i < def->nserials; i++) { - virDomainChrDefPtr serial = def->serials[i]; - char *devstr; + for (i = 0; i < def->nserials; i++) { + virDomainChrDefPtr serial = def->serials[i]; + char *devstr;
This hunk was a bit hard to follow; in cases like this, I will sometimes split into two patches - one that changes the logic but leaves indentation off (or adds {}) so that the just the logic change is shown, then the second that fixes indentation. The combined diff is the same, but it is much easier to review the two halves.
OK, I'll split these into 2.
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport-nospice.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <title>A description of the test machine.</title> + <description> + A test of qemu's minimal configuration. + This test also tests the description and title elements.
Copied and pasted from another test :) Not a problem, but also not minimal for testing this particular XML. Thanks for the tests.
Probably worth a v3, or at least a better explanation of how the qemu capability setting is done, before I feel good with ack.
No problem with v3, at least it'll be cleaner. Thanks for the review. Martin
participants (2)
-
Eric Blake -
Martin Kletzander