[libvirt] [PATCH 0/4] some graphics cleanups and improvements
Pavel Hrdina (4): qemu_process: separate graphics port reservation graphics: generate fake ports also for tests qemu_process: merge graphics code into qemuProcessSetupGraphics qemu_hotplug: fix checking graphics ports src/qemu/qemu_driver.c | 12 --- src/qemu/qemu_hotplug.c | 13 ++-- src/qemu/qemu_process.c | 86 +++++++++++----------- src/qemu/qemu_process.h | 8 -- .../qemuhotplug-graphics-spice-listen-network.xml | 2 +- .../qemuxml2argv-controller-order.args | 2 +- .../qemuxml2argv-hugepages-numa.args | 2 +- .../qemuxml2argv-video-device-pciaddr-default.args | 2 +- 8 files changed, 51 insertions(+), 76 deletions(-) -- 2.8.2
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 22b27b3..f110a66 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4348,12 +4348,10 @@ qemuProcessStartHook(virQEMUDriverPtr driver, static int -qemuProcessSetupGraphics(virQEMUDriverPtr driver, - virDomainObjPtr vm) +qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver, + virDomainObjPtr vm) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); size_t i; - int ret = -1; for (i = 0; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; @@ -4362,7 +4360,7 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, if (virPortAllocatorSetUsed(driver->remotePorts, graphics->data.vnc.port, true) < 0) - goto cleanup; + return -1; graphics->data.vnc.portReserved = true; } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && @@ -4371,7 +4369,7 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, if (virPortAllocatorSetUsed(driver->remotePorts, graphics->data.spice.port, true) < 0) - goto cleanup; + return -1; graphics->data.spice.portReserved = true; } @@ -4379,12 +4377,27 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, if (virPortAllocatorSetUsed(driver->remotePorts, graphics->data.spice.tlsPort, true) < 0) - goto cleanup; + return -1; graphics->data.spice.tlsPortReserved = true; } } } + return 0; +} + + +static int +qemuProcessSetupGraphics(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + size_t i; + int ret = -1; + + if (qemuProcessGraphicsReservePorts(driver, vm) < 0) + goto cleanup; + for (i = 0; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; -- 2.8.2
On 05/09/2016 09:48 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-)
ACK - Cole
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_driver.c | 12 ------------ src/qemu/qemu_process.c | 22 ++++++++++++---------- src/qemu/qemu_process.h | 8 -------- .../qemuxml2argv-controller-order.args | 2 +- .../qemuxml2argv-hugepages-numa.args | 2 +- .../qemuxml2argv-video-device-pciaddr-default.args | 2 +- 6 files changed, 15 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b0cfb10..c4c4968 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7060,18 +7060,6 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, net->mac = mac; } - /* do fake auto-alloc of graphics ports, if such config is used */ - for (i = 0; i < vm->def->ngraphics; ++i) { - virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; - if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { - if (qemuProcessVNCAllocatePorts(driver, graphics, false) < 0) - goto cleanup; - } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - if (qemuProcessSPICEAllocatePorts(driver, cfg, graphics, false) < 0) - goto cleanup; - } - } - if (!(cmd = qemuProcessCreatePretendCmd(conn, driver, vm, NULL, qemuCheckFips(), true, VIR_QEMU_PROCESS_START_COLD))) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f110a66..3d46695 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3856,7 +3856,7 @@ qemuProcessReconnectAll(virConnectPtr conn, virQEMUDriverPtr driver) virDomainObjListForEach(driver->domains, qemuProcessReconnectHelper, &data); } -int +static int qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver, virDomainGraphicsDefPtr graphics, bool allocate) @@ -3888,7 +3888,7 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver, return 0; } -int +static int qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg, virDomainGraphicsDefPtr graphics, @@ -4389,13 +4389,15 @@ qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver, static int qemuProcessSetupGraphics(virQEMUDriverPtr driver, - virDomainObjPtr vm) + virDomainObjPtr vm, + unsigned int flags) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + bool allocate = !(flags & VIR_QEMU_PROCESS_START_PRETEND); size_t i; int ret = -1; - if (qemuProcessGraphicsReservePorts(driver, vm) < 0) + if (allocate && qemuProcessGraphicsReservePorts(driver, vm) < 0) goto cleanup; for (i = 0; i < vm->def->ngraphics; ++i) { @@ -4403,12 +4405,12 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, switch (graphics->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - if (qemuProcessVNCAllocatePorts(driver, graphics, true) < 0) + if (qemuProcessVNCAllocatePorts(driver, graphics, allocate) < 0) goto cleanup; break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - if (qemuProcessSPICEAllocatePorts(driver, cfg, graphics, true) < 0) + if (qemuProcessSPICEAllocatePorts(driver, cfg, graphics, allocate) < 0) goto cleanup; break; @@ -5175,6 +5177,10 @@ qemuProcessPrepareDomain(virConnectPtr conn, if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0) goto cleanup; + VIR_DEBUG("Setting up ports for graphics"); + if (qemuProcessSetupGraphics(driver, vm, flags) < 0) + goto cleanup; + /* Fill in run-time values for graphics devices. */ for (i = 0; i < vm->def->ngraphics; i++) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; @@ -5313,10 +5319,6 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, VIR_DEBUG("Ensuring no historical cgroup is lying around"); qemuRemoveCgroup(vm); - VIR_DEBUG("Setting up ports for graphics"); - if (qemuProcessSetupGraphics(driver, vm) < 0) - goto cleanup; - if (virFileMakePath(cfg->logDir) < 0) { virReportSystemError(errno, _("cannot create log directory %s"), diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 9e2e036..623e1e7 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -175,14 +175,6 @@ bool qemuProcessAutoDestroyActive(virQEMUDriverPtr driver, int qemuProcessSetSchedParams(int id, pid_t pid, size_t nsp, virDomainThreadSchedParamPtr sp); -int qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver, - virDomainGraphicsDefPtr graphics, - bool allocate); -int qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, - virQEMUDriverConfigPtr cfg, - virDomainGraphicsDefPtr graphics, - bool allocate); - virDomainDiskDefPtr qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm, const char *alias); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args index b47193a..b46685f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args @@ -37,7 +37,7 @@ media=cdrom,id=drive-ide0-1-0 \ -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,\ id=channel0,name=com.redhat.spice.0 \ -device usb-tablet,id=input0 \ --spice port=0 \ +-spice port=5901,tls-port=5902,addr=0.0.0.0,x509-dir=/etc/pki/libvirt-spice \ -vga cirrus \ -device intel-hda,id=sound0,bus=pci.0,addr=0x4 \ -device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args index 56956a3..ae8ef71 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args @@ -47,7 +47,7 @@ id=channel0,name=org.qemu.guest_agent.0 \ -device virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,\ id=channel1,name=com.redhat.spice.0 \ -device usb-tablet,id=input0 \ --spice port=0 \ +-spice port=5901,tls-port=5902,addr=127.0.0.1,x509-dir=/etc/pki/libvirt-spice \ -vga qxl \ -global qxl-vga.ram_size=67108864 \ -global qxl-vga.vram_size=67108864 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-device-pciaddr-default.args b/tests/qemuxml2argvdata/qemuxml2argv-video-device-pciaddr-default.args index c85c550..40937b8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-video-device-pciaddr-default.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-device-pciaddr-default.args @@ -19,7 +19,7 @@ QEMU_AUDIO_DRV=none \ -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 \ --vnc 127.0.0.1:-5900 \ +-vnc 127.0.0.1:0 \ -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,bus=pci.0,\ addr=0x3 \ -device qxl,id=video1,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x4 \ -- 2.8.2
On 05/09/2016 09:48 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_driver.c | 12 ------------ src/qemu/qemu_process.c | 22 ++++++++++++---------- src/qemu/qemu_process.h | 8 -------- .../qemuxml2argv-controller-order.args | 2 +- .../qemuxml2argv-hugepages-numa.args | 2 +- .../qemuxml2argv-video-device-pciaddr-default.args | 2 +- 6 files changed, 15 insertions(+), 33 deletions(-)
ACK - Cole
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 45 ++++++++++++++------------------------------- 1 file changed, 14 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3d46695..eddf3a7 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4402,16 +4402,21 @@ 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: @@ -4420,6 +4425,14 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; } + + if (graphics->nListens == 0 && listenAddr) { + if (virDomainGraphicsListenAppendAddress(graphics, + listenAddr) < 0) + goto cleanup; + + graphics->listens[0].fromConfig = true; + } } ret = 0; @@ -5177,40 +5190,10 @@ qemuProcessPrepareDomain(virConnectPtr conn, if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0) goto cleanup; - VIR_DEBUG("Setting up ports for graphics"); + VIR_DEBUG("Setting graphics devices"); if (qemuProcessSetupGraphics(driver, vm, flags) < 0) goto cleanup; - /* Fill in run-time values for graphics devices. */ - 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: - 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; - } - - if (graphics->nListens == 0 && listenAddr) { - if (virDomainGraphicsListenAppendAddress(graphics, - listenAddr) < 0) - goto cleanup; - - graphics->listens[0].fromConfig = true; - } - } - /* "volume" type disk's source must be translated before * cgroup and security setting. */ -- 2.8.2
On 05/09/2016 09:48 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 45 ++++++++++++++------------------------------- 1 file changed, 14 insertions(+), 31 deletions(-)
ACK - Cole
We cannot change ports for running domain and we should error out if autoport is enabled. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_hotplug.c | 13 +++++-------- .../qemuhotplug-graphics-spice-listen-network.xml | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 07b419d..f40b34d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2666,9 +2666,8 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, switch (dev->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - if ((olddev->data.vnc.autoport != dev->data.vnc.autoport) || - (!dev->data.vnc.autoport && - (olddev->data.vnc.port != dev->data.vnc.port))) { + if (olddev->data.vnc.autoport != dev->data.vnc.autoport || + olddev->data.vnc.port != dev->data.vnc.port) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot change port settings on vnc graphics")); goto cleanup; @@ -2710,11 +2709,9 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - if ((olddev->data.spice.autoport != dev->data.spice.autoport) || - (!dev->data.spice.autoport && - (olddev->data.spice.port != dev->data.spice.port)) || - (!dev->data.spice.autoport && - (olddev->data.spice.tlsPort != dev->data.spice.tlsPort))) { + if (olddev->data.spice.autoport != dev->data.spice.autoport || + olddev->data.spice.port != dev->data.spice.port || + olddev->data.spice.tlsPort != dev->data.spice.tlsPort) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot change port settings on spice graphics")); goto cleanup; diff --git a/tests/qemuhotplugtestdata/qemuhotplug-graphics-spice-listen-network.xml b/tests/qemuhotplugtestdata/qemuhotplug-graphics-spice-listen-network.xml index 426a14d..f2a6aeb 100644 --- a/tests/qemuhotplugtestdata/qemuhotplug-graphics-spice-listen-network.xml +++ b/tests/qemuhotplugtestdata/qemuhotplug-graphics-spice-listen-network.xml @@ -1,4 +1,4 @@ - <graphics autoport='yes' connected='disconnect' keymap='en-us' passwd='password2' passwdValidTo='2013-06-20T01:34:37' port='5900' tlsPort='5901' type='spice'> + <graphics autoport='yes' connected='disconnect' keymap='en-us' passwd='password2' passwdValidTo='2013-06-20T01:34:37' type='spice'> <listen address='10.65.210.231' network='vdsm-rhevm' type='network'/> <channel mode='secure' name='main'/> <channel mode='secure' name='display'/> -- 2.8.2
On 05/09/2016 09:48 AM, Pavel Hrdina wrote:
We cannot change ports for running domain and we should error out if autoport is enabled.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_hotplug.c | 13 +++++-------- .../qemuhotplug-graphics-spice-listen-network.xml | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-)
ACK - Cole
On Mon, May 09, 2016 at 09:24:13PM -0400, Cole Robinson wrote:
On 05/09/2016 09:48 AM, Pavel Hrdina wrote:
We cannot change ports for running domain and we should error out if autoport is enabled.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_hotplug.c | 13 +++++-------- .../qemuhotplug-graphics-spice-listen-network.xml | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-)
ACK
Thanks, I've pushed the series. Pavel
participants (2)
-
Cole Robinson -
Pavel Hrdina