[libvirt] [PATCH v4 00/14] improve graphics listen configuration

The first 8 patches are cleanups and refactors of the graphics code. 9th patch introduces listen type socket and updates VNC graphics to use that new listen type. Patches 10 and 11 implements listen type socket for SPICE graphics and patch 12 adds spice_auto_unix_socket config option into qemu.conf. The last two patches introduces and implements new listen type none for VNC and SPICE graphics. The listen type none and socket are required to enable OpenGL support for SPICE graphics. Changes in v4: - removed the port patch that will be posted later as separate patch series - reordered some patches and added some more cleanups Changes in v3: - add listen type none support for vnc - fix issues pointed out in v2 Changes in v2: - don't remove vnc_auto_unix_socket from qemu.conf - add spice_auto_unix_socket Pavel Hrdina (14): qemu_domain: add a empty listen type address if we remove socket for VNC tests: cleanup vnc auto socket test graphics: rename gListen to glisten domain_conf: introduce virDomainGraphicsAddListenAddr qemu_command: move sasl parameter after port and addr definition graphics: resolve address for listen type network in qemu_process qemu_process: separate graphics socket and address generation qemu_command: refactor spice channel code graphics: introduce listen type socket and use it for VNC qemu_capabilites: add QEMU_CAPS_SPICE_UNIX spice: add support for listen type socket spice: introduce spice_auto_unix_socket config option spice: introduce listen type none vnc: add support for listen type none docs/formatdomain.html.in | 28 ++ docs/schemas/domaincommon.rng | 15 + src/conf/domain_conf.c | 314 ++++++++++++++++----- src/conf/domain_conf.h | 8 +- src/libvirt_private.syms | 1 + src/libxl/libxl_conf.c | 16 +- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 17 +- src/qemu/qemu_capabilities.c | 3 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 276 ++++++++---------- src/qemu/qemu_conf.c | 1 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_domain.c | 34 ++- src/qemu/qemu_hotplug.c | 9 + src/qemu/qemu_migration.c | 49 +++- src/qemu/qemu_parse_command.c | 2 +- src/qemu/qemu_process.c | 130 +++++++-- src/qemu/test_libvirtd_qemu.aug.in | 1 + src/security/virt-aa-helper.c | 15 +- src/vbox/vbox_common.c | 10 +- src/vmx/vmx.c | 8 +- src/vz/vz_sdk.c | 8 +- src/xenconfig/xen_common.c | 14 +- src/xenconfig/xen_sxpr.c | 16 +- src/xenconfig/xen_xl.c | 8 +- ...ric-graphics-vnc-socket-attr-listen-address.xml | 30 ++ ...hics-vnc-socket-attr-listen-socket-mismatch.xml | 30 ++ ...eric-graphics-vnc-socket-attr-listen-socket.xml | 30 ++ ...ric-graphics-vnc-socket-attr-listen-address.xml | 30 ++ ...eric-graphics-vnc-socket-attr-listen-socket.xml | 30 ++ .../generic-graphics-vnc-socket-listen.xml | 4 +- .../generic-graphics-vnc-socket.xml | 4 +- tests/genericxml2xmltest.c | 4 + .../qemuargv2xml-graphics-vnc-socket.xml | 4 +- ...emuxml2argv-graphics-spice-auto-socket-cfg.args | 20 ++ ...qemuxml2argv-graphics-spice-auto-socket-cfg.xml | 30 ++ .../qemuxml2argv-graphics-spice-auto-socket.args | 20 ++ .../qemuxml2argv-graphics-spice-auto-socket.xml | 30 ++ .../qemuxml2argv-graphics-spice-sasl.args | 2 +- .../qemuxml2argv-graphics-spice-socket.args | 20 ++ .../qemuxml2argv-graphics-spice-socket.xml | 30 ++ .../qemuxml2argv-graphics-vnc-auto-socket-cfg.args | 22 ++ .../qemuxml2argv-graphics-vnc-auto-socket-cfg.xml | 34 +++ .../qemuxml2argv-graphics-vnc-auto-socket.args | 20 ++ .../qemuxml2argv-graphics-vnc-auto-socket.xml | 30 ++ .../qemuxml2argv-graphics-vnc-autosocket.args | 22 -- .../qemuxml2argv-graphics-vnc-autosocket.xml | 34 --- .../qemuxml2argv-graphics-vnc-none.args | 20 ++ .../qemuxml2argv-graphics-vnc-none.xml | 30 ++ ...2argv-graphics-vnc-remove-generated-socket.args | 22 ++ ...l2argv-graphics-vnc-remove-generated-socket.xml | 34 +++ .../qemuxml2argv-graphics-vnc-socket.args | 4 +- .../qemuxml2argv-graphics-vnc-socket.xml | 10 +- .../qemuxml2argv-video-virtio-gpu-spice-gl.args | 2 +- tests/qemuxml2argvtest.c | 18 ++ ...muxml2xmlout-graphics-spice-auto-socket-cfg.xml | 35 +++ .../qemuxml2xmlout-graphics-spice-auto-socket.xml | 35 +++ .../qemuxml2xmlout-graphics-spice-socket.xml | 35 +++ ...qemuxml2xmlout-graphics-vnc-auto-socket-cfg.xml | 41 +++ .../qemuxml2xmlout-graphics-vnc-auto-socket.xml | 35 +++ .../qemuxml2xmlout-graphics-vnc-autosocket.xml | 39 --- ...graphics-vnc-remove-generated-socket-active.xml | 41 +++ ...aphics-vnc-remove-generated-socket-inactive.xml | 41 +++ .../qemuxml2xmlout-graphics-vnc-socket.xml | 35 +++ .../qemuxml2xmlout-video-virtio-gpu-spice-gl.xml | 4 +- tests/qemuxml2xmltest.c | 20 +- 67 files changed, 1530 insertions(+), 439 deletions(-) create mode 100644 tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-address.xml create mode 100644 tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-socket-mismatch.xml create mode 100644 tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-socket.xml create mode 100644 tests/genericxml2xmloutdata/generic-graphics-vnc-socket-attr-listen-address.xml create mode 100644 tests/genericxml2xmloutdata/generic-graphics-vnc-socket-attr-listen-socket.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket-cfg.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket-cfg.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket.xml delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-none.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-none.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-remove-generated-socket.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-remove-generated-socket.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-auto-socket-cfg.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-auto-socket.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-socket.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-auto-socket-cfg.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-auto-socket.xml delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-remove-generated-socket-active.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-remove-generated-socket-inactive.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-socket.xml -- 2.8.2

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_domain.c | 12 +++++++++--- .../qemuxml2xmlout-graphics-vnc-autosocket.xml | 4 +++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0cec340..65dfa37 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1995,7 +1995,7 @@ qemuCanonicalizeMachine(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) } -static void +static int qemuDomainRecheckInternalPaths(virDomainDefPtr def, virQEMUDriverConfigPtr cfg, unsigned int flags) @@ -2008,12 +2008,17 @@ qemuDomainRecheckInternalPaths(virDomainDefPtr def, if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && graphics->data.vnc.socket && STRPREFIX(graphics->data.vnc.socket, cfg->libDir)) { - if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) + if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) { VIR_FREE(graphics->data.vnc.socket); + if (virDomainGraphicsListenAppendAddress(graphics, NULL) < 0) + return -1; + } else graphics->data.vnc.socketAutogenerated = true; } } + + return 0; } @@ -2066,7 +2071,8 @@ qemuDomainDefPostParse(virDomainDefPtr def, qemuDomainDefEnableDefaultFeatures(def, qemuCaps); - qemuDomainRecheckInternalPaths(def, cfg, parseFlags); + if (qemuDomainRecheckInternalPaths(def, cfg, parseFlags) < 0) + goto cleanup; if (virSecurityManagerVerify(driver->securityManager, def) < 0) goto cleanup; diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml index 7440533..5718041 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml @@ -29,7 +29,9 @@ <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen type='address'/> + </graphics> <video> <model type='cirrus' vram='16384' heads='1' primary='yes'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> -- 2.8.2

On 05/19/2016 07:35 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_domain.c | 12 +++++++++--- .../qemuxml2xmlout-graphics-vnc-autosocket.xml | 4 +++- 2 files changed, 12 insertions(+), 4 deletions(-)
ACK - Cole

Commit 55320c23 introduced a new test for VNC to test if vnc_auto_unix_socket is set in qemu.conf, but forget to enable it in qemuxml2argvtest.c. This patch also moves the code in qemuxml2xmltest.c next to other VNC tests and refactor the test so we also check the case for parsing active XML. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- .../qemuxml2argv-graphics-vnc-auto-socket-cfg.args | 22 ++++++++++++ .../qemuxml2argv-graphics-vnc-auto-socket-cfg.xml | 34 ++++++++++++++++++ .../qemuxml2argv-graphics-vnc-autosocket.args | 22 ------------ .../qemuxml2argv-graphics-vnc-autosocket.xml | 34 ------------------ ...2argv-graphics-vnc-remove-generated-socket.args | 22 ++++++++++++ ...l2argv-graphics-vnc-remove-generated-socket.xml | 34 ++++++++++++++++++ tests/qemuxml2argvtest.c | 4 +++ ...qemuxml2xmlout-graphics-vnc-auto-socket-cfg.xml | 41 ++++++++++++++++++++++ .../qemuxml2xmlout-graphics-vnc-autosocket.xml | 41 ---------------------- ...graphics-vnc-remove-generated-socket-active.xml | 39 ++++++++++++++++++++ ...aphics-vnc-remove-generated-socket-inactive.xml | 41 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 12 ++++--- 12 files changed, 244 insertions(+), 102 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket-cfg.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket-cfg.xml delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-remove-generated-socket.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-remove-generated-socket.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-auto-socket-cfg.xml delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-remove-generated-socket-active.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-remove-generated-socket-inactive.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket-cfg.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket-cfg.args new file mode 100644 index 0000000..cfa63b1 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket-cfg.args @@ -0,0 +1,22 @@ +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 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-vnc unix:/tmp/lib/domain--1-QEMUGuest1/vnc.sock \ +-vga cirrus diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket-cfg.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket-cfg.xml new file mode 100644 index 0000000..af961a5 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket-cfg.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <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'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc'/> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.args deleted file mode 100644 index 7e1fb6b..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.args +++ /dev/null @@ -1,22 +0,0 @@ -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 \ --uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ --nodefaults \ --monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ --no-acpi \ --boot c \ --usb \ --drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ --device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ --vnc unix:/tmp/lib/domain--1-QEMUGuest1/vnc.socket \ --vga cirrus diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.xml deleted file mode 100644 index fa59c39..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.xml +++ /dev/null @@ -1,34 +0,0 @@ -<domain type='qemu'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory unit='KiB'>219100</memory> - <currentMemory unit='KiB'>219100</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> - <disk type='block' device='disk'> - <driver name='qemu' type='raw'/> - <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'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <graphics type='vnc' socket='/tmp/lib/domain-99-QEMUGuest1/delete.this.socket'/> - <video> - <model type='cirrus' vram='16384' heads='1'/> - </video> - <memballoon model='none'/> - </devices> -</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-remove-generated-socket.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-remove-generated-socket.args new file mode 100644 index 0000000..ea2e4d0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-remove-generated-socket.args @@ -0,0 +1,22 @@ +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 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-vnc 127.0.0.1:0 \ +-vga cirrus diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-remove-generated-socket.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-remove-generated-socket.xml new file mode 100644 index 0000000..fa59c39 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-remove-generated-socket.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <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'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' socket='/tmp/lib/domain-99-QEMUGuest1/delete.this.socket'/> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3bfb5c4..de96e19 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -899,6 +899,10 @@ mymain(void) DO_TEST("graphics-vnc-websocket", QEMU_CAPS_VNC, QEMU_CAPS_VNC_WEBSOCKET); DO_TEST("graphics-vnc-policy", QEMU_CAPS_VNC, QEMU_CAPS_VNC_SHARE_POLICY); DO_TEST("graphics-vnc-no-listen-attr", QEMU_CAPS_VNC); + DO_TEST("graphics-vnc-remove-generated-socket", QEMU_CAPS_VNC); + driver.config->vncAutoUnixSocket = true; + DO_TEST("graphics-vnc-auto-socket-cfg", QEMU_CAPS_VNC); + driver.config->vncAutoUnixSocket = false; driver.config->vncSASL = 1; VIR_FREE(driver.config->vncSASLdir); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-auto-socket-cfg.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-auto-socket-cfg.xml new file mode 100644 index 0000000..5718041 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-auto-socket-cfg.xml @@ -0,0 +1,41 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <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'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen type='address'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml deleted file mode 100644 index 5718041..0000000 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml +++ /dev/null @@ -1,41 +0,0 @@ -<domain type='qemu'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory unit='KiB'>219100</memory> - <currentMemory unit='KiB'>219100</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> - <disk type='block' device='disk'> - <driver name='qemu' type='raw'/> - <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'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> - </controller> - <controller type='ide' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> - </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes'> - <listen type='address'/> - </graphics> - <video> - <model type='cirrus' vram='16384' heads='1' primary='yes'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> - </video> - <memballoon model='none'/> - </devices> -</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-remove-generated-socket-active.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-remove-generated-socket-active.xml new file mode 100644 index 0000000..f3ccfdf --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-remove-generated-socket-active.xml @@ -0,0 +1,39 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <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'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' socket='/tmp/lib/domain-99-QEMUGuest1/delete.this.socket'/> + <video> + <model type='cirrus' vram='16384' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-remove-generated-socket-inactive.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-remove-generated-socket-inactive.xml new file mode 100644 index 0000000..5718041 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-remove-generated-socket-inactive.xml @@ -0,0 +1,41 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <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'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen type='address'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 30cf3e8..962e9ba 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -265,6 +265,8 @@ mymain(void) if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; + cfg = virQEMUDriverGetConfig(&driver); + /* TODO: test with format probing disabled too */ driver.config->allowDiskFormatProbing = true; @@ -431,6 +433,11 @@ mymain(void) DO_TEST("graphics-vnc-sasl"); DO_TEST("graphics-vnc-tls"); DO_TEST("graphics-vnc-no-listen-attr"); + DO_TEST("graphics-vnc-remove-generated-socket"); + cfg->vncAutoUnixSocket = true; + DO_TEST("graphics-vnc-auto-socket-cfg"); + cfg->vncAutoUnixSocket = false; + DO_TEST("graphics-sdl"); DO_TEST("graphics-sdl-fullscreen"); DO_TEST("graphics-spice"); @@ -800,11 +807,6 @@ mymain(void) DO_TEST("virtio-input"); DO_TEST("virtio-input-passthrough"); - cfg = virQEMUDriverGetConfig(&driver); - cfg->vncAutoUnixSocket = true; - DO_TEST_FULL("graphics-vnc-autosocket", WHEN_INACTIVE, GIC_NONE, NONE); - cfg->vncAutoUnixSocket = false; - virObjectUnref(cfg); qemuTestDriverFree(&driver); -- 2.8.2

On 05/19/2016 07:35 AM, Pavel Hrdina wrote:
Commit 55320c23 introduced a new test for VNC to test if vnc_auto_unix_socket is set in qemu.conf, but forget to enable it in qemuxml2argvtest.c.
This patch also moves the code in qemuxml2xmltest.c next to other VNC tests and refactor the test so we also check the case for parsing active XML.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
ACK - Cole

We have both in the code. Let's use only one format. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 12 ++++++------ src/libxl/libxl_conf.c | 16 ++++++++-------- src/qemu/qemu_command.c | 28 ++++++++++++++-------------- src/qemu/qemu_migration.c | 4 ++-- src/vbox/vbox_common.c | 10 +++++----- src/vmx/vmx.c | 8 ++++---- src/vz/vz_sdk.c | 8 ++++---- src/xenconfig/xen_common.c | 14 +++++++------- src/xenconfig/xen_sxpr.c | 16 ++++++++-------- src/xenconfig/xen_xl.c | 8 ++++---- 10 files changed, 62 insertions(+), 62 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1dc45f4..b1b2bb9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23845,21 +23845,21 @@ int virDomainGraphicsListenAppendAddress(virDomainGraphicsDefPtr def, const char *address) { - virDomainGraphicsListenDef gListen; + virDomainGraphicsListenDef glisten; - memset(&gListen, 0, sizeof(gListen)); + memset(&glisten, 0, sizeof(glisten)); - gListen.type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; + glisten.type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; - if (VIR_STRDUP(gListen.address, address) < 0) + if (VIR_STRDUP(glisten.address, address) < 0) goto error; - if (VIR_APPEND_ELEMENT_COPY(def->listens, def->nListens, gListen) < 0) + if (VIR_APPEND_ELEMENT_COPY(def->listens, def->nListens, glisten) < 0) goto error; return 0; error: - VIR_FREE(gListen.address); + VIR_FREE(glisten.address); return -1; } diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index c399f5c..13e56ac 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1430,7 +1430,7 @@ libxlMakeVfb(virPortAllocatorPtr graphicsports, libxl_device_vfb *x_vfb) { unsigned short port; - virDomainGraphicsListenDefPtr gListen = NULL; + virDomainGraphicsListenDefPtr glisten = NULL; libxl_device_vfb_init(x_vfb); @@ -1457,11 +1457,11 @@ libxlMakeVfb(virPortAllocatorPtr graphicsports, } x_vfb->vnc.display = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN; - if ((gListen = virDomainGraphicsGetListen(l_vfb, 0)) && - gListen->address) { + if ((glisten = virDomainGraphicsGetListen(l_vfb, 0)) && + glisten->address) { /* libxl_device_vfb_init() does VIR_STRDUP("127.0.0.1") */ VIR_FREE(x_vfb->vnc.listen); - if (VIR_STRDUP(x_vfb->vnc.listen, gListen->address) < 0) + if (VIR_STRDUP(x_vfb->vnc.listen, glisten->address) < 0) return -1; } if (VIR_STRDUP(x_vfb->vnc.passwd, l_vfb->data.vnc.auth.passwd) < 0) @@ -1551,7 +1551,7 @@ libxlMakeBuildInfoVfb(virPortAllocatorPtr graphicsports, for (i = 0; i < def->ngraphics; i++) { virDomainGraphicsDefPtr l_vfb = def->graphics[i]; unsigned short port; - virDomainGraphicsListenDefPtr gListen = NULL; + virDomainGraphicsListenDefPtr glisten = NULL; if (l_vfb->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) continue; @@ -1565,9 +1565,9 @@ libxlMakeBuildInfoVfb(virPortAllocatorPtr graphicsports, } b_info->u.hvm.spice.port = l_vfb->data.spice.port; - if ((gListen = virDomainGraphicsGetListen(l_vfb, 0)) && - gListen->address && - VIR_STRDUP(b_info->u.hvm.spice.host, gListen->address) < 0) + if ((glisten = virDomainGraphicsGetListen(l_vfb, 0)) && + glisten->address && + VIR_STRDUP(b_info->u.hvm.spice.host, glisten->address) < 0) return -1; if (VIR_STRDUP(b_info->u.hvm.keymap, l_vfb->data.spice.keymap) < 0) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4a2fa1d..61ab30e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7383,7 +7383,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, const char *domainLibDir) { virBuffer opt = VIR_BUFFER_INITIALIZER; - virDomainGraphicsListenDefPtr gListen = NULL; + virDomainGraphicsListenDefPtr glisten = NULL; const char *listenAddr = NULL; char *netAddr = NULL; bool escapeAddr; @@ -7416,18 +7416,18 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, goto error; } - if ((gListen = virDomainGraphicsGetListen(graphics, 0))) { + if ((glisten = virDomainGraphicsGetListen(graphics, 0))) { - switch (gListen->type) { + switch (glisten->type) { case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = gListen->address; + listenAddr = glisten->address; break; case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - if (!gListen->network) + if (!glisten->network) break; - ret = networkGetNetworkAddress(gListen->network, &netAddr); + ret = networkGetNetworkAddress(glisten->network, &netAddr); if (ret <= -2) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("network-based listen not possible, " @@ -7440,7 +7440,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, listenAddr = netAddr; /* store the address we found in the <graphics> element so it * will show up in status. */ - if (VIR_STRDUP(gListen->address, netAddr) < 0) + if (VIR_STRDUP(glisten->address, netAddr) < 0) goto error; break; @@ -7538,7 +7538,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, virDomainGraphicsDefPtr graphics) { virBuffer opt = VIR_BUFFER_INITIALIZER; - virDomainGraphicsListenDefPtr gListen = NULL; + virDomainGraphicsListenDefPtr glisten = NULL; const char *listenAddr = NULL; char *netAddr = NULL; int ret; @@ -7577,18 +7577,18 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, } if (port > 0 || tlsPort > 0) { - if ((gListen = virDomainGraphicsGetListen(graphics, 0))) { + if ((glisten = virDomainGraphicsGetListen(graphics, 0))) { - switch (gListen->type) { + switch (glisten->type) { case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = gListen->address; + listenAddr = glisten->address; break; case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - if (!gListen->network) + if (!glisten->network) break; - ret = networkGetNetworkAddress(gListen->network, &netAddr); + ret = networkGetNetworkAddress(glisten->network, &netAddr); if (ret <= -2) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("network-based listen not possible, " @@ -7601,7 +7601,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, listenAddr = netAddr; /* store the address we found in the <graphics> element so it will * show up in status. */ - if (VIR_STRDUP(gListen->address, listenAddr) < 0) + if (VIR_STRDUP(glisten->address, listenAddr) < 0) goto error; break; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 680c9ba..25093ac 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -319,7 +319,7 @@ qemuMigrationCookieGraphicsSpiceAlloc(virQEMUDriverPtr driver, { qemuMigrationCookieGraphicsPtr mig = NULL; const char *listenAddr; - virDomainGraphicsListenDefPtr gListen = virDomainGraphicsGetListen(def, 0); + virDomainGraphicsListenDefPtr glisten = virDomainGraphicsGetListen(def, 0); virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); if (VIR_ALLOC(mig) < 0) @@ -332,7 +332,7 @@ qemuMigrationCookieGraphicsSpiceAlloc(virQEMUDriverPtr driver, else mig->tlsPort = -1; - if (!gListen || !(listenAddr = gListen->address)) + if (!glisten || !(listenAddr = glisten->address)) listenAddr = cfg->spiceListen; #ifdef WITH_GNUTLS diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 13c3fec..ee2b6ad 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -1579,7 +1579,7 @@ vboxAttachDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) char *guiDisplay = NULL; char *sdlDisplay = NULL; size_t i = 0; - virDomainGraphicsListenDefPtr gListen; + virDomainGraphicsListenDefPtr glisten; for (i = 0; i < def->ngraphics; i++) { IVRDxServer *VRDxServer = NULL; @@ -1607,15 +1607,15 @@ vboxAttachDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) VIR_DEBUG("VRDP set to allow multiple connection"); } - if ((gListen = virDomainGraphicsGetListen(def->graphics[i], 0)) && - gListen->address) { + if ((glisten = virDomainGraphicsGetListen(def->graphics[i], 0)) && + glisten->address) { PRUnichar *netAddressUtf16 = NULL; - VBOX_UTF8_TO_UTF16(gListen->address, &netAddressUtf16); + VBOX_UTF8_TO_UTF16(glisten->address, &netAddressUtf16); gVBoxAPI.UIVRDxServer.SetNetAddress(data, VRDxServer, netAddressUtf16); VIR_DEBUG("VRDP listen address is set to: %s", - gListen->address); + glisten->address); VBOX_UTF16_FREE(netAddressUtf16); } diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 6505dd7..f729e5d 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -3402,7 +3402,7 @@ virVMXFormatConfig(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virDomainDe int virVMXFormatVNC(virDomainGraphicsDefPtr def, virBufferPtr buffer) { - virDomainGraphicsListenDefPtr gListen; + virDomainGraphicsListenDefPtr glisten; if (def->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); @@ -3424,10 +3424,10 @@ virVMXFormatVNC(virDomainGraphicsDefPtr def, virBufferPtr buffer) def->data.vnc.port); } - if ((gListen = virDomainGraphicsGetListen(def, 0)) && - gListen->address) { + if ((glisten = virDomainGraphicsGetListen(def, 0)) && + glisten->address) { virBufferAsprintf(buffer, "RemoteDisplay.vnc.ip = \"%s\"\n", - gListen->address); + glisten->address); } if (def->data.vnc.keymap != NULL) { diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 2275572..9aa5a76 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -2683,7 +2683,7 @@ static int prlsdkCheckFSUnsupportedParams(virDomainFSDefPtr fs) static int prlsdkApplyGraphicsParams(PRL_HANDLE sdkdom, virDomainDefPtr def) { virDomainGraphicsDefPtr gr; - virDomainGraphicsListenDefPtr gListen; + virDomainGraphicsListenDefPtr glisten; PRL_RESULT pret; int ret = -1; @@ -2706,10 +2706,10 @@ static int prlsdkApplyGraphicsParams(PRL_HANDLE sdkdom, virDomainDefPtr def) prlsdkCheckRetGoto(pret, cleanup); } - if ((gListen = virDomainGraphicsGetListen(gr, 0))) { - if (!gListen->address) + if ((glisten = virDomainGraphicsGetListen(gr, 0))) { + if (!glisten->address) goto cleanup; - pret = PrlVmCfg_SetVNCHostName(sdkdom, gListen->address); + pret = PrlVmCfg_SetVNCHostName(sdkdom, glisten->address); prlsdkCheckRetGoto(pret, cleanup); } diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 5014bad..a568666 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1643,7 +1643,7 @@ xenFormatVfb(virConfPtr conf, virDomainDefPtr def) def->graphics[0]->data.sdl.xauth) < 0) return -1; } else { - virDomainGraphicsListenDefPtr gListen; + virDomainGraphicsListenDefPtr glisten; if (xenConfigSetInt(conf, "sdl", 0) < 0) return -1; @@ -1660,9 +1660,9 @@ xenFormatVfb(virConfPtr conf, virDomainDefPtr def) def->graphics[0]->data.vnc.port - 5900) < 0) return -1; - if ((gListen = virDomainGraphicsGetListen(def->graphics[0], 0)) && - gListen->address && - xenConfigSetString(conf, "vnclisten", gListen->address) < 0) + if ((glisten = virDomainGraphicsGetListen(def->graphics[0], 0)) && + glisten->address && + xenConfigSetString(conf, "vnclisten", glisten->address) < 0) return -1; if (def->graphics[0]->data.vnc.auth.passwd && @@ -1689,7 +1689,7 @@ xenFormatVfb(virConfPtr conf, virDomainDefPtr def) virBufferAsprintf(&buf, ",xauthority=%s", def->graphics[0]->data.sdl.xauth); } else { - virDomainGraphicsListenDefPtr gListen + virDomainGraphicsListenDefPtr glisten = virDomainGraphicsGetListen(def->graphics[0], 0); virBufferAddLit(&buf, "type=vnc"); @@ -1698,8 +1698,8 @@ xenFormatVfb(virConfPtr conf, virDomainDefPtr def) if (!def->graphics[0]->data.vnc.autoport) virBufferAsprintf(&buf, ",vncdisplay=%d", def->graphics[0]->data.vnc.port - 5900); - if (gListen && gListen->address) - virBufferAsprintf(&buf, ",vnclisten=%s", gListen->address); + if (glisten && glisten->address) + virBufferAsprintf(&buf, ",vnclisten=%s", glisten->address); if (def->graphics[0]->data.vnc.auth.passwd) virBufferAsprintf(&buf, ",vncpasswd=%s", def->graphics[0]->data.vnc.auth.passwd); diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index 8b5e063..6c7b2c7 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -1523,7 +1523,7 @@ static int xenFormatSxprGraphicsNew(virDomainGraphicsDefPtr def, virBufferPtr buf) { - virDomainGraphicsListenDefPtr gListen; + virDomainGraphicsListenDefPtr glisten; if (def->type != VIR_DOMAIN_GRAPHICS_TYPE_SDL && def->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) { @@ -1551,9 +1551,9 @@ xenFormatSxprGraphicsNew(virDomainGraphicsDefPtr def, virBufferAsprintf(buf, "(vncdisplay %d)", def->data.vnc.port-5900); } - if ((gListen = virDomainGraphicsGetListen(def, 0)) && - gListen->address) - virBufferAsprintf(buf, "(vnclisten '%s')", gListen->address); + if ((glisten = virDomainGraphicsGetListen(def, 0)) && + glisten->address) + virBufferAsprintf(buf, "(vnclisten '%s')", glisten->address); if (def->data.vnc.auth.passwd) virBufferAsprintf(buf, "(vncpasswd '%s')", def->data.vnc.auth.passwd); if (def->data.vnc.keymap) @@ -1579,7 +1579,7 @@ xenFormatSxprGraphicsNew(virDomainGraphicsDefPtr def, static int xenFormatSxprGraphicsOld(virDomainGraphicsDefPtr def, virBufferPtr buf) { - virDomainGraphicsListenDefPtr gListen; + virDomainGraphicsListenDefPtr glisten; if (def->type != VIR_DOMAIN_GRAPHICS_TYPE_SDL && def->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) { @@ -1604,9 +1604,9 @@ xenFormatSxprGraphicsOld(virDomainGraphicsDefPtr def, virBufferPtr buf) virBufferAsprintf(buf, "(vncdisplay %d)", def->data.vnc.port-5900); } - if ((gListen = virDomainGraphicsGetListen(def, 0)) && - gListen->address) - virBufferAsprintf(buf, "(vnclisten '%s')", gListen->address); + if ((glisten = virDomainGraphicsGetListen(def, 0)) && + glisten->address) + virBufferAsprintf(buf, "(vnclisten '%s')", glisten->address); if (def->data.vnc.auth.passwd) virBufferAsprintf(buf, "(vncpasswd '%s')", def->data.vnc.auth.passwd); if (def->data.vnc.keymap) diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index 15350f9..9b8306f 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -837,7 +837,7 @@ xenFormatXLDomainDisks(virConfPtr conf, virDomainDefPtr def) static int xenFormatXLSpice(virConfPtr conf, virDomainDefPtr def) { - virDomainGraphicsListenDefPtr gListen; + virDomainGraphicsListenDefPtr glisten; virDomainGraphicsDefPtr graphics; if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { @@ -854,9 +854,9 @@ xenFormatXLSpice(virConfPtr conf, virDomainDefPtr def) if (xenConfigSetInt(conf, "spice", 1) < 0) return -1; - if ((gListen = virDomainGraphicsGetListen(graphics, 0)) && - gListen->address && - xenConfigSetString(conf, "spicehost", gListen->address) < 0) + if ((glisten = virDomainGraphicsGetListen(graphics, 0)) && + glisten->address && + xenConfigSetString(conf, "spicehost", glisten->address) < 0) return -1; if (xenConfigSetInt(conf, "spiceport", -- 2.8.2

Move code that decide whether we print the 'listen' attribute or not into virDomainGraphicsAddListenAddr() function. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 59 +++++++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b1b2bb9..b3b60f1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21397,13 +21397,43 @@ virDomainGraphicsListenDefFormat(virBufferPtr buf, } +/** + * virDomainGraphicsAddListenAddr: + * @buf: buffer where the output XML is written + * @glisten: first listen element + * @flags: bit-wise or of VIR_DOMAIN_DEF_FORMAT_* + * + * This is used to add a legacy 'listen' attribute into <graphics> element to + * improve backward compatibility. + */ +static void +virDomainGraphicsAddListenAddr(virBufferPtr buf, + virDomainGraphicsListenDefPtr glisten, + unsigned int flags) +{ + if (!glisten) + return; + + if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE && glisten->fromConfig) + return; + + if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK && + flags & (VIR_DOMAIN_DEF_FORMAT_INACTIVE | + VIR_DOMAIN_DEF_FORMAT_MIGRATABLE)) + return; + + if (glisten->address) + virBufferAsprintf(buf, " listen='%s'", glisten->address); +} + + static int virDomainGraphicsDefFormat(virBufferPtr buf, virDomainGraphicsDefPtr def, unsigned int flags) { + virDomainGraphicsListenDefPtr glisten = virDomainGraphicsGetListen(def, 0); const char *type = virDomainGraphicsTypeToString(def->type); - const char *listenAddr = NULL; bool children = false; size_t i; @@ -21413,24 +21443,6 @@ virDomainGraphicsDefFormat(virBufferPtr buf, return -1; } - /* find the first listen subelement with a valid address and - * duplicate its address attribute as the listen attribute of - * <graphics>. This is done to improve backward compatibility. - */ - for (i = 0; i < def->nListens; i++) { - if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE && - def->listens[i].fromConfig) - continue; - - if (def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK && - flags & (VIR_DOMAIN_DEF_FORMAT_INACTIVE | - VIR_DOMAIN_DEF_FORMAT_MIGRATABLE)) - continue; - - if ((listenAddr = def->listens[i].address)) - break; - } - virBufferAsprintf(buf, "<graphics type='%s'", type); switch (def->type) { @@ -21455,8 +21467,7 @@ virDomainGraphicsDefFormat(virBufferPtr buf, if (def->data.vnc.websocket) virBufferAsprintf(buf, " websocket='%d'", def->data.vnc.websocket); - if (listenAddr) - virBufferAsprintf(buf, " listen='%s'", listenAddr); + virDomainGraphicsAddListenAddr(buf, glisten, flags); } if (def->data.vnc.keymap) @@ -21500,8 +21511,7 @@ virDomainGraphicsDefFormat(virBufferPtr buf, if (def->data.rdp.multiUser) virBufferAddLit(buf, " multiUser='yes'"); - if (listenAddr) - virBufferAsprintf(buf, " listen='%s'", listenAddr); + virDomainGraphicsAddListenAddr(buf, glisten, flags); break; @@ -21527,8 +21537,7 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " autoport='%s'", def->data.spice.autoport ? "yes" : "no"); - if (listenAddr) - virBufferAsprintf(buf, " listen='%s'", listenAddr); + virDomainGraphicsAddListenAddr(buf, glisten, flags); if (def->data.spice.keymap) virBufferEscapeString(buf, " keymap='%s'", -- 2.8.2

On 05/19/2016 07:35 AM, Pavel Hrdina wrote:
Move code that decide whether we print the 'listen' attribute or not into virDomainGraphicsAddListenAddr() function.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 59 +++++++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 25 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b1b2bb9..b3b60f1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21397,13 +21397,43 @@ virDomainGraphicsListenDefFormat(virBufferPtr buf, }
+/** + * virDomainGraphicsAddListenAddr:
The naming confused me, how about virDomainGraphicsListenDefFormatAddr? To follow the previous function name of virDomainGraphicsListenDefFormat. Just something with Format in it at least Also this patch makes me realize we totally lack MIGRATABLE xml tests, but that can be additive. ACK - Cole

On Thu, May 19, 2016 at 03:58:58PM -0400, Cole Robinson wrote:
On 05/19/2016 07:35 AM, Pavel Hrdina wrote:
Move code that decide whether we print the 'listen' attribute or not into virDomainGraphicsAddListenAddr() function.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 59 +++++++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 25 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b1b2bb9..b3b60f1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21397,13 +21397,43 @@ virDomainGraphicsListenDefFormat(virBufferPtr buf, }
+/** + * virDomainGraphicsAddListenAddr:
The naming confused me, how about virDomainGraphicsListenDefFormatAddr? To follow the previous function name of virDomainGraphicsListenDefFormat. Just something with Format in it at least
Sure, that's a better name.
Also this patch makes me realize we totally lack MIGRATABLE xml tests, but that can be additive.
I've noticed that too and we definitely need those tests to not break migration.
ACK
Thanks

This is required for following patches where new listen types will be introduced. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 20 ++++++++++---------- .../qemuxml2argv-graphics-spice-sasl.args | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 61ab30e..e5847f7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7566,16 +7566,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, virBufferAsprintf(&opt, "tls-port=%u,", tlsPort); } - if (cfg->spiceSASL) { - virBufferAddLit(&opt, "sasl,"); - - if (cfg->spiceSASLdir) - virCommandAddEnvPair(cmd, "SASL_CONF_PATH", - cfg->spiceSASLdir); - - /* TODO: Support ACLs later */ - } - if (port > 0 || tlsPort > 0) { if ((glisten = virDomainGraphicsGetListen(graphics, 0))) { @@ -7619,6 +7609,16 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, VIR_FREE(netAddr); } + if (cfg->spiceSASL) { + virBufferAddLit(&opt, "sasl,"); + + if (cfg->spiceSASLdir) + virCommandAddEnvPair(cmd, "SASL_CONF_PATH", + cfg->spiceSASLdir); + + /* TODO: Support ACLs later */ + } + if (graphics->data.spice.mousemode) { switch (graphics->data.spice.mousemode) { case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args index e21d699..bf9045f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args @@ -19,7 +19,7 @@ QEMU_AUDIO_DRV=spice \ -usb \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ --spice port=5903,tls-port=5904,sasl,addr=127.0.0.1,\ +-spice port=5903,tls-port=5904,addr=127.0.0.1,sasl,\ x509-dir=/etc/pki/libvirt-spice,tls-channel=default \ -vga qxl \ -global qxl-vga.ram_size=67108864 \ -- 2.8.2

On 05/19/2016 07:35 AM, Pavel Hrdina wrote:
This is required for following patches where new listen types will be introduced.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 20 ++++++++++---------- .../qemuxml2argv-graphics-spice-sasl.args | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-)
ACK - Cole

Both VNC and SPICE requires the same code to resolve address for listen type network. Remove code duplication and create a new function that will be used in qemuProcessSetupGraphics(). Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 103 ++++++------------------------------------------ src/qemu/qemu_process.c | 47 +++++++++++++++++++++- 2 files changed, 57 insertions(+), 93 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e5847f7..ee43e21 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7384,10 +7384,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, { virBuffer opt = VIR_BUFFER_INITIALIZER; virDomainGraphicsListenDefPtr glisten = NULL; - const char *listenAddr = NULL; - char *netAddr = NULL; bool escapeAddr; - int ret; if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -7395,6 +7392,8 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, goto error; } + glisten = virDomainGraphicsGetListen(graphics, 0); + if (graphics->data.vnc.socket || cfg->vncAutoUnixSocket) { if (!graphics->data.vnc.socket) { if (virAsprintf(&graphics->data.vnc.socket, @@ -7416,52 +7415,15 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, goto error; } - if ((glisten = virDomainGraphicsGetListen(graphics, 0))) { - - switch (glisten->type) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = glisten->address; - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - if (!glisten->network) - break; - - ret = networkGetNetworkAddress(glisten->network, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) - goto error; - - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it - * will show up in status. */ - if (VIR_STRDUP(glisten->address, netAddr) < 0) - goto error; - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: - break; - } + if (glisten && glisten->address) { + escapeAddr = strchr(glisten->address, ':') != NULL; + if (escapeAddr) + virBufferAsprintf(&opt, "[%s]", glisten->address); + else + virBufferAdd(&opt, glisten->address, -1); } - - if (!listenAddr) - listenAddr = cfg->vncListen; - - escapeAddr = strchr(listenAddr, ':') != NULL; - if (escapeAddr) - virBufferAsprintf(&opt, "[%s]", listenAddr); - else - virBufferAdd(&opt, listenAddr, -1); virBufferAsprintf(&opt, ":%d", graphics->data.vnc.port - 5900); - - VIR_FREE(netAddr); } if (!graphics->data.vnc.socket && @@ -7525,7 +7487,6 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, return 0; error: - VIR_FREE(netAddr); virBufferFreeAndReset(&opt); return -1; } @@ -7539,9 +7500,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, { virBuffer opt = VIR_BUFFER_INITIALIZER; virDomainGraphicsListenDefPtr glisten = NULL; - const char *listenAddr = NULL; - char *netAddr = NULL; - int ret; int defaultMode = graphics->data.spice.defaultMode; int port = graphics->data.spice.port; int tlsPort = graphics->data.spice.tlsPort; @@ -7553,6 +7511,8 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, goto error; } + glisten = virDomainGraphicsGetListen(graphics, 0); + if (port > 0) virBufferAsprintf(&opt, "port=%u,", port); @@ -7567,46 +7527,8 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, } if (port > 0 || tlsPort > 0) { - if ((glisten = virDomainGraphicsGetListen(graphics, 0))) { - - switch (glisten->type) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = glisten->address; - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - if (!glisten->network) - break; - - ret = networkGetNetworkAddress(glisten->network, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) - goto error; - - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it will - * show up in status. */ - if (VIR_STRDUP(glisten->address, listenAddr) < 0) - goto error; - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: - break; - } - } - - if (!listenAddr) - listenAddr = cfg->spiceListen; - if (listenAddr) - virBufferAsprintf(&opt, "addr=%s,", listenAddr); - - VIR_FREE(netAddr); + if (glisten && glisten->address) + virBufferAsprintf(&opt, "addr=%s,", glisten->address); } if (cfg->spiceSASL) { @@ -7775,7 +7697,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, return 0; error: - VIR_FREE(netAddr); virBufferFreeAndReset(&opt); return -1; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f4bf6c1..75c8e53 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4388,6 +4388,32 @@ qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver, static int +qemuProcessGraphicsSetupNetworkAddress(virDomainGraphicsListenDefPtr glisten, + const char *listenAddr) +{ + int rc; + + if (!glisten->network) { + if (VIR_STRDUP(glisten->address, listenAddr) < 0) + return -1; + return 0; + } + + rc = networkGetNetworkAddress(glisten->network, &glisten->address); + if (rc <= -2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("network-based listen isn't possible, " + "network driver isn't present")); + return -1; + } + if (rc < 0) + return -1; + + return 0; +} + + +static int qemuProcessSetupGraphics(virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int flags) @@ -4429,12 +4455,29 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, for (j = 0; j < graphics->nListens; j++) { virDomainGraphicsListenDefPtr glisten = &graphics->listens[j]; - if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS && - !glisten->address && listenAddr) { + switch (glisten->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + if (glisten->address || !listenAddr) + continue; + if (VIR_STRDUP(glisten->address, listenAddr) < 0) goto cleanup; glisten->fromConfig = true; + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + if (glisten->address || !listenAddr) + continue; + + if (qemuProcessGraphicsSetupNetworkAddress(glisten, + listenAddr) < 0) + goto cleanup; + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: + break; } } } -- 2.8.2

On 05/19/2016 07:35 AM, Pavel Hrdina wrote:
Both VNC and SPICE requires the same code to resolve address for listen type network. Remove code duplication and create a new function that will be used in qemuProcessSetupGraphics().
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 103 ++++++------------------------------------------ src/qemu/qemu_process.c | 47 +++++++++++++++++++++- 2 files changed, 57 insertions(+), 93 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e5847f7..ee43e21 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7384,10 +7384,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, { virBuffer opt = VIR_BUFFER_INITIALIZER; virDomainGraphicsListenDefPtr glisten = NULL; - const char *listenAddr = NULL; - char *netAddr = NULL; bool escapeAddr; - int ret;
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -7395,6 +7392,8 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, goto error; }
+ glisten = virDomainGraphicsGetListen(graphics, 0); + if (graphics->data.vnc.socket || cfg->vncAutoUnixSocket) { if (!graphics->data.vnc.socket) { if (virAsprintf(&graphics->data.vnc.socket, @@ -7416,52 +7415,15 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, goto error; }
- if ((glisten = virDomainGraphicsGetListen(graphics, 0))) { - - switch (glisten->type) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = glisten->address; - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - if (!glisten->network) - break; - - ret = networkGetNetworkAddress(glisten->network, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) - goto error; - - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it - * will show up in status. */ - if (VIR_STRDUP(glisten->address, netAddr) < 0) - goto error; - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: - break; - } + if (glisten && glisten->address) { + escapeAddr = strchr(glisten->address, ':') != NULL; + if (escapeAddr) + virBufferAsprintf(&opt, "[%s]", glisten->address); + else + virBufferAdd(&opt, glisten->address, -1); } - - if (!listenAddr) - listenAddr = cfg->vncListen; -
This bit being dropped kinda confused me, but I see that this is taken care of at the new SetupNetworkAddress callers already
- escapeAddr = strchr(listenAddr, ':') != NULL; - if (escapeAddr) - virBufferAsprintf(&opt, "[%s]", listenAddr); - else - virBufferAdd(&opt, listenAddr, -1); virBufferAsprintf(&opt, ":%d", graphics->data.vnc.port - 5900); - - VIR_FREE(netAddr); }
if (!graphics->data.vnc.socket && @@ -7525,7 +7487,6 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, return 0;
error: - VIR_FREE(netAddr); virBufferFreeAndReset(&opt); return -1; } @@ -7539,9 +7500,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, { virBuffer opt = VIR_BUFFER_INITIALIZER; virDomainGraphicsListenDefPtr glisten = NULL; - const char *listenAddr = NULL; - char *netAddr = NULL; - int ret; int defaultMode = graphics->data.spice.defaultMode; int port = graphics->data.spice.port; int tlsPort = graphics->data.spice.tlsPort; @@ -7553,6 +7511,8 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, goto error; }
+ glisten = virDomainGraphicsGetListen(graphics, 0); + if (port > 0) virBufferAsprintf(&opt, "port=%u,", port);
@@ -7567,46 +7527,8 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, }
if (port > 0 || tlsPort > 0) { - if ((glisten = virDomainGraphicsGetListen(graphics, 0))) { - - switch (glisten->type) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = glisten->address; - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - if (!glisten->network) - break; - - ret = networkGetNetworkAddress(glisten->network, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) - goto error; - - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it will - * show up in status. */ - if (VIR_STRDUP(glisten->address, listenAddr) < 0) - goto error; - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: - break; - } - } - - if (!listenAddr) - listenAddr = cfg->spiceListen; - if (listenAddr) - virBufferAsprintf(&opt, "addr=%s,", listenAddr); - - VIR_FREE(netAddr); + if (glisten && glisten->address) + virBufferAsprintf(&opt, "addr=%s,", glisten->address); }
if (cfg->spiceSASL) { @@ -7775,7 +7697,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, return 0;
error: - VIR_FREE(netAddr); virBufferFreeAndReset(&opt); return -1; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f4bf6c1..75c8e53 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4388,6 +4388,32 @@ qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver,
static int +qemuProcessGraphicsSetupNetworkAddress(virDomainGraphicsListenDefPtr glisten, + const char *listenAddr) +{ + int rc; + + if (!glisten->network) { + if (VIR_STRDUP(glisten->address, listenAddr) < 0) + return -1; + return 0; + } +
This is a logic change. Previously we accept this XML <graphics ...> <listen type='network'/> </graphics> and silently treat that as using vnc_listen/spice_listen. Now we stick that address in the XML like <graphics ...> <listen type='network' address='$vnc_listen'/> </graphics> Which at least is more explicit, but it is a logic change. Just shows that the address type='network' stuff needs more test coverage at least. I think at some point we should reject bare type='network' if it doesn't have a @network attribute If that change was intentional it should be an additive patch after this cleanup, with a test case
+ rc = networkGetNetworkAddress(glisten->network, &glisten->address); + if (rc <= -2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("network-based listen isn't possible, " + "network driver isn't present")); + return -1; + } + if (rc < 0) + return -1; + + return 0; +} + + +static int qemuProcessSetupGraphics(virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int flags) @@ -4429,12 +4455,29 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, for (j = 0; j < graphics->nListens; j++) { virDomainGraphicsListenDefPtr glisten = &graphics->listens[j];
- if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS && - !glisten->address && listenAddr) { + switch (glisten->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + if (glisten->address || !listenAddr) + continue; + if (VIR_STRDUP(glisten->address, listenAddr) < 0) goto cleanup;
glisten->fromConfig = true; + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + if (glisten->address || !listenAddr) + continue; +
Can listenAddr ever be NULL? I know the logic is pre-existing, but I think the check is redundant. Particularly so for this case if the bit I mention above is changed - Cole

On 05/19/2016 04:50 PM, Cole Robinson wrote:
On 05/19/2016 07:35 AM, Pavel Hrdina wrote:
Both VNC and SPICE requires the same code to resolve address for listen type network. Remove code duplication and create a new function that will be used in qemuProcessSetupGraphics().
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 103 ++++++------------------------------------------ src/qemu/qemu_process.c | 47 +++++++++++++++++++++- 2 files changed, 57 insertions(+), 93 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f4bf6c1..75c8e53 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4388,6 +4388,32 @@ qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver,
static int +qemuProcessGraphicsSetupNetworkAddress(virDomainGraphicsListenDefPtr glisten, + const char *listenAddr) +{ + int rc; + + if (!glisten->network) { + if (VIR_STRDUP(glisten->address, listenAddr) < 0) + return -1; + return 0; + } +
This is a logic change. Previously we accept this XML
<graphics ...> <listen type='network'/> </graphics>
and silently treat that as using vnc_listen/spice_listen. Now we stick that address in the XML like
<graphics ...> <listen type='network' address='$vnc_listen'/> </graphics>
Which at least is more explicit, but it is a logic change. Just shows that the address type='network' stuff needs more test coverage at least. I think at some point we should reject bare type='network' if it doesn't have a @network attribute
If that change was intentional it should be an additive patch after this cleanup, with a test case
Hmm okay I see that it must be intentional, because the qemu_command code now depends on glisten->address. So ACK to this as long as you add a todo item to add some test cases for this stuff, and to figure out the bare <listen type='network'/> weirdness Thanks, Cole

On Thu, May 19, 2016 at 04:50:24PM -0400, Cole Robinson wrote:
On 05/19/2016 07:35 AM, Pavel Hrdina wrote:
Both VNC and SPICE requires the same code to resolve address for listen type network. Remove code duplication and create a new function that will be used in qemuProcessSetupGraphics().
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 103 ++++++------------------------------------------ src/qemu/qemu_process.c | 47 +++++++++++++++++++++- 2 files changed, 57 insertions(+), 93 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e5847f7..ee43e21 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7384,10 +7384,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, { virBuffer opt = VIR_BUFFER_INITIALIZER; virDomainGraphicsListenDefPtr glisten = NULL; - const char *listenAddr = NULL; - char *netAddr = NULL; bool escapeAddr; - int ret;
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -7395,6 +7392,8 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, goto error; }
+ glisten = virDomainGraphicsGetListen(graphics, 0); + if (graphics->data.vnc.socket || cfg->vncAutoUnixSocket) { if (!graphics->data.vnc.socket) { if (virAsprintf(&graphics->data.vnc.socket, @@ -7416,52 +7415,15 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, goto error; }
- if ((glisten = virDomainGraphicsGetListen(graphics, 0))) { - - switch (glisten->type) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = glisten->address; - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - if (!glisten->network) - break;
[1]
- - ret = networkGetNetworkAddress(glisten->network, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) - goto error; - - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it - * will show up in status. */ - if (VIR_STRDUP(glisten->address, netAddr) < 0) - goto error; - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: - break; - } + if (glisten && glisten->address) { + escapeAddr = strchr(glisten->address, ':') != NULL; + if (escapeAddr) + virBufferAsprintf(&opt, "[%s]", glisten->address); + else + virBufferAdd(&opt, glisten->address, -1); } - - if (!listenAddr) - listenAddr = cfg->vncListen; -
[2]
This bit being dropped kinda confused me, but I see that this is taken care of at the new SetupNetworkAddress callers already
- escapeAddr = strchr(listenAddr, ':') != NULL; - if (escapeAddr) - virBufferAsprintf(&opt, "[%s]", listenAddr); - else - virBufferAdd(&opt, listenAddr, -1); virBufferAsprintf(&opt, ":%d", graphics->data.vnc.port - 5900); - - VIR_FREE(netAddr); }
if (!graphics->data.vnc.socket && @@ -7525,7 +7487,6 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, return 0;
error: - VIR_FREE(netAddr); virBufferFreeAndReset(&opt); return -1; } @@ -7539,9 +7500,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, { virBuffer opt = VIR_BUFFER_INITIALIZER; virDomainGraphicsListenDefPtr glisten = NULL; - const char *listenAddr = NULL; - char *netAddr = NULL; - int ret; int defaultMode = graphics->data.spice.defaultMode; int port = graphics->data.spice.port; int tlsPort = graphics->data.spice.tlsPort; @@ -7553,6 +7511,8 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, goto error; }
+ glisten = virDomainGraphicsGetListen(graphics, 0); + if (port > 0) virBufferAsprintf(&opt, "port=%u,", port);
@@ -7567,46 +7527,8 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, }
if (port > 0 || tlsPort > 0) { - if ((glisten = virDomainGraphicsGetListen(graphics, 0))) { - - switch (glisten->type) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = glisten->address; - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - if (!glisten->network) - break; - - ret = networkGetNetworkAddress(glisten->network, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) - goto error; - - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it will - * show up in status. */ - if (VIR_STRDUP(glisten->address, listenAddr) < 0) - goto error; - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: - break; - } - } - - if (!listenAddr) - listenAddr = cfg->spiceListen; - if (listenAddr) - virBufferAsprintf(&opt, "addr=%s,", listenAddr); - - VIR_FREE(netAddr); + if (glisten && glisten->address) + virBufferAsprintf(&opt, "addr=%s,", glisten->address); }
if (cfg->spiceSASL) { @@ -7775,7 +7697,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, return 0;
error: - VIR_FREE(netAddr); virBufferFreeAndReset(&opt); return -1; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f4bf6c1..75c8e53 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4388,6 +4388,32 @@ qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver,
static int +qemuProcessGraphicsSetupNetworkAddress(virDomainGraphicsListenDefPtr glisten, + const char *listenAddr) +{ + int rc; + + if (!glisten->network) { + if (VIR_STRDUP(glisten->address, listenAddr) < 0) + return -1; + return 0; + } +
This is a logic change. Previously we accept this XML
<graphics ...> <listen type='network'/> </graphics>
and silently treat that as using vnc_listen/spice_listen. Now we stick that address in the XML like
<graphics ...> <listen type='network' address='$vnc_listen'/> </graphics>
This isn't a logic change, it only improves the live XML, so the only change here is that the address will appear in the live XML. If you look at the old code we've always used the config listen address if there wasn't any address resolved for listen type network. If you look at the switch [1] we jump out from the switch in case there is no network specified and we use the config address [2].
Which at least is more explicit, but it is a logic change. Just shows that the address type='network' stuff needs more test coverage at least. I think at some point we should reject bare type='network' if it doesn't have a @network attribute
I agree that this configuration should be rejected, but I would wait for Peter's patches about the validation domain XMLs.
If that change was intentional it should be an additive patch after this cleanup, with a test case
+ rc = networkGetNetworkAddress(glisten->network, &glisten->address); + if (rc <= -2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("network-based listen isn't possible, " + "network driver isn't present")); + return -1; + } + if (rc < 0) + return -1; + + return 0; +} + + +static int qemuProcessSetupGraphics(virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int flags) @@ -4429,12 +4455,29 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, for (j = 0; j < graphics->nListens; j++) { virDomainGraphicsListenDefPtr glisten = &graphics->listens[j];
- if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS && - !glisten->address && listenAddr) { + switch (glisten->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + if (glisten->address || !listenAddr) + continue; + if (VIR_STRDUP(glisten->address, listenAddr) < 0) goto cleanup;
glisten->fromConfig = true; + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + if (glisten->address || !listenAddr) + continue; +
Can listenAddr ever be NULL? I know the logic is pre-existing, but I think the check is redundant. Particularly so for this case if the bit I mention above is changed
You're probably right, QEMU supports VNC, SPICE and SDL but only VNC, SPICE and RDP have listens so currently it cannot be NULL in this place, but I would rather leave the check here just in case. Pavel

On Fri, May 20, 2016 at 09:59:43AM +0200, Pavel Hrdina wrote:
This is a logic change. Previously we accept this XML
<graphics ...> <listen type='network'/> </graphics>
and silently treat that as using vnc_listen/spice_listen. Now we stick that address in the XML like
<graphics ...> <listen type='network' address='$vnc_listen'/> </graphics>
This isn't a logic change, it only improves the live XML, so the only change here is that the address will appear in the live XML.
It would be nice to mention that in the commit message. Jan
If you look at the old code we've always used the config listen address if there wasn't any address resolved for listen type network. If you look at the switch [1] we jump out from the switch in case there is no network specified and we use the config address [2].

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 93 +++++++++++++++++++++++++++++++------------------ 1 file changed, 59 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 75c8e53..2019c73 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4414,13 +4414,69 @@ qemuProcessGraphicsSetupNetworkAddress(virDomainGraphicsListenDefPtr glisten, static int +qemuProcessGraphicsSetupListen(virQEMUDriverConfigPtr cfg, + virDomainGraphicsDefPtr graphics) +{ + char *listenAddr = NULL; + size_t i; + + switch (graphics->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + listenAddr = cfg->vncListen; + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + listenAddr = cfg->spiceListen; + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: + break; + } + + for (i = 0; i < graphics->nListens; i++) { + virDomainGraphicsListenDefPtr glisten = &graphics->listens[i]; + + switch (glisten->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + if (glisten->address || !listenAddr) + continue; + + if (VIR_STRDUP(glisten->address, listenAddr) < 0) + return -1; + + glisten->fromConfig = true; + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + if (glisten->address || !listenAddr) + continue; + + if (qemuProcessGraphicsSetupNetworkAddress(glisten, + listenAddr) < 0) + return -1; + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: + break; + } + } + + return 0; +} + + +static int qemuProcessSetupGraphics(virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int flags) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); bool allocate = !(flags & VIR_QEMU_PROCESS_START_PRETEND); - size_t i, j; + size_t i; int ret = -1; if (allocate && qemuProcessGraphicsReservePorts(driver, vm) < 0) @@ -4428,21 +4484,16 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, for (i = 0; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; - char *listenAddr = NULL; switch (graphics->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: if (qemuProcessVNCAllocatePorts(driver, graphics, allocate) < 0) goto cleanup; - - listenAddr = cfg->vncListen; break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: if (qemuProcessSPICEAllocatePorts(driver, cfg, graphics, allocate) < 0) goto cleanup; - - listenAddr = cfg->spiceListen; break; case VIR_DOMAIN_GRAPHICS_TYPE_SDL: @@ -4452,34 +4503,8 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, break; } - for (j = 0; j < graphics->nListens; j++) { - virDomainGraphicsListenDefPtr glisten = &graphics->listens[j]; - - switch (glisten->type) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - if (glisten->address || !listenAddr) - continue; - - if (VIR_STRDUP(glisten->address, listenAddr) < 0) - goto cleanup; - - glisten->fromConfig = true; - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - if (glisten->address || !listenAddr) - continue; - - if (qemuProcessGraphicsSetupNetworkAddress(glisten, - listenAddr) < 0) - goto cleanup; - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: - break; - } - } + if (qemuProcessGraphicsSetupListen(cfg, graphics) < 0) + goto cleanup; } ret = 0; -- 2.8.2

On 05/19/2016 07:35 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 93 +++++++++++++++++++++++++++++++------------------ 1 file changed, 59 insertions(+), 34 deletions(-)
ACK - Cole

This prepares the code for other listen types. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 57 ++++++++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ee43e21..e401ac5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7500,10 +7500,11 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, { virBuffer opt = VIR_BUFFER_INITIALIZER; virDomainGraphicsListenDefPtr glisten = NULL; - int defaultMode = graphics->data.spice.defaultMode; int port = graphics->data.spice.port; int tlsPort = graphics->data.spice.tlsPort; size_t i; + bool hasSecure = false; + bool hasInsecure = false; if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -7513,8 +7514,10 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, glisten = virDomainGraphicsGetListen(graphics, 0); - if (port > 0) + if (port > 0) { virBufferAsprintf(&opt, "port=%u,", port); + hasInsecure = true; + } if (tlsPort > 0) { if (!cfg->spiceTLS) { @@ -7524,6 +7527,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, goto error; } virBufferAsprintf(&opt, "tls-port=%u,", tlsPort); + hasSecure = true; } if (port > 0 || tlsPort > 0) { @@ -7561,17 +7565,30 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, !cfg->spicePassword) virBufferAddLit(&opt, "disable-ticketing,"); - if (tlsPort > 0) + if (hasSecure) virBufferAsprintf(&opt, "x509-dir=%s,", cfg->spiceTLSx509certdir); - switch (defaultMode) { + switch (graphics->data.spice.defaultMode) { case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + if (!hasSecure) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice defaultMode secure requested in XML " + "configuration, but TLS is not available")); + goto error; + } virBufferAddLit(&opt, "tls-channel=default,"); break; case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + if (!hasInsecure) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice defaultMode insecure requested in XML " + "configuration, but plaintext is not available")); + goto error; + } virBufferAddLit(&opt, "plaintext-channel=default,"); break; case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_LAST: /* nothing */ break; } @@ -7579,10 +7596,10 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, for (i = 0; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST; i++) { switch (graphics->data.spice.channels[i]) { case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - if (tlsPort <= 0) { + if (!hasSecure) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice secure channels set in XML configuration, " - "but TLS port is not provided")); + _("spice secure channels set in XML " + "configuration, but TLS is not available")); goto error; } virBufferAsprintf(&opt, "tls-channel=%s,", @@ -7590,10 +7607,10 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, break; case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - if (port <= 0) { + if (!hasInsecure) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("spice insecure channels set in XML " - "configuration, but plain port is not provided")); + "configuration, but plaintext is not available")); goto error; } virBufferAsprintf(&opt, "plaintext-channel=%s,", @@ -7601,29 +7618,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, break; case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: - switch (defaultMode) { - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - if (tlsPort <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice defaultMode secure requested in XML " - "configuration but TLS port not provided")); - goto error; - } - break; - - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - if (port <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice defaultMode insecure requested in XML " - "configuration but plain port not provided")); - goto error; - } - break; - - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: - /* don't care */ break; - } } } -- 2.8.2

On 05/19/2016 07:35 AM, Pavel Hrdina wrote:
This prepares the code for other listen types.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 57 ++++++++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ee43e21..e401ac5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7500,10 +7500,11 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, { virBuffer opt = VIR_BUFFER_INITIALIZER; virDomainGraphicsListenDefPtr glisten = NULL; - int defaultMode = graphics->data.spice.defaultMode; int port = graphics->data.spice.port; int tlsPort = graphics->data.spice.tlsPort; size_t i; + bool hasSecure = false; + bool hasInsecure = false;
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -7513,8 +7514,10 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
glisten = virDomainGraphicsGetListen(graphics, 0);
- if (port > 0) + if (port > 0) { virBufferAsprintf(&opt, "port=%u,", port); + hasInsecure = true; + }
if (tlsPort > 0) { if (!cfg->spiceTLS) { @@ -7524,6 +7527,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, goto error; } virBufferAsprintf(&opt, "tls-port=%u,", tlsPort); + hasSecure = true; }
if (port > 0 || tlsPort > 0) { @@ -7561,17 +7565,30 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, !cfg->spicePassword) virBufferAddLit(&opt, "disable-ticketing,");
- if (tlsPort > 0) + if (hasSecure) virBufferAsprintf(&opt, "x509-dir=%s,", cfg->spiceTLSx509certdir);
- switch (defaultMode) { + switch (graphics->data.spice.defaultMode) { case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + if (!hasSecure) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice defaultMode secure requested in XML " + "configuration, but TLS is not available")); + goto error; + } virBufferAddLit(&opt, "tls-channel=default,"); break; case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + if (!hasInsecure) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice defaultMode insecure requested in XML " + "configuration, but plaintext is not available")); + goto error; + } virBufferAddLit(&opt, "plaintext-channel=default,"); break; case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_LAST: /* nothing */ break; } @@ -7579,10 +7596,10 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, for (i = 0; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST; i++) { switch (graphics->data.spice.channels[i]) { case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - if (tlsPort <= 0) { + if (!hasSecure) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice secure channels set in XML configuration, " - "but TLS port is not provided")); + _("spice secure channels set in XML " + "configuration, but TLS is not available")); goto error; }
I kinda prefer the original messages mentioning the lack of port as the culprit. So maybe plaintext port and TLS port? If I saw the 'plaintext' error as is I know I'd be confused and go digging into the code to figure out what was triggering it. ACK otherwise. IMO these first 8 patches are worth pushing and we work out any additional stuff like the type='network' weirdness, and additional test cases, on top of these cleanups - Cole

On Thu, May 19, 2016 at 05:37:43PM -0400, Cole Robinson wrote:
On 05/19/2016 07:35 AM, Pavel Hrdina wrote:
This prepares the code for other listen types.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 57 ++++++++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ee43e21..e401ac5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7500,10 +7500,11 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, { virBuffer opt = VIR_BUFFER_INITIALIZER; virDomainGraphicsListenDefPtr glisten = NULL; - int defaultMode = graphics->data.spice.defaultMode; int port = graphics->data.spice.port; int tlsPort = graphics->data.spice.tlsPort; size_t i; + bool hasSecure = false; + bool hasInsecure = false;
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -7513,8 +7514,10 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
glisten = virDomainGraphicsGetListen(graphics, 0);
- if (port > 0) + if (port > 0) { virBufferAsprintf(&opt, "port=%u,", port); + hasInsecure = true; + }
if (tlsPort > 0) { if (!cfg->spiceTLS) { @@ -7524,6 +7527,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, goto error; } virBufferAsprintf(&opt, "tls-port=%u,", tlsPort); + hasSecure = true; }
if (port > 0 || tlsPort > 0) { @@ -7561,17 +7565,30 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, !cfg->spicePassword) virBufferAddLit(&opt, "disable-ticketing,");
- if (tlsPort > 0) + if (hasSecure) virBufferAsprintf(&opt, "x509-dir=%s,", cfg->spiceTLSx509certdir);
- switch (defaultMode) { + switch (graphics->data.spice.defaultMode) { case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + if (!hasSecure) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice defaultMode secure requested in XML " + "configuration, but TLS is not available")); + goto error; + } virBufferAddLit(&opt, "tls-channel=default,"); break; case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + if (!hasInsecure) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice defaultMode insecure requested in XML " + "configuration, but plaintext is not available")); + goto error; + } virBufferAddLit(&opt, "plaintext-channel=default,"); break; case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_LAST: /* nothing */ break; } @@ -7579,10 +7596,10 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, for (i = 0; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST; i++) { switch (graphics->data.spice.channels[i]) { case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - if (tlsPort <= 0) { + if (!hasSecure) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice secure channels set in XML configuration, " - "but TLS port is not provided")); + _("spice secure channels set in XML " + "configuration, but TLS is not available")); goto error; }
I kinda prefer the original messages mentioning the lack of port as the culprit. So maybe plaintext port and TLS port? If I saw the 'plaintext' error as is I know I'd be confused and go digging into the code to figure out what was triggering it.
I've just come up with a little improvement, instead of port I'll use connection. It cannot refer to a port, because those error messages could be printed also for listen type socket or none, where we don't have any ports.
ACK otherwise. IMO these first 8 patches are worth pushing and we work out any additional stuff like the type='network' weirdness, and additional test cases, on top of these cleanups
Thanks, I've fixed some of the nits you've pointed out and I'll push those patches. Pavel

Introduce a new listen type that will be used to tell a graphics device to listen on unix socket and use it for VNC graphics instead of socket attribute. The socket attribute will remain in the XML for backward compatibility. Since old libvirt supports 'socket' attribute inside 'graphics' element for socket path provided by user libvirt will generate migratable XML without that listen type='socket' but only with 'socket' attribute in order to be able to migrate back to old libvirt. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 16 +++ docs/schemas/domaincommon.rng | 10 ++ src/conf/domain_conf.c | 152 ++++++++++++++++++--- src/conf/domain_conf.h | 8 +- src/libvirt_private.syms | 1 + src/qemu/qemu.conf | 6 +- src/qemu/qemu_command.c | 58 ++++---- src/qemu/qemu_domain.c | 28 ++-- src/qemu/qemu_hotplug.c | 9 ++ src/qemu/qemu_parse_command.c | 2 +- src/qemu/qemu_process.c | 45 ++++-- src/security/virt-aa-helper.c | 15 +- ...ric-graphics-vnc-socket-attr-listen-address.xml | 30 ++++ ...hics-vnc-socket-attr-listen-socket-mismatch.xml | 30 ++++ ...eric-graphics-vnc-socket-attr-listen-socket.xml | 30 ++++ ...ric-graphics-vnc-socket-attr-listen-address.xml | 30 ++++ ...eric-graphics-vnc-socket-attr-listen-socket.xml | 30 ++++ .../generic-graphics-vnc-socket-listen.xml | 4 +- .../generic-graphics-vnc-socket.xml | 4 +- tests/genericxml2xmltest.c | 4 + .../qemuargv2xml-graphics-vnc-socket.xml | 4 +- .../qemuxml2argv-graphics-vnc-auto-socket.args | 20 +++ .../qemuxml2argv-graphics-vnc-auto-socket.xml | 30 ++++ .../qemuxml2argv-graphics-vnc-socket.args | 4 +- .../qemuxml2argv-graphics-vnc-socket.xml | 10 +- tests/qemuxml2argvtest.c | 2 + .../qemuxml2xmlout-graphics-vnc-auto-socket.xml | 35 +++++ ...graphics-vnc-remove-generated-socket-active.xml | 4 +- .../qemuxml2xmlout-graphics-vnc-socket.xml | 35 +++++ tests/qemuxml2xmltest.c | 2 + 30 files changed, 564 insertions(+), 94 deletions(-) create mode 100644 tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-address.xml create mode 100644 tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-socket-mismatch.xml create mode 100644 tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-socket.xml create mode 100644 tests/genericxml2xmloutdata/generic-graphics-vnc-socket-attr-listen-address.xml create mode 100644 tests/genericxml2xmloutdata/generic-graphics-vnc-socket-attr-listen-socket.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-auto-socket.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-socket.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1336c82..3b0febc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5343,6 +5343,22 @@ qemu-kvm -net nic,model=? /dev/null of the first forward dev will be used. </p> </dd> + <dt><code>socket</code> <span class="since">since 1.3.5</span></dt> + <dd> + <p> + This listen type tells a graphics server to listen on unix socket. + Attribute <code>socket</code> contains a path to unix socket. If this + attribute is omitted libvirt will generate this path for you. + Supported by graphics type <code>vnc</code>. + </p> + <p> + For <code>vnc</code> graphics be backward compatible + the <code>socket</code> attribute of first <code>listen</code> element + is duplicated as <code>socket</code> attribute in <code>graphics</code> + element. If <code>graphics</code> element contains a <code>socket</code> + attribute all <code>listen</code> elements are ignored. + </p> + </dd> </dl> <h4><a name="elementsVideo">Video devices</a></h4> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 903fd7e..60f9f52 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2971,6 +2971,16 @@ </attribute> </optional> </group> + <group> + <attribute name="type"> + <value>socket</value> + </attribute> + <optional> + <attribute name="socket"> + <ref name="absFilePath"/> + </attribute> + </optional> + </group> </choice> </element> </zeroOrMore> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b3b60f1..c635238 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -561,7 +561,8 @@ VIR_ENUM_IMPL(virDomainGraphics, VIR_DOMAIN_GRAPHICS_TYPE_LAST, VIR_ENUM_IMPL(virDomainGraphicsListen, VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST, "none", "address", - "network") + "network", + "socket") VIR_ENUM_IMPL(virDomainGraphicsAuthConnected, VIR_DOMAIN_GRAPHICS_AUTH_CONNECTED_LAST, @@ -1229,6 +1230,7 @@ virDomainGraphicsListenDefClear(virDomainGraphicsListenDefPtr def) VIR_FREE(def->address); VIR_FREE(def->network); + VIR_FREE(def->socket); return; } @@ -1242,7 +1244,6 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) switch (def->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - VIR_FREE(def->data.vnc.socket); VIR_FREE(def->data.vnc.keymap); virDomainGraphicsAuthDefClear(&def->data.vnc.auth); break; @@ -10660,6 +10661,7 @@ virDomainGraphicsAuthDefParseXML(xmlNodePtr node, * virDomainGraphicsListenDefParseXML: * @def: listen def pointer to be filled * @node: xml node of <listen/> element + * * @parent: xml node of <graphics/> element * @flags: bit-wise or of VIR_DOMAIN_DEF_PARSE_* * @@ -10671,6 +10673,7 @@ virDomainGraphicsAuthDefParseXML(xmlNodePtr node, static int virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, xmlNodePtr node, + virDomainGraphicsDefPtr graphics, xmlNodePtr parent, unsigned int flags) { @@ -10678,12 +10681,17 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, char *type = virXMLPropString(node, "type"); char *address = virXMLPropString(node, "address"); char *network = virXMLPropString(node, "network"); + char *socket = virXMLPropString(node, "socket"); char *fromConfig = virXMLPropString(node, "fromConfig"); char *addressCompat = NULL; + char *socketCompat = NULL; + const char *graphicsType = virDomainGraphicsTypeToString(graphics->type); int tmp, typeVal; - if (parent) + if (parent) { addressCompat = virXMLPropString(parent, "listen"); + socketCompat = virXMLPropString(parent, "socket"); + } if (!type) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -10698,6 +10706,14 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, } def->type = typeVal; + if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET && + graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("listen type 'socket' is not available for " + "graphics type '%s'"), graphicsType); + goto error; + } + if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS) { if (address && addressCompat && STRNEQ(address, addressCompat)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -10713,6 +10729,21 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, } } + if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET) { + if (socket && socketCompat && STRNEQ(socket, socketCompat)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("graphics 'socket' attribute '%s' must match " + "'socket' attribute of first listen element " + "(found '%s')"), socketCompat, socket); + goto error; + } + + if (!socket) { + socket = socketCompat; + socketCompat = NULL; + } + } + if (address && address[0] && (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS || (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK && @@ -10732,6 +10763,17 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, network = NULL; } + if (socket && socket[0]) { + if (def->type != VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'socket' attribute is valid only for listen " + "type 'socket'")); + goto error; + } + def->socket = socket; + socket = NULL; + } + if (fromConfig && flags & VIR_DOMAIN_DEF_PARSE_STATUS) { if (virStrToLong_i(fromConfig, NULL, 10, &tmp) < 0) { @@ -10750,8 +10792,10 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, VIR_FREE(type); VIR_FREE(address); VIR_FREE(network); + VIR_FREE(socket); VIR_FREE(fromConfig); VIR_FREE(addressCompat); + VIR_FREE(socketCompat); return ret; } @@ -10771,12 +10815,6 @@ virDomainGraphicsListensParseXML(virDomainGraphicsDefPtr def, ctxt->node = node; - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - (socketPath = virXMLPropString(node, "socket"))) { - ret = 0; - goto error; - } - /* parse the <listen> subelements for graphics types that support it */ nListens = virXPathNodeSet("./listen", ctxt, &listenNodes); if (nListens < 0) @@ -10791,6 +10829,7 @@ virDomainGraphicsListensParseXML(virDomainGraphicsDefPtr def, for (i = 0; i < nListens; i++) { if (virDomainGraphicsListenDefParseXML(&def->listens[i], listenNodes[i], + def, i == 0 ? node : NULL, flags) < 0) goto error; @@ -10798,16 +10837,43 @@ virDomainGraphicsListensParseXML(virDomainGraphicsDefPtr def, def->nListens++; } VIR_FREE(listenNodes); + } + + /* If no <listen/> element was found in XML for backward compatibility + * we should try to parse 'listen' or 'socket' attribute from <graphics/> + * element. */ + if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) + socketPath = virXMLPropString(node, "socket"); + + if (socketPath) { + newListen.type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET; + newListen.socket = socketPath; + socketPath = NULL; } else { - /* If no <listen/> element was found in XML for backward compatibility - * we should try to parse 'listen' attribute from <graphics/> element. */ newListen.type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; newListen.address = virXMLPropString(node, "listen"); if (STREQ_NULLABLE(newListen.address, "")) VIR_FREE(newListen.address); + } + /* If no <listen/> element was found add a new one created by parsing + * <graphics/> element. */ + if (def->nListens == 0) { if (VIR_APPEND_ELEMENT(def->listens, def->nListens, newListen) < 0) goto error; + } else { + virDomainGraphicsListenDefPtr glisten = &def->listens[0]; + + /* If the first <listen/> element is 'address' or 'network' and we found + * 'socket' attribute inside <graphics/> element for backward + * compatibility we need to replace the first listen by + * <listen type='socket' .../> element based on the 'socket' attribite. */ + if ((glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS || + glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK) && + newListen.type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET) { + virDomainGraphicsListenDefClear(glisten); + *glisten = newListen; + } } ret = 0; @@ -10885,7 +10951,6 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr def, } } - def->data.vnc.socket = virXMLPropString(node, "socket"); def->data.vnc.keymap = virXMLPropString(node, "keymap"); if (virDomainGraphicsAuthDefParseXML(node, &def->data.vnc.auth, @@ -21390,6 +21455,13 @@ virDomainGraphicsListenDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " network='%s'", def->network); } + if (def->socket && + def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET && + !(def->autogenerated && + (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE))) { + virBufferEscapeString(buf, " socket='%s'", def->socket); + } + if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) virBufferAsprintf(buf, " fromConfig='%d'", def->fromConfig); @@ -21447,11 +21519,17 @@ virDomainGraphicsDefFormat(virBufferPtr buf, switch (def->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - if (def->data.vnc.socket) { - if (!def->data.vnc.socketAutogenerated || - !(flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE)) { - virBufferEscapeString(buf, " socket='%s'", - def->data.vnc.socket); + if (!glisten) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing listen element for graphics")); + return -1; + } + + if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET) { + if (glisten->socket && + !((glisten->autogenerated || glisten->fromConfig) && + (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE))) { + virBufferEscapeString(buf, " socket='%s'", glisten->socket); } } else { if (def->data.vnc.port && @@ -21557,9 +21635,19 @@ virDomainGraphicsDefFormat(virBufferPtr buf, for (i = 0; i < def->nListens; i++) { if (def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE) continue; - if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE && - def->listens[i].fromConfig) - continue; + if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) { + if (def->listens[i].fromConfig) + continue; + + /* If the socket is provided by user in the XML we need to skip this + * listen type to support migration back to old libvirt since old + * libvirt supports specifying socket path inside graphics element + * as 'socket' attribute. */ + if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET && + !def->listens[i].autogenerated) + continue; + } if (!children) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); @@ -23873,6 +23961,30 @@ virDomainGraphicsListenAppendAddress(virDomainGraphicsDefPtr def, } +int +virDomainGraphicsListenAppendSocket(virDomainGraphicsDefPtr def, + const char *socket) +{ + virDomainGraphicsListenDef listen; + + memset(&listen, 0, sizeof(listen)); + + listen.type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET; + + if (VIR_STRDUP(listen.socket, socket) < 0) + goto error; + + if (VIR_APPEND_ELEMENT_COPY(def->listens, def->nListens, listen) < 0) + goto error; + + return 0; + + error: + VIR_FREE(listen.socket); + return -1; +} + + /** * virDomainNetFind: * @def: domain's def diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4e21826..53696a0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1539,6 +1539,7 @@ typedef enum { VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE = 0, VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS, VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK, + VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET, VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST } virDomainGraphicsListenType; @@ -1555,7 +1556,9 @@ struct _virDomainGraphicsListenDef { virDomainGraphicsListenType type; char *address; char *network; + char *socket; bool fromConfig; /* true if the @address is config file originated */ + bool autogenerated; }; struct _virDomainGraphicsDef { @@ -1572,8 +1575,6 @@ struct _virDomainGraphicsDef { int websocket; bool autoport; char *keymap; - char *socket; - bool socketAutogenerated; virDomainGraphicsAuthDef auth; int sharePolicy; } vnc; @@ -2849,6 +2850,9 @@ virDomainGraphicsGetListen(virDomainGraphicsDefPtr def, size_t i); int virDomainGraphicsListenAppendAddress(virDomainGraphicsDefPtr def, const char *address) ATTRIBUTE_NONNULL(1); +int virDomainGraphicsListenAppendSocket(virDomainGraphicsDefPtr def, + const char *socket) + ATTRIBUTE_NONNULL(1); int virDomainNetGetActualType(virDomainNetDefPtr iface); const char *virDomainNetGetActualBridgeName(virDomainNetDefPtr iface); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 868bcfa..1b25c9b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -305,6 +305,7 @@ virDomainGraphicsAuthConnectedTypeToString; virDomainGraphicsDefFree; virDomainGraphicsGetListen; virDomainGraphicsListenAppendAddress; +virDomainGraphicsListenAppendSocket; virDomainGraphicsSpiceChannelModeTypeFromString; virDomainGraphicsSpiceChannelModeTypeToString; virDomainGraphicsSpiceChannelNameTypeFromString; diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 4fa5e8a..59c839e 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -15,9 +15,9 @@ # unix socket. This prevents unprivileged access from users on the # host machine, though most VNC clients do not support it. # -# This will only be enabled for VNC configurations that do not have -# a hardcoded 'listen' or 'socket' value. This setting takes preference -# over vnc_listen. +# This will only be enabled for VNC configurations that have listen +# type=address but without any address specified. This setting takes +# preference over vnc_listen. # #vnc_auto_unix_socket = 1 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e401ac5..d41935e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7379,8 +7379,7 @@ static int qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virCommandPtr cmd, virQEMUCapsPtr qemuCaps, - virDomainGraphicsDefPtr graphics, - const char *domainLibDir) + virDomainGraphicsDefPtr graphics) { virBuffer opt = VIR_BUFFER_INITIALIZER; virDomainGraphicsListenDefPtr glisten = NULL; @@ -7392,21 +7391,20 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, goto error; } - glisten = virDomainGraphicsGetListen(graphics, 0); - - if (graphics->data.vnc.socket || cfg->vncAutoUnixSocket) { - if (!graphics->data.vnc.socket) { - if (virAsprintf(&graphics->data.vnc.socket, - "%s/vnc.sock", domainLibDir) < 0) - goto error; - - graphics->data.vnc.socketAutogenerated = true; - } + if (!(glisten = virDomainGraphicsGetListen(graphics, 0))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing listen element")); + goto error; + } + switch (glisten->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: virBufferAddLit(&opt, "unix:"); - qemuBufferEscapeComma(&opt, graphics->data.vnc.socket); + qemuBufferEscapeComma(&opt, glisten->socket); + break; - } else { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: if (!graphics->data.vnc.autoport && (graphics->data.vnc.port < 5900 || graphics->data.vnc.port > 65535)) { @@ -7415,7 +7413,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, goto error; } - if (glisten && glisten->address) { + if (glisten->address) { escapeAddr = strchr(glisten->address, ':') != NULL; if (escapeAddr) virBufferAsprintf(&opt, "[%s]", glisten->address); @@ -7424,17 +7422,21 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, } virBufferAsprintf(&opt, ":%d", graphics->data.vnc.port - 5900); - } - if (!graphics->data.vnc.socket && - graphics->data.vnc.websocket) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("VNC WebSockets are not supported " - "with this QEMU binary")); - goto error; + if (graphics->data.vnc.websocket) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("VNC WebSockets are not supported " + "with this QEMU binary")); + goto error; + } + virBufferAsprintf(&opt, ",websocket=%d", graphics->data.vnc.websocket); } - virBufferAsprintf(&opt, ",websocket=%d", graphics->data.vnc.websocket); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: + break; } if (graphics->data.vnc.sharePolicy) { @@ -7701,8 +7703,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, virCommandPtr cmd, virDomainDefPtr def, virQEMUCapsPtr qemuCaps, - virDomainGraphicsDefPtr graphics, - const char *domainLibDir) + virDomainGraphicsDefPtr graphics) { switch (graphics->type) { case VIR_DOMAIN_GRAPHICS_TYPE_SDL: @@ -7734,8 +7735,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, break; case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - return qemuBuildGraphicsVNCCommandLine(cfg, cmd, qemuCaps, - graphics, domainLibDir); + return qemuBuildGraphicsVNCCommandLine(cfg, cmd, qemuCaps, graphics); case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: return qemuBuildGraphicsSPICECommandLine(cfg, cmd, qemuCaps, graphics); @@ -9463,7 +9463,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, for (i = 0; i < def->ngraphics; ++i) { if (qemuBuildGraphicsCommandLine(cfg, cmd, def, qemuCaps, - def->graphics[i], domainLibDir) < 0) + def->graphics[i]) < 0) goto error; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 65dfa37..31b28a7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2001,20 +2001,30 @@ qemuDomainRecheckInternalPaths(virDomainDefPtr def, unsigned int flags) { size_t i = 0; + size_t j = 0; for (i = 0; i < def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = def->graphics[i]; - if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - graphics->data.vnc.socket && - STRPREFIX(graphics->data.vnc.socket, cfg->libDir)) { - if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) { - VIR_FREE(graphics->data.vnc.socket); - if (virDomainGraphicsListenAppendAddress(graphics, NULL) < 0) - return -1; + for (j = 0; j < graphics->nListens; ++j) { + virDomainGraphicsListenDefPtr glisten = &graphics->listens[j]; + + /* This will happen only if we parse XML from old libvirts where + * unix socket was available only for VNC graphics. In this + * particular case we should follow the behavior and if we remove + * the auto-generated socket we need to change the listen type to + * address. */ + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET && + glisten->socket && + STRPREFIX(glisten->socket, cfg->libDir)) { + if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) { + VIR_FREE(glisten->socket); + glisten->type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; + } else { + glisten->autogenerated = true; + } } - else - graphics->data.vnc.socketAutogenerated = true; } } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index aa897d2..acf49d0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2662,6 +2662,15 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, break; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: + if (STRNEQ_NULLABLE(newlisten->socket, oldlisten->socket)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("cannot change listen socket setting " + "on '%s' graphics"), type); + goto cleanup; + } + break; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: /* nada */ diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index e30586f..a813705 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -509,7 +509,7 @@ qemuParseCommandLineVnc(virDomainDefPtr def, if (STRPREFIX(val, "unix:")) { /* -vnc unix:/some/big/path */ - if (VIR_STRDUP(vnc->data.vnc.socket, val + 5) < 0) + if (virDomainGraphicsListenAppendSocket(vnc, val + 5) < 0) goto cleanup; } else { /* diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2019c73..096c025 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3863,9 +3863,6 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver, { unsigned short port; - if (graphics->data.vnc.socket) - return 0; - if (!allocate) { if (graphics->data.vnc.autoport) graphics->data.vnc.port = 5900; @@ -4415,13 +4412,18 @@ qemuProcessGraphicsSetupNetworkAddress(virDomainGraphicsListenDefPtr glisten, static int qemuProcessGraphicsSetupListen(virQEMUDriverConfigPtr cfg, - virDomainGraphicsDefPtr graphics) + virDomainGraphicsDefPtr graphics, + virDomainObjPtr vm) { + qemuDomainObjPrivatePtr priv = vm->privateData; + const char *type = virDomainGraphicsTypeToString(graphics->type); char *listenAddr = NULL; + bool useSocket = false; size_t i; switch (graphics->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + useSocket = cfg->vncAutoUnixSocket; listenAddr = cfg->vncListen; break; @@ -4441,13 +4443,23 @@ qemuProcessGraphicsSetupListen(virQEMUDriverConfigPtr cfg, switch (glisten->type) { case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - if (glisten->address || !listenAddr) - continue; - - if (VIR_STRDUP(glisten->address, listenAddr) < 0) - return -1; - - glisten->fromConfig = true; + if (!glisten->address) { + /* If there is no address specified and qemu.conf has + * *_auto_unix_socket set we should use unix socket as + * default instead of tcp listen. */ + if (useSocket) { + memset(glisten, 0, sizeof(virDomainGraphicsListenDef)); + if (virAsprintf(&glisten->socket, "%s/%s.sock", + priv->libDir, type) < 0) + return -1; + glisten->fromConfig = true; + glisten->type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET; + } else if (listenAddr) { + if (VIR_STRDUP(glisten->address, listenAddr) < 0) + return -1; + glisten->fromConfig = true; + } + } break; case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: @@ -4459,6 +4471,15 @@ qemuProcessGraphicsSetupListen(virQEMUDriverConfigPtr cfg, return -1; break; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: + if (!glisten->socket) { + if (virAsprintf(&glisten->socket, "%s/%s.sock", + priv->libDir, type) < 0) + return -1; + glisten->autogenerated = true; + } + break; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: break; @@ -4503,7 +4524,7 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, break; } - if (qemuProcessGraphicsSetupListen(cfg, graphics) < 0) + if (qemuProcessGraphicsSetupListen(cfg, graphics, vm) < 0) goto cleanup; } diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 537e89d..cbe09af 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1002,10 +1002,17 @@ get_files(vahControl * ctl) goto cleanup; for (i = 0; i < ctl->def->ngraphics; i++) { - if (ctl->def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - ctl->def->graphics[i]->data.vnc.socket && - vah_add_file(&buf, ctl->def->graphics[i]->data.vnc.socket, "w")) - goto cleanup; + virDomainGraphicsDefPtr graphics = ctl->def->graphics[i]; + size_t n; + + for (n = 0; n < graphics->nListens; n++) { + virDomainGraphicsListenDef listenObj = graphics->listens[n]; + + if (listenObj.type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET && + listenObj.socket && + vah_add_file(&buf, listenObj.socket, "rw")) + goto cleanup; + } } if (ctl->def->ngraphics == 1 && diff --git a/tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-address.xml b/tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-address.xml new file mode 100644 index 0000000..a32c20b --- /dev/null +++ b/tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-address.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' socket='/tmp/vnc.sock'> + <listen type='address' address='0.0.0.0'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-socket-mismatch.xml b/tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-socket-mismatch.xml new file mode 100644 index 0000000..980b64c --- /dev/null +++ b/tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-socket-mismatch.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' socket='/tmp/vnc.sock'> + <listen type='socket' socket='/tmp/mismatch.sock'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-socket.xml b/tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-socket.xml new file mode 100644 index 0000000..ea3efca --- /dev/null +++ b/tests/genericxml2xmlindata/generic-graphics-vnc-socket-attr-listen-socket.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' socket='/tmp/vnc.sock'> + <listen type='socket' socket='/tmp/vnc.sock'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmloutdata/generic-graphics-vnc-socket-attr-listen-address.xml b/tests/genericxml2xmloutdata/generic-graphics-vnc-socket-attr-listen-address.xml new file mode 100644 index 0000000..f205e13 --- /dev/null +++ b/tests/genericxml2xmloutdata/generic-graphics-vnc-socket-attr-listen-address.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' socket='/tmp/vnc.sock'> + <listen type='socket' socket='/tmp/vnc.sock'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1' primary='yes'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmloutdata/generic-graphics-vnc-socket-attr-listen-socket.xml b/tests/genericxml2xmloutdata/generic-graphics-vnc-socket-attr-listen-socket.xml new file mode 100644 index 0000000..f205e13 --- /dev/null +++ b/tests/genericxml2xmloutdata/generic-graphics-vnc-socket-attr-listen-socket.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' socket='/tmp/vnc.sock'> + <listen type='socket' socket='/tmp/vnc.sock'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1' primary='yes'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmloutdata/generic-graphics-vnc-socket-listen.xml b/tests/genericxml2xmloutdata/generic-graphics-vnc-socket-listen.xml index d8742c6..cb4e5ac 100644 --- a/tests/genericxml2xmloutdata/generic-graphics-vnc-socket-listen.xml +++ b/tests/genericxml2xmloutdata/generic-graphics-vnc-socket-listen.xml @@ -19,7 +19,9 @@ <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <graphics type='vnc' socket='/tmp/QEMUGuest1-vnc.sock'/> + <graphics type='vnc' socket='/tmp/QEMUGuest1-vnc.sock'> + <listen type='socket' socket='/tmp/QEMUGuest1-vnc.sock'/> + </graphics> <video> <model type='cirrus' vram='16384' heads='1' primary='yes'/> </video> diff --git a/tests/genericxml2xmloutdata/generic-graphics-vnc-socket.xml b/tests/genericxml2xmloutdata/generic-graphics-vnc-socket.xml index d8742c6..cb4e5ac 100644 --- a/tests/genericxml2xmloutdata/generic-graphics-vnc-socket.xml +++ b/tests/genericxml2xmloutdata/generic-graphics-vnc-socket.xml @@ -19,7 +19,9 @@ <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <graphics type='vnc' socket='/tmp/QEMUGuest1-vnc.sock'/> + <graphics type='vnc' socket='/tmp/QEMUGuest1-vnc.sock'> + <listen type='socket' socket='/tmp/QEMUGuest1-vnc.sock'/> + </graphics> <video> <model type='cirrus' vram='16384' heads='1' primary='yes'/> </video> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index 70ecd2d..a193a33 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -87,6 +87,10 @@ mymain(void) DO_TEST_DIFFERENT("graphics-vnc-listen-attr-only"); DO_TEST_DIFFERENT("graphics-vnc-listen-element-minimal"); DO_TEST_DIFFERENT("graphics-vnc-listen-element-with-address"); + DO_TEST_DIFFERENT("graphics-vnc-socket-attr-listen-address"); + DO_TEST_DIFFERENT("graphics-vnc-socket-attr-listen-socket"); + DO_TEST_FULL("graphics-vnc-socket-attr-listen-socket-mismatch", 0, false, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_FULL("name-slash-parse", 0, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); diff --git a/tests/qemuargv2xmldata/qemuargv2xml-graphics-vnc-socket.xml b/tests/qemuargv2xmldata/qemuargv2xml-graphics-vnc-socket.xml index edbaab3..efd2601 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-graphics-vnc-socket.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-graphics-vnc-socket.xml @@ -29,7 +29,9 @@ </controller> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <graphics type='vnc' socket='/tmp/foo.socket'/> + <graphics type='vnc' socket='/tmp/foo.socket'> + <listen type='socket' socket='/tmp/foo.socket'/> + </graphics> <video> <model type='cirrus' vram='16384' heads='1' primary='yes'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket.args new file mode 100644 index 0000000..84ce727 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket.args @@ -0,0 +1,20 @@ +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 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-vnc unix:/tmp/lib/domain--1-QEMUGuest1/vnc.sock \ +-vga cirrus diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket.xml new file mode 100644 index 0000000..3e455df --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-socket.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc'> + <listen type='socket'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.args index 2464867..abf724c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.args @@ -16,7 +16,5 @@ QEMU_AUDIO_DRV=none \ -no-acpi \ -boot c \ -usb \ --drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ --device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ --vnc unix:/tmp/foo.socket \ +-vnc unix:/tmp/vnc.sock \ -vga cirrus diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml index de70bc4..522c3af 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml @@ -14,18 +14,14 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu</emulator> - <disk type='block' device='disk'> - <driver name='qemu' type='raw'/> - <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'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <graphics type='vnc' socket='/tmp/foo.socket'/> + <graphics type='vnc'> + <listen type='socket' socket='/tmp/vnc.sock'/> + </graphics> <video> <model type='cirrus' vram='16384' heads='1'/> </video> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index de96e19..da060d2 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -903,6 +903,8 @@ mymain(void) driver.config->vncAutoUnixSocket = true; DO_TEST("graphics-vnc-auto-socket-cfg", QEMU_CAPS_VNC); driver.config->vncAutoUnixSocket = false; + DO_TEST("graphics-vnc-socket", QEMU_CAPS_VNC); + DO_TEST("graphics-vnc-auto-socket", QEMU_CAPS_VNC); driver.config->vncSASL = 1; VIR_FREE(driver.config->vncSASLdir); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-auto-socket.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-auto-socket.xml new file mode 100644 index 0000000..e14bbd1 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-auto-socket.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc'> + <listen type='socket'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-remove-generated-socket-active.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-remove-generated-socket-active.xml index f3ccfdf..1aeffe8 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-remove-generated-socket-active.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-remove-generated-socket-active.xml @@ -29,7 +29,9 @@ <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <graphics type='vnc' socket='/tmp/lib/domain-99-QEMUGuest1/delete.this.socket'/> + <graphics type='vnc' socket='/tmp/lib/domain-99-QEMUGuest1/delete.this.socket'> + <listen type='socket' socket='/tmp/lib/domain-99-QEMUGuest1/delete.this.socket'/> + </graphics> <video> <model type='cirrus' vram='16384' heads='1' primary='yes'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-socket.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-socket.xml new file mode 100644 index 0000000..9a83328 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-socket.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' socket='/tmp/vnc.sock'> + <listen type='socket' socket='/tmp/vnc.sock'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 962e9ba..92372ee 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -437,6 +437,8 @@ mymain(void) cfg->vncAutoUnixSocket = true; DO_TEST("graphics-vnc-auto-socket-cfg"); cfg->vncAutoUnixSocket = false; + DO_TEST("graphics-vnc-socket"); + DO_TEST("graphics-vnc-auto-socket"); DO_TEST("graphics-sdl"); DO_TEST("graphics-sdl-fullscreen"); -- 2.8.2

Add a new capability to detect support of unix sockets for spice graphics. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 13427ed..3e13b9e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -329,6 +329,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "nec-usb-xhci-ports", "virtio-scsi-pci.iothread", "name-guest", + + "spice-unix", /* 225 */ ); @@ -2671,6 +2673,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "chardev", "logfile", QEMU_CAPS_CHARDEV_LOGFILE }, { "name", "debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS }, { "name", "guest", QEMU_CAPS_NAME_GUEST }, + { "spice", "unix", QEMU_CAPS_SPICE_UNIX }, }; static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index b6e0f8a..ad3f089 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -361,6 +361,9 @@ typedef enum { QEMU_CAPS_VIRTIO_SCSI_IOTHREAD, /* virtio-scsi-{pci,ccw}.iothread */ QEMU_CAPS_NAME_GUEST, /* -name guest= */ + /* 225 */ + QEMU_CAPS_SPICE_UNIX, /* -spice unix */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; -- 2.8.2

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1335832 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 2 +- src/conf/domain_conf.c | 31 ++++++++---- src/qemu/qemu_command.c | 56 ++++++++++++++++------ src/qemu/qemu_migration.c | 47 +++++++++++++----- .../qemuxml2argv-graphics-spice-auto-socket.args | 20 ++++++++ .../qemuxml2argv-graphics-spice-auto-socket.xml | 30 ++++++++++++ .../qemuxml2argv-graphics-spice-socket.args | 20 ++++++++ .../qemuxml2argv-graphics-spice-socket.xml | 30 ++++++++++++ tests/qemuxml2argvtest.c | 6 +++ .../qemuxml2xmlout-graphics-spice-auto-socket.xml | 35 ++++++++++++++ .../qemuxml2xmlout-graphics-spice-socket.xml | 35 ++++++++++++++ tests/qemuxml2xmltest.c | 2 + 12 files changed, 277 insertions(+), 37 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-auto-socket.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-socket.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3b0febc..fa8fac7 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5349,7 +5349,7 @@ qemu-kvm -net nic,model=? /dev/null This listen type tells a graphics server to listen on unix socket. Attribute <code>socket</code> contains a path to unix socket. If this attribute is omitted libvirt will generate this path for you. - Supported by graphics type <code>vnc</code>. + Supported by graphics type <code>vnc</code> and <code>spice</code>. </p> <p> For <code>vnc</code> graphics be backward compatible diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c635238..b72cfec 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10707,7 +10707,8 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, def->type = typeVal; if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET && - graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC && + graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("listen type 'socket' is not available for " "graphics type '%s'"), graphicsType); @@ -21604,18 +21605,28 @@ virDomainGraphicsDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - if (def->data.spice.port) - virBufferAsprintf(buf, " port='%d'", - def->data.spice.port); + switch (glisten->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + 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"); - virDomainGraphicsAddListenAddr(buf, glisten, flags); + virDomainGraphicsAddListenAddr(buf, glisten, flags); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: + break; + } if (def->data.spice.keymap) virBufferEscapeString(buf, " keymap='%s'", diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d41935e..d97b525 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7514,27 +7514,53 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, goto error; } - glisten = virDomainGraphicsGetListen(graphics, 0); - - if (port > 0) { - virBufferAsprintf(&opt, "port=%u,", port); - hasInsecure = true; + if (!(glisten = virDomainGraphicsGetListen(graphics, 0))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing listen element")); + goto error; } - if (tlsPort > 0) { - if (!cfg->spiceTLS) { + switch (glisten->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_UNIX)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice TLS port set in XML configuration," - " but TLS is disabled in qemu.conf")); + _("unix socket for spice graphics are not supported " + "with this QEMU")); goto error; } - virBufferAsprintf(&opt, "tls-port=%u,", tlsPort); - hasSecure = true; - } - if (port > 0 || tlsPort > 0) { - if (glisten && glisten->address) - virBufferAsprintf(&opt, "addr=%s,", glisten->address); + virBufferAsprintf(&opt, "unix,addr=%s,", glisten->socket); + hasInsecure = true; + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + if (port > 0) { + virBufferAsprintf(&opt, "port=%u,", port); + hasInsecure = true; + } + + if (tlsPort > 0) { + if (!cfg->spiceTLS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice TLS port set in XML configuration, " + "but TLS is disabled in qemu.conf")); + goto error; + } + virBufferAsprintf(&opt, "tls-port=%u,", tlsPort); + hasSecure = true; + } + + if (port > 0 || tlsPort > 0) { + if (glisten->address) + virBufferAsprintf(&opt, "addr=%s,", glisten->address); + } + + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: + break; } if (cfg->spiceSASL) { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 25093ac..c63341d 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -315,17 +315,17 @@ qemuDomainExtractTLSSubject(const char *certdir) static qemuMigrationCookieGraphicsPtr qemuMigrationCookieGraphicsSpiceAlloc(virQEMUDriverPtr driver, - virDomainGraphicsDefPtr def) + virDomainGraphicsDefPtr def, + virDomainGraphicsListenDefPtr glisten) { qemuMigrationCookieGraphicsPtr mig = NULL; const char *listenAddr; - virDomainGraphicsListenDefPtr glisten = virDomainGraphicsGetListen(def, 0); virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); if (VIR_ALLOC(mig) < 0) goto error; - mig->type = def->type; + mig->type = VIR_DOMAIN_GRAPHICS_TYPE_SPICE; mig->port = def->data.spice.port; if (cfg->spiceTLS) mig->tlsPort = def->data.spice.tlsPort; @@ -452,14 +452,39 @@ qemuMigrationCookieAddGraphics(qemuMigrationCookiePtr mig, } for (i = 0; i < dom->def->ngraphics; i++) { - if (dom->def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - if (!(mig->graphics = - qemuMigrationCookieGraphicsSpiceAlloc(driver, - dom->def->graphics[i]))) - return -1; - mig->flags |= QEMU_MIGRATION_COOKIE_GRAPHICS; - break; - } + if (dom->def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + virDomainGraphicsListenDefPtr glisten = + virDomainGraphicsGetListen(dom->def->graphics[i], 0); + + if (!glisten) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing listen element")); + return -1; + } + + switch (glisten->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + /* Seamless migration is supported only for listen types + * 'address and 'network'. */ + if (!(mig->graphics = + qemuMigrationCookieGraphicsSpiceAlloc(driver, + dom->def->graphics[i], + glisten))) + return -1; + mig->flags |= QEMU_MIGRATION_COOKIE_GRAPHICS; + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: + break; + } + + /* Seamless migration is supported only for one graphics. */ + if (mig->graphics) + break; + } } return 0; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args new file mode 100644 index 0000000..61335b0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.args @@ -0,0 +1,20 @@ +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 \ +-usb \ +-spice unix,addr=/tmp/lib/domain--1-QEMUGuest1/spice.sock \ +-vga cirrus diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.xml new file mode 100644 index 0000000..acb325a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice'> + <listen type='socket'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args new file mode 100644 index 0000000..26d0671 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.args @@ -0,0 +1,20 @@ +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 \ +-usb \ +-spice unix,addr=/tmp/spice.sock \ +-vga cirrus diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.xml new file mode 100644 index 0000000..13bbef1 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-socket.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice'> + <listen type='socket' socket='/tmp/spice.sock'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index da060d2..f7d01f9 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -968,6 +968,12 @@ mymain(void) QEMU_CAPS_DEVICE_QXL_VGA, QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_SPICE_FILE_XFER_DISABLE); + DO_TEST("graphics-spice-socket", + QEMU_CAPS_SPICE, + QEMU_CAPS_SPICE_UNIX); + DO_TEST("graphics-spice-auto-socket", + QEMU_CAPS_SPICE, + QEMU_CAPS_SPICE_UNIX); DO_TEST("input-usbmouse", NONE); DO_TEST("input-usbtablet", NONE); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-auto-socket.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-auto-socket.xml new file mode 100644 index 0000000..931ec0f --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-auto-socket.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice'> + <listen type='socket'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-socket.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-socket.xml new file mode 100644 index 0000000..dd672ed --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-socket.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice'> + <listen type='socket' socket='/tmp/spice.sock'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 92372ee..ab5a2da 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -445,6 +445,8 @@ mymain(void) DO_TEST("graphics-spice"); DO_TEST("graphics-spice-compression"); DO_TEST("graphics-spice-qxl-vga"); + DO_TEST("graphics-spice-socket"); + DO_TEST("graphics-spice-auto-socket"); DO_TEST("nographics-vga"); DO_TEST("input-usbmouse"); DO_TEST("input-usbtablet"); -- 2.8.2

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 11 +++++++ src/qemu/qemu_conf.c | 1 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_process.c | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + ...emuxml2argv-graphics-spice-auto-socket-cfg.args | 20 +++++++++++++ ...qemuxml2argv-graphics-spice-auto-socket-cfg.xml | 30 +++++++++++++++++++ tests/qemuxml2argvtest.c | 5 ++++ ...muxml2xmlout-graphics-spice-auto-socket-cfg.xml | 35 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 4 +++ 11 files changed, 110 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-auto-socket-cfg.xml diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index b6f6dc4..8bc23ba 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -37,6 +37,7 @@ module Libvirtd_qemu = let spice_entry = str_entry "spice_listen" | bool_entry "spice_tls" | str_entry "spice_tls_x509_cert_dir" + | bool_entry "spice_auto_unix_socket" | str_entry "spice_password" | bool_entry "spice_sasl" | str_entry "spice_sasl_dir" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 59c839e..7964273 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -130,6 +130,17 @@ #spice_tls_x509_cert_dir = "/etc/pki/libvirt-spice" +# 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 have listen +# type=address but without any address specified. This setting takes +# preference over spice_listen. +# +#spice_auto_unix_socket = 1 + + # The default SPICE password. This parameter is only used if the # per-domain XML config does not already provide a password. To # allow access without passwords, leave this commented out. An diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e00ddca..d4c34c9 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -588,6 +588,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/qemu_process.c b/src/qemu/qemu_process.c index 096c025..1e3781c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4428,6 +4428,7 @@ qemuProcessGraphicsSetupListen(virQEMUDriverConfigPtr cfg, break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + useSocket = cfg->spiceAutoUnixSocket; listenAddr = cfg->spiceListen; break; diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 8bec743..c4d4f19 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -14,6 +14,7 @@ module Test_libvirtd_qemu = { "spice_listen" = "0.0.0.0" } { "spice_tls" = "1" } { "spice_tls_x509_cert_dir" = "/etc/pki/libvirt-spice" } +{ "spice_auto_unix_socket" = "1" } { "spice_password" = "XYZ12345" } { "spice_sasl" = "1" } { "spice_sasl_dir" = "/some/directory/sasl2" } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.args new file mode 100644 index 0000000..61335b0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.args @@ -0,0 +1,20 @@ +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 \ +-usb \ +-spice unix,addr=/tmp/lib/domain--1-QEMUGuest1/spice.sock \ +-vga cirrus diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.xml new file mode 100644 index 0000000..f2e3d12 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-auto-socket-cfg.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' autoport='yes'> + <listen type='address'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index f7d01f9..da678d7 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -974,6 +974,11 @@ mymain(void) DO_TEST("graphics-spice-auto-socket", QEMU_CAPS_SPICE, QEMU_CAPS_SPICE_UNIX); + driver.config->spiceAutoUnixSocket = true; + DO_TEST("graphics-spice-auto-socket-cfg", + QEMU_CAPS_SPICE, + QEMU_CAPS_SPICE_UNIX); + driver.config->spiceAutoUnixSocket = false; DO_TEST("input-usbmouse", NONE); DO_TEST("input-usbtablet", NONE); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-auto-socket-cfg.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-auto-socket-cfg.xml new file mode 100644 index 0000000..7548184 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-auto-socket-cfg.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' autoport='yes'> + <listen type='address'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index ab5a2da..cb16ed5 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -447,6 +447,10 @@ mymain(void) DO_TEST("graphics-spice-qxl-vga"); DO_TEST("graphics-spice-socket"); DO_TEST("graphics-spice-auto-socket"); + cfg->spiceAutoUnixSocket = true; + DO_TEST("graphics-spice-auto-socket-cfg"); + cfg->spiceAutoUnixSocket = false; + DO_TEST("nographics-vga"); DO_TEST("input-usbmouse"); DO_TEST("input-usbtablet"); -- 2.8.2

This new listen type is currently supported only by spice graphics. It's introduced to make it easier and clearer specify to not listen anywhere in order to start a guest with OpenGL support. The old way to do this was set spice graphics autoport='no' and don't specify any ports. The new way is to use <listen type='none'/>. In order to be able to migrate to old libvirt the migratable XML will be generated without the listen element and with autoport='no'. Also the old configuration will be automatically converted to the this listen type. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 11 ++++ docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 77 ++++++++++++++++++---- src/qemu/qemu_command.c | 13 ++-- .../qemuxml2argv-video-virtio-gpu-spice-gl.args | 2 +- .../qemuxml2xmlout-video-virtio-gpu-spice-gl.xml | 4 +- 6 files changed, 89 insertions(+), 23 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fa8fac7..18857bc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5359,6 +5359,17 @@ qemu-kvm -net nic,model=? /dev/null attribute all <code>listen</code> elements are ignored. </p> </dd> + <dt><code>none</code> <span class="since">since 1.3.5</span></dt> + <dd> + <p> + This listen type doesn't have any other attribute. Libvirt supports + passing a file descriptor through our APIs virDomainOpenGraphics() and + virDomainOpenGraphicsFD(). No other listen types are allowed if this + one is used and the graphics device doesn't listen anywhere. You need + to use one of the two APIs to pass a FD to QEMU in order to connect to + this graphics device. Supported only by <code>spice</code>. + </p> + </dd> </dl> <h4><a name="elementsVideo">Video devices</a></h4> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 60f9f52..70d3e3c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2981,6 +2981,11 @@ </attribute> </optional> </group> + <group> + <attribute name="type"> + <value>none</value> + </attribute> + </group> </choice> </element> </zeroOrMore> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b72cfec..da4f104 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3925,6 +3925,33 @@ virDomainDefPostParseTimer(virDomainDefPtr def) } +static void +virDomainDefPostParseGraphics(virDomainDef *def) +{ + size_t i; + + for (i = 0; i < def->ngraphics; i++) { + virDomainGraphicsDefPtr graphics = def->graphics[i]; + + /* If spice graphics is configured without ports and with autoport='no' + * then we start qemu with Spice to not listen anywhere. Let's convert + * this configuration to the new listen type='none' which does the + * same. */ + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + virDomainGraphicsListenDefPtr glisten = &graphics->listens[0]; + + if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS && + graphics->data.spice.port == 0 && + graphics->data.spice.tlsPort == 0 && + !graphics->data.spice.autoport) { + VIR_FREE(glisten->address); + glisten->type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE; + } + } + } +} + + /* Check if a drive type address $controller:$bus:$target:$unit is already * taken by a disk or not. */ @@ -4404,6 +4431,8 @@ virDomainDefPostParseInternal(virDomainDefPtr def, /* clean up possibly duplicated metadata entries */ virDomainDefMetadataSanitize(def); + virDomainDefPostParseGraphics(def); + return 0; } @@ -10706,13 +10735,28 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, } def->type = typeVal; - if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET && - graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC && - graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("listen type 'socket' is not available for " - "graphics type '%s'"), graphicsType); - goto error; + switch (def->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: + if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC && + graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("listen type 'socket' is not available for " + "graphics type '%s'"), graphicsType); + goto error; + } + break; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("listen type 'none' is not available for " + "graphics type '%s'"), graphicsType); + goto error; + } + break; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: + break; } if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS) { @@ -21437,10 +21481,8 @@ virDomainGraphicsListenDefFormat(virBufferPtr buf, return; virBufferAddLit(buf, "<listen"); - if (def->type) { - virBufferAsprintf(buf, " type='%s'", - virDomainGraphicsListenTypeToString(def->type)); - } + virBufferAsprintf(buf, " type='%s'", + virDomainGraphicsListenTypeToString(def->type)); if (def->address && (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS || @@ -21623,6 +21665,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) + virBufferAddStr(buf, " autoport='no'"); + break; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: break; @@ -21644,8 +21690,6 @@ virDomainGraphicsDefFormat(virBufferPtr buf, } for (i = 0; i < def->nListens; i++) { - if (def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE) - continue; if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) { if (def->listens[i].fromConfig) continue; @@ -21658,6 +21702,13 @@ virDomainGraphicsDefFormat(virBufferPtr buf, def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET && !def->listens[i].autogenerated) continue; + + /* The new listen type none is in the migratable XML represented as + * port=0 and autoport=no because old libvirt support this + * configuration for spice. */ + if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE) + continue; } if (!children) { virBufferAddLit(buf, ">\n"); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d97b525..e3e2481 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7559,6 +7559,11 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, break; case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + /* QEMU requires either port or tls-port to be specified if there is no + * other argument. Use a dummy port=0. */ + virBufferAddLit(&opt, "port=0,"); + hasInsecure = true; + break; case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: break; } @@ -7702,13 +7707,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, virBufferTrim(&opt, ",", -1); virCommandAddArg(cmd, "-spice"); - /* If we did not add any SPICE arguments, add a dummy 'port=0' one - * as -spice alone is not allowed on QEMU command line - */ - if (virBufferUse(&opt) == 0) - virCommandAddArg(cmd, "port=0"); - else - virCommandAddArgBuffer(cmd, &opt); + virCommandAddArgBuffer(cmd, &opt); if (graphics->data.spice.keymap) virCommandAddArgList(cmd, "-k", graphics->data.spice.keymap, NULL); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args index b80ad16..edecca1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args @@ -19,6 +19,6 @@ QEMU_AUDIO_DRV=spice \ -drive file=/var/lib/libvirt/images/QEMUGuest1,format=qcow2,if=none,\ id=drive-ide0-0-0,cache=none \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ --spice gl=on \ +-spice port=0,gl=on \ -device virtio-vga,id=video0,virgl=on,bus=pci.0,addr=0x2 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-virtio-gpu-spice-gl.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-virtio-gpu-spice-gl.xml index a6dddab..9fb533a 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-virtio-gpu-spice-gl.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-virtio-gpu-spice-gl.xml @@ -29,8 +29,8 @@ <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <graphics type='spice' autoport='no'> - <listen type='address'/> + <graphics type='spice'> + <listen type='none'/> <gl enable='yes'/> </graphics> <video> -- 2.8.2

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 3 ++- src/conf/domain_conf.c | 3 ++- src/qemu/qemu_command.c | 3 +++ .../qemuxml2argv-graphics-vnc-none.args | 20 +++++++++++++++ .../qemuxml2argv-graphics-vnc-none.xml | 30 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 6 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-none.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-none.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 18857bc..c8d881c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5367,7 +5367,8 @@ qemu-kvm -net nic,model=? /dev/null virDomainOpenGraphicsFD(). No other listen types are allowed if this one is used and the graphics device doesn't listen anywhere. You need to use one of the two APIs to pass a FD to QEMU in order to connect to - this graphics device. Supported only by <code>spice</code>. + this graphics device. Supported only by <code>vnc</code> and + <code>spice</code>. </p> </dd> </dl> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index da4f104..5eb5aa2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10746,7 +10746,8 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, } break; case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: - if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("listen type 'none' is not available for " "graphics type '%s'"), graphicsType); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e3e2481..b3ab91b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7435,6 +7435,9 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, break; case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + virBufferAddLit(&opt, "none"); + break; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: break; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-none.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-none.args new file mode 100644 index 0000000..69d1991 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-none.args @@ -0,0 +1,20 @@ +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 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-vnc none \ +-vga cirrus diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-none.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-none.xml new file mode 100644 index 0000000..72a4819 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-none.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</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' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc'> + <listen type='none'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index da678d7..66f8f92 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -905,6 +905,7 @@ mymain(void) driver.config->vncAutoUnixSocket = false; DO_TEST("graphics-vnc-socket", QEMU_CAPS_VNC); DO_TEST("graphics-vnc-auto-socket", QEMU_CAPS_VNC); + DO_TEST("graphics-vnc-none", QEMU_CAPS_VNC); driver.config->vncSASL = 1; VIR_FREE(driver.config->vncSASLdir); -- 2.8.2
participants (3)
-
Cole Robinson
-
Ján Tomko
-
Pavel Hrdina