[libvirt] [PATCH 0/2] qemu: support SPICE+unix socket
qemu has supported SPICE+unix socket listening since 2.3.0. Patch #1 wires it up from domain_conf.c down to qemu, patch #2 adds qemu.conf spice_auto_unix_socket option Full disclosure: this duplicates the logic of VNC socket usage, which has some pre-existing warts: - When the VM is running with graphics socket=FOO, the runtime XML still lists <listen type='address' address='127.0.0.1'/>. - If using vnc/spice auto socket, it looks like ports can still be allocated in the code, though they are never used. - In fact I suspect there's lots of issues with stray XML port/listen being parsed and ending up in the device config, but selectively being ignored later. We should probably explicitly ignore/error about those combos when parsing the XML so we never have to ignore specific values later on. - The auto socket path is set in qemu_command.c but it should probably be set in qemu_process.c with the port stuff. Though there are references to vncListen in qemu_command.c which should probably be moved as well. The issue with moving them to qemu_process.c is now there isn't any pre-existing way to test those bits like via qemuxml2argvtest for example. Though finding a way to test the graphics prep stuff would be valuable anyways - teuf suggests: converting <graphics socket=X/> to <graphics><listen type='socket' path='FOO'/></graphics> like was done with listen= address - tuef: virsh domdisplay doesn't handle sockets To avoid yet another rabbit hole I didn't touch any of the above issues, which basically means spice+unix is no less operationally worse than vnc+unix :) Cole Robinson (2): qemu: Support SPICE listen over unix socket qemu: Add qemu.conf option spice_auto_unix_socket docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 26 +++++++++++++-------- src/conf/domain_conf.h | 1 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 11 +++++++++ src/qemu/qemu_command.c | 15 +++++++++--- src/qemu/qemu_conf.c | 1 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_process.c | 3 +++ src/qemu/test_libvirtd_qemu.aug.in | 1 + src/security/virt-aa-helper.c | 5 ++++ .../qemuxml2argv-graphics-spice-unix-auto.args | 21 +++++++++++++++++ .../qemuxml2argv-graphics-spice-unix-auto.xml | 27 ++++++++++++++++++++++ .../qemuxml2argv-graphics-spice-unix.args | 21 +++++++++++++++++ .../qemuxml2argv-graphics-spice-unix.xml | 27 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 10 ++++++++ 16 files changed, 163 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix-auto.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix-auto.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml -- 2.5.0
Add support for SPICE listen over unix socket. This has been in qemu since v2.3. The XML is: <spice socket='/path/to/socket'/> Which matches support for VNC listen over unix socket. https://bugzilla.redhat.com/show_bug.cgi?id=1151761 --- docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 26 +++++++++++++-------- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 4 +++- src/qemu/qemu_process.c | 3 +++ src/security/virt-aa-helper.c | 5 ++++ .../qemuxml2argv-graphics-spice-unix.args | 21 +++++++++++++++++ .../qemuxml2argv-graphics-spice-unix.xml | 27 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++++ 9 files changed, 85 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index da6de40..4d3f951 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2700,6 +2700,11 @@ </attribute> </optional> <optional> + <attribute name="socket"> + <ref name="absFilePath"/> + </attribute> + </optional> + <optional> <attribute name="passwdValidTo"> <data type="dateTime"/> </attribute> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d5d9ff7..985d8bd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1249,6 +1249,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + VIR_FREE(def->data.spice.socket); VIR_FREE(def->data.spice.keymap); virDomainGraphicsAuthDefClear(&def->data.spice.auth); break; @@ -10986,6 +10987,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, def->data.spice.tlsPort = 0; } + def->data.spice.socket = virXMLPropString(node, "socket"); def->data.spice.keymap = virXMLPropString(node, "keymap"); if (virDomainGraphicsAuthDefParseXML(node, &def->data.spice.auth, @@ -21267,19 +21269,23 @@ virDomainGraphicsDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - if (def->data.spice.port) - virBufferAsprintf(buf, " port='%d'", - def->data.spice.port); + if (def->data.spice.socket) { + virBufferEscapeString(buf, " socket='%s'", def->data.spice.socket); + } else { + if (def->data.spice.port) + virBufferAsprintf(buf, " port='%d'", + def->data.spice.port); - if (def->data.spice.tlsPort) - virBufferAsprintf(buf, " tlsPort='%d'", - def->data.spice.tlsPort); + if (def->data.spice.tlsPort) + virBufferAsprintf(buf, " tlsPort='%d'", + def->data.spice.tlsPort); - virBufferAsprintf(buf, " autoport='%s'", - def->data.spice.autoport ? "yes" : "no"); + virBufferAsprintf(buf, " autoport='%s'", + def->data.spice.autoport ? "yes" : "no"); - if (listenAddr) - virBufferAsprintf(buf, " listen='%s'", listenAddr); + if (listenAddr) + virBufferAsprintf(buf, " listen='%s'", listenAddr); + } if (def->data.spice.keymap) virBufferEscapeString(buf, " keymap='%s'", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 83bdd67..884476d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1578,6 +1578,7 @@ struct _virDomainGraphicsDef { bool tlsPortReserved; int mousemode; char *keymap; + char *socket; virDomainGraphicsAuthDef auth; bool autoport; int channels[VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST]; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index eb02553..8a5baf5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7411,7 +7411,9 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, /* TODO: Support ACLs later */ } - if (port > 0 || tlsPort > 0) { + if (graphics->data.spice.socket) { + virBufferAsprintf(&opt, "unix,addr=%s,", graphics->data.spice.socket); + } else if (port > 0 || tlsPort > 0) { switch (virDomainGraphicsListenGetType(graphics, 0)) { case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c332747..6cf993b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3759,6 +3759,9 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, bool needTLSPort = false; bool needPort = false; + if (graphics->data.spice.socket) + return 0; + if (graphics->data.spice.autoport) { /* check if tlsPort or port need allocation */ for (i = 0; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST; i++) { diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index a2d7226..f46742c 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1064,6 +1064,11 @@ get_files(vahControl * ctl) ctl->def->graphics[i]->data.vnc.socket && vah_add_file(&buf, ctl->def->graphics[i]->data.vnc.socket, "rw")) goto cleanup; + + if (ctl->def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + ctl->def->graphics[i]->data.spice.socket && + vah_add_file(&buf, ctl->def->graphics[i]->data.spice.socket, "rw")) + goto cleanup; } if (ctl->def->ngraphics == 1 && diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.args new file mode 100644 index 0000000..b965ea4 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.args @@ -0,0 +1,21 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=spice \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-spice unix,addr=/tmp/spice.socket \ +-vga qxl \ +-global qxl-vga.ram_size=67108864 \ +-global qxl-vga.vram_size=33554432 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml new file mode 100644 index 0000000..6c6be44 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml @@ -0,0 +1,27 @@ +<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'>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> + <controller type='usb' model='none' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' socket='/tmp/spice.socket'/> + <video> + <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4fac77d..76b64bd 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -990,6 +990,10 @@ mymain(void) QEMU_CAPS_DEVICE_QXL_VGA, QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_SPICE_FILE_XFER_DISABLE); + DO_TEST("graphics-spice-unix", + QEMU_CAPS_VGA_QXL, + QEMU_CAPS_SPICE, + QEMU_CAPS_DEVICE_QXL); DO_TEST("input-usbmouse", NONE); DO_TEST("input-usbtablet", NONE); -- 2.5.0
On Mon, Mar 21, 2016 at 07:30:44PM -0400, Cole Robinson wrote:
Add support for SPICE listen over unix socket. This has been in qemu since v2.3. The XML is:
<spice socket='/path/to/socket'/>
Which matches support for VNC listen over unix socket.
https://bugzilla.redhat.com/show_bug.cgi?id=1151761 --- docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 26 +++++++++++++-------- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 4 +++- src/qemu/qemu_process.c | 3 +++ src/security/virt-aa-helper.c | 5 ++++ .../qemuxml2argv-graphics-spice-unix.args | 21 +++++++++++++++++ .../qemuxml2argv-graphics-spice-unix.xml | 27 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++++ 9 files changed, 85 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index da6de40..4d3f951 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2700,6 +2700,11 @@ </attribute> </optional> <optional> + <attribute name="socket"> + <ref name="absFilePath"/> + </attribute> + </optional> + <optional> <attribute name="passwdValidTo"> <data type="dateTime"/> </attribute> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d5d9ff7..985d8bd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1249,6 +1249,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) break;
case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + VIR_FREE(def->data.spice.socket); VIR_FREE(def->data.spice.keymap); virDomainGraphicsAuthDefClear(&def->data.spice.auth); break; @@ -10986,6 +10987,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, def->data.spice.tlsPort = 0; }
+ def->data.spice.socket = virXMLPropString(node, "socket"); def->data.spice.keymap = virXMLPropString(node, "keymap");
if (virDomainGraphicsAuthDefParseXML(node, &def->data.spice.auth, @@ -21267,19 +21269,23 @@ virDomainGraphicsDefFormat(virBufferPtr buf, break;
case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - if (def->data.spice.port) - virBufferAsprintf(buf, " port='%d'", - def->data.spice.port); + if (def->data.spice.socket) { + virBufferEscapeString(buf, " socket='%s'", def->data.spice.socket); + } else { + if (def->data.spice.port) + virBufferAsprintf(buf, " port='%d'", + def->data.spice.port);
- if (def->data.spice.tlsPort) - virBufferAsprintf(buf, " tlsPort='%d'", - def->data.spice.tlsPort); + if (def->data.spice.tlsPort) + virBufferAsprintf(buf, " tlsPort='%d'", + def->data.spice.tlsPort);
- virBufferAsprintf(buf, " autoport='%s'", - def->data.spice.autoport ? "yes" : "no"); + virBufferAsprintf(buf, " autoport='%s'", + def->data.spice.autoport ? "yes" : "no");
- if (listenAddr) - virBufferAsprintf(buf, " listen='%s'", listenAddr); + if (listenAddr) + virBufferAsprintf(buf, " listen='%s'", listenAddr); + }
if (def->data.spice.keymap) virBufferEscapeString(buf, " keymap='%s'", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 83bdd67..884476d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1578,6 +1578,7 @@ struct _virDomainGraphicsDef { bool tlsPortReserved; int mousemode; char *keymap; + char *socket; virDomainGraphicsAuthDef auth; bool autoport; int channels[VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST]; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index eb02553..8a5baf5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7411,7 +7411,9 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, /* TODO: Support ACLs later */ }
- if (port > 0 || tlsPort > 0) { + if (graphics->data.spice.socket) { + virBufferAsprintf(&opt, "unix,addr=%s,", graphics->data.spice.socket); + } else if (port > 0 || tlsPort > 0) { switch (virDomainGraphicsListenGetType(graphics, 0)) { case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c332747..6cf993b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3759,6 +3759,9 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, bool needTLSPort = false; bool needPort = false;
+ if (graphics->data.spice.socket) + return 0; + if (graphics->data.spice.autoport) { /* check if tlsPort or port need allocation */ for (i = 0; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST; i++) { diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index a2d7226..f46742c 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1064,6 +1064,11 @@ get_files(vahControl * ctl) ctl->def->graphics[i]->data.vnc.socket && vah_add_file(&buf, ctl->def->graphics[i]->data.vnc.socket, "rw")) goto cleanup; + + if (ctl->def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + ctl->def->graphics[i]->data.spice.socket && + vah_add_file(&buf, ctl->def->graphics[i]->data.spice.socket, "rw")) + goto cleanup; }
if (ctl->def->ngraphics == 1 && diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.args new file mode 100644 index 0000000..b965ea4 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.args @@ -0,0 +1,21 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=spice \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-spice unix,addr=/tmp/spice.socket \ +-vga qxl \ +-global qxl-vga.ram_size=67108864 \ +-global qxl-vga.vram_size=33554432 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml new file mode 100644 index 0000000..6c6be44 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml @@ -0,0 +1,27 @@ +<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'>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> + <controller type='usb' model='none' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' socket='/tmp/spice.socket'/>
This is an old way to specify listen type. It would be better to add a new <listen type='socket' socket='/tmp/spice.socket'/>. Actually I'm working on this support and I have my patches almost finished but they depends on this patch series: https://www.redhat.com/archives/libvir-list/2016-March/msg00631.html You can see my progress there: https://github.com/Antique/libvirt/tree/spice-unix-socket
+ <video> + <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4fac77d..76b64bd 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -990,6 +990,10 @@ mymain(void) QEMU_CAPS_DEVICE_QXL_VGA, QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_SPICE_FILE_XFER_DISABLE); + DO_TEST("graphics-spice-unix", + QEMU_CAPS_VGA_QXL, + QEMU_CAPS_SPICE, + QEMU_CAPS_DEVICE_QXL);
DO_TEST("input-usbmouse", NONE); DO_TEST("input-usbtablet", NONE); -- 2.5.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hey, On Tue, Mar 22, 2016 at 09:13:44AM +0100, Pavel Hrdina wrote:
On Mon, Mar 21, 2016 at 07:30:44PM -0400, Cole Robinson wrote:
Add support for SPICE listen over unix socket. This has been in qemu since v2.3. The XML is:
<spice socket='/path/to/socket'/>
Which matches support for VNC listen over unix socket.
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml new file mode 100644 index 0000000..6c6be44 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml @@ -0,0 +1,27 @@ +<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'>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> + <controller type='usb' model='none' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' socket='/tmp/spice.socket'/>
This is an old way to specify listen type. It would be better to add a new <listen type='socket' socket='/tmp/spice.socket'/>. Actually I'm working on this support and I have my patches almost finished but they depends on this patch series:
https://www.redhat.com/archives/libvir-list/2016-March/msg00631.html
You can see my progress there:
We briefly discussed about this with Cole on IRC, and what you suggest is indeed the way forward. His patch is a quick way to get feature-parity with what VNC does, and we were not aware of your series. You seem to be adding <listen type='socket' socket='/tmp/foo.sock'/> I think <listen type='unix' path='/tmp/foo.sock'/> would be more consistent with what is done for channels, /interface/source, ... Christophe
On Tue, Mar 22, 2016 at 09:49:34AM +0100, Christophe Fergeau wrote:
Hey,
On Tue, Mar 22, 2016 at 09:13:44AM +0100, Pavel Hrdina wrote:
On Mon, Mar 21, 2016 at 07:30:44PM -0400, Cole Robinson wrote:
Add support for SPICE listen over unix socket. This has been in qemu since v2.3. The XML is:
<spice socket='/path/to/socket'/>
Which matches support for VNC listen over unix socket.
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml new file mode 100644 index 0000000..6c6be44 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml @@ -0,0 +1,27 @@ +<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'>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> + <controller type='usb' model='none' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' socket='/tmp/spice.socket'/>
This is an old way to specify listen type. It would be better to add a new <listen type='socket' socket='/tmp/spice.socket'/>. Actually I'm working on this support and I have my patches almost finished but they depends on this patch series:
https://www.redhat.com/archives/libvir-list/2016-March/msg00631.html
You can see my progress there:
We briefly discussed about this with Cole on IRC, and what you suggest is indeed the way forward. His patch is a quick way to get feature-parity with what VNC does, and we were not aware of your series.
You seem to be adding <listen type='socket' socket='/tmp/foo.sock'/>
I think <listen type='unix' path='/tmp/foo.sock'/> would be more consistent with what is done for channels, /interface/source, ...
Christophe
I know, Cole's patch is an easy and quick way but we will have to deal with it for the rest of libvirt's lifetime. That's why I've ended with rather larger series that cleanup a lot of staff and introduce new listen type instead. I'm waiting for review of my qemu-process cleanups and after that I'll post this series to list. Pavel
On 03/22/2016 04:59 AM, Pavel Hrdina wrote:
On Tue, Mar 22, 2016 at 09:49:34AM +0100, Christophe Fergeau wrote:
Hey,
On Tue, Mar 22, 2016 at 09:13:44AM +0100, Pavel Hrdina wrote:
On Mon, Mar 21, 2016 at 07:30:44PM -0400, Cole Robinson wrote:
Add support for SPICE listen over unix socket. This has been in qemu since v2.3. The XML is:
<spice socket='/path/to/socket'/>
Which matches support for VNC listen over unix socket.
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml new file mode 100644 index 0000000..6c6be44 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml @@ -0,0 +1,27 @@ +<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'>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> + <controller type='usb' model='none' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' socket='/tmp/spice.socket'/>
This is an old way to specify listen type. It would be better to add a new <listen type='socket' socket='/tmp/spice.socket'/>. Actually I'm working on this support and I have my patches almost finished but they depends on this patch series:
https://www.redhat.com/archives/libvir-list/2016-March/msg00631.html
You can see my progress there:
We briefly discussed about this with Cole on IRC, and what you suggest is indeed the way forward. His patch is a quick way to get feature-parity with what VNC does, and we were not aware of your series.
You seem to be adding <listen type='socket' socket='/tmp/foo.sock'/>
I think <listen type='unix' path='/tmp/foo.sock'/> would be more consistent with what is done for channels, /interface/source, ...
Agreed, that seems more consistent to me.
I know, Cole's patch is an easy and quick way but we will have to deal with it for the rest of libvirt's lifetime. That's why I've ended with rather larger series that cleanup a lot of staff and introduce new listen type instead.
Right, but we already have to deal with that <graphics socket=.../> format for VNC... not adding that to SPICE is also inconsistent. Just pointing it out, doesn't matter to me much either way - Cole
On Tue, Mar 22, 2016 at 07:52:12AM -0400, Cole Robinson wrote:
On 03/22/2016 04:59 AM, Pavel Hrdina wrote:
On Tue, Mar 22, 2016 at 09:49:34AM +0100, Christophe Fergeau wrote:
Hey,
On Tue, Mar 22, 2016 at 09:13:44AM +0100, Pavel Hrdina wrote:
On Mon, Mar 21, 2016 at 07:30:44PM -0400, Cole Robinson wrote:
Add support for SPICE listen over unix socket. This has been in qemu since v2.3. The XML is:
<spice socket='/path/to/socket'/>
Which matches support for VNC listen over unix socket.
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml new file mode 100644 index 0000000..6c6be44 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml @@ -0,0 +1,27 @@ +<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'>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> + <controller type='usb' model='none' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' socket='/tmp/spice.socket'/>
This is an old way to specify listen type. It would be better to add a new <listen type='socket' socket='/tmp/spice.socket'/>. Actually I'm working on this support and I have my patches almost finished but they depends on this patch series:
https://www.redhat.com/archives/libvir-list/2016-March/msg00631.html
You can see my progress there:
We briefly discussed about this with Cole on IRC, and what you suggest is indeed the way forward. His patch is a quick way to get feature-parity with what VNC does, and we were not aware of your series.
You seem to be adding <listen type='socket' socket='/tmp/foo.sock'/>
I think <listen type='unix' path='/tmp/foo.sock'/> would be more consistent with what is done for channels, /interface/source, ...
Agreed, that seems more consistent to me.
I know, Cole's patch is an easy and quick way but we will have to deal with it for the rest of libvirt's lifetime. That's why I've ended with rather larger series that cleanup a lot of staff and introduce new listen type instead.
Right, but we already have to deal with that <graphics socket=.../> format for VNC... not adding that to SPICE is also inconsistent. Just pointing it out, doesn't matter to me much either way
That's a good point actually but I would rather have this inconsistency rather than duplicating this information. We do the same for <listen type='address'/> but for backward compatibility and the same rule would apply to the VNC case just to by compatible with old applications. This is a new design and a new feature and I would like to avoid to having <graphics socket=.../> for SPICE. Pavel
On 03/22/2016 04:13 AM, Pavel Hrdina wrote:
On Mon, Mar 21, 2016 at 07:30:44PM -0400, Cole Robinson wrote:
Add support for SPICE listen over unix socket. This has been in qemu since v2.3. The XML is:
<spice socket='/path/to/socket'/>
Which matches support for VNC listen over unix socket.
+++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml @@ -0,0 +1,27 @@ +<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'>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> + <controller type='usb' model='none' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' socket='/tmp/spice.socket'/>
This is an old way to specify listen type. It would be better to add a new <listen type='socket' socket='/tmp/spice.socket'/>. Actually I'm working on this support and I have my patches almost finished but they depends on this patch series:
https://www.redhat.com/archives/libvir-list/2016-March/msg00631.html
I'll give that a review
You can see my progress there:
Okay I'll defer to your patches. Check my cover letter for more ideas if you're motivated, I didn't check to see if your patches fixed all those issues. Thanks, Cole
On Tue, Mar 22, 2016 at 07:44:33AM -0400, Cole Robinson wrote:
On 03/22/2016 04:13 AM, Pavel Hrdina wrote:
On Mon, Mar 21, 2016 at 07:30:44PM -0400, Cole Robinson wrote:
Add support for SPICE listen over unix socket. This has been in qemu since v2.3. The XML is:
<spice socket='/path/to/socket'/>
Which matches support for VNC listen over unix socket.
+++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix.xml @@ -0,0 +1,27 @@ +<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'>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> + <controller type='usb' model='none' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' socket='/tmp/spice.socket'/>
This is an old way to specify listen type. It would be better to add a new <listen type='socket' socket='/tmp/spice.socket'/>. Actually I'm working on this support and I have my patches almost finished but they depends on this patch series:
https://www.redhat.com/archives/libvir-list/2016-March/msg00631.html
I'll give that a review
You can see my progress there:
Okay I'll defer to your patches. Check my cover letter for more ideas if you're motivated, I didn't check to see if your patches fixed all those issues.
Nice summary, I'll check all the things and include the missing ones into the series. Thanks, Pavel
Similar to vnc_auto_unix_socket, this option tells libvirt to allocate a listening socket path for default <graphics type='spice'/> config, taking precedence over spice_listen. https://bugzilla.redhat.com/show_bug.cgi?id=1043919 --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 11 +++++++++ src/qemu/qemu_command.c | 13 ++++++++--- src/qemu/qemu_conf.c | 1 + src/qemu/qemu_conf.h | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + .../qemuxml2argv-graphics-spice-unix-auto.args | 21 +++++++++++++++++ .../qemuxml2argv-graphics-spice-unix-auto.xml | 27 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 6 +++++ 9 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix-auto.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix-auto.xml diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index b6f6dc4..49d59ad 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -40,6 +40,7 @@ module Libvirtd_qemu = | str_entry "spice_password" | bool_entry "spice_sasl" | str_entry "spice_sasl_dir" + | bool_entry "spice_auto_unix_socket" let nogfx_entry = bool_entry "nographics_allow_host_audio" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 4fa5e8a..cd0a614 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -156,6 +156,17 @@ #spice_sasl_dir = "/some/directory/sasl2" +# Enable this option to have SPICE served over an automatically created +# unix socket. This prevents unprivileged access from users on the +# host machine. +# +# This will only be enabled for SPICE configurations that do not have +# a hardcoded 'listen' or 'socket' value. This setting takes preference +# over spice_listen. +# +#spice_auto_unix_socket = 1 + + # By default, if no graphical front end is configured, libvirt will disable # QEMU audio output since directly talking to alsa/pulseaudio may not work # with various security settings. If you know what you're doing, enable diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8a5baf5..d2bfae6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7370,7 +7370,8 @@ static int qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, virCommandPtr cmd, virQEMUCapsPtr qemuCaps, - virDomainGraphicsDefPtr graphics) + virDomainGraphicsDefPtr graphics, + const char *domainLibDir) { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *listenNetwork; @@ -7411,7 +7412,12 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, /* TODO: Support ACLs later */ } - if (graphics->data.spice.socket) { + if (graphics->data.spice.socket || cfg->spiceAutoUnixSocket) { + if (!graphics->data.spice.socket && + virAsprintf(&graphics->data.spice.socket, + "%s/spice.sock", domainLibDir) == -1) + goto error; + virBufferAsprintf(&opt, "unix,addr=%s,", graphics->data.spice.socket); } else if (port > 0 || tlsPort > 0) { switch (virDomainGraphicsListenGetType(graphics, 0)) { @@ -7653,7 +7659,8 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, graphics, domainLibDir); case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - return qemuBuildGraphicsSPICECommandLine(cfg, cmd, qemuCaps, graphics); + return qemuBuildGraphicsSPICECommandLine(cfg, cmd, qemuCaps, + graphics, domainLibDir); case VIR_DOMAIN_GRAPHICS_TYPE_RDP: case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 77ef4fe..ac9e275 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -587,6 +587,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, GET_VALUE_STR("spice_sasl_dir", cfg->spiceSASLdir); GET_VALUE_STR("spice_listen", cfg->spiceListen); GET_VALUE_STR("spice_password", cfg->spicePassword); + GET_VALUE_BOOL("spice_auto_unix_socket", cfg->spiceAutoUnixSocket); GET_VALUE_ULONG("remote_websocket_port_min", cfg->webSocketPortMin); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a714b84..c94bf13 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -123,6 +123,7 @@ struct _virQEMUDriverConfig { char *spiceSASLdir; char *spiceListen; char *spicePassword; + bool spiceAutoUnixSocket; int remotePortMin; int remotePortMax; diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 8bec743..d09ecd3 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -17,6 +17,7 @@ module Test_libvirtd_qemu = { "spice_password" = "XYZ12345" } { "spice_sasl" = "1" } { "spice_sasl_dir" = "/some/directory/sasl2" } +{ "spice_auto_unix_socket" = "1" } { "nographics_allow_host_audio" = "1" } { "remote_display_port_min" = "5900" } { "remote_display_port_max" = "65535" } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix-auto.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix-auto.args new file mode 100644 index 0000000..7f54855 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix-auto.args @@ -0,0 +1,21 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=spice \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-spice unix,addr=/tmp/lib/domain--1-QEMUGuest1/spice.sock \ +-vga qxl \ +-global qxl-vga.ram_size=67108864 \ +-global qxl-vga.vram_size=33554432 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix-auto.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix-auto.xml new file mode 100644 index 0000000..7378be8 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-unix-auto.xml @@ -0,0 +1,27 @@ +<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'>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> + <controller type='usb' model='none' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice'/> + <video> + <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 76b64bd..15dde3d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -994,6 +994,12 @@ mymain(void) QEMU_CAPS_VGA_QXL, QEMU_CAPS_SPICE, QEMU_CAPS_DEVICE_QXL); + driver.config->spiceAutoUnixSocket = true; + DO_TEST("graphics-spice-unix-auto", + QEMU_CAPS_VGA_QXL, + QEMU_CAPS_SPICE, + QEMU_CAPS_DEVICE_QXL); + driver.config->spiceAutoUnixSocket = false; DO_TEST("input-usbmouse", NONE); DO_TEST("input-usbtablet", NONE); -- 2.5.0
participants (3)
-
Christophe Fergeau -
Cole Robinson -
Pavel Hrdina