[libvirt] [PATCH v1 22/26] qemu: move qemuBuildGraphicsSPICECommandLine validation to qemu_domain.c

Move the SPICE caps validation from qemuBuildGraphicsSPICECommandLine() to a new function called qemuDomainDeviceDefValidateSPICEGraphics(). This function is called by qemuDomainDeviceDefValidateGraphics(), which in turn is called by qemuDomainDefValidate(), validating the graphics parameters in domain define time. This validation move exposed a flaw in the 'default-video-type' tests for PPC64, AARCH64 and s390 archs. The XML was considering 'spice' as the default video type, which isn't true for those architectures. This was flying under the radar until now because the SPICE validation was being made in 'virsh start' time, while the XML validation done in qemuxml2xmltest.c considers define time. All other tests were adapted to consider SPICE validation in this earlier stage. Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; --- src/qemu/qemu_command.c | 45 +--------- src/qemu/qemu_domain.c | 83 ++++++++++++++++++- tests/qemuhotplugtest.c | 7 ++ .../default-video-type-aarch64.xml | 2 +- .../default-video-type-ppc64.xml | 2 +- .../default-video-type-s390x.xml | 2 +- ...ault-video-type-aarch64.aarch64-latest.xml | 4 +- .../default-video-type-ppc64.ppc64-latest.xml | 4 +- .../default-video-type-s390x.s390x-latest.xml | 4 +- tests/qemuxml2xmltest.c | 62 +++++++++++--- 10 files changed, 150 insertions(+), 65 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7661527d39..aceb42a289 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7520,7 +7520,6 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, static int qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, virCommandPtr cmd, - virQEMUCapsPtr qemuCaps, virDomainGraphicsDefPtr graphics) { g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER; @@ -7531,12 +7530,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, bool hasSecure = false; bool hasInsecure = false; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice graphics are not supported with this QEMU")); - return -1; - } - if (!(glisten = virDomainGraphicsGetListen(graphics, 0))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing listen element")); @@ -7545,13 +7538,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, switch (glisten->type) { case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_UNIX)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("unix socket for spice graphics are not supported " - "with this QEMU")); - return -1; - } - virBufferAddLit(&opt, "unix,addr="); virQEMUBuildBufferEscapeComma(&opt, glisten->socket); virBufferAddLit(&opt, ","); @@ -7566,12 +7552,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, } 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")); - return -1; - } virBufferAsprintf(&opt, "tls-port=%u,", tlsPort); hasSecure = true; } @@ -7710,35 +7690,16 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, if (graphics->data.spice.copypaste == VIR_TRISTATE_BOOL_NO) virBufferAddLit(&opt, "disable-copy-paste,"); - if (graphics->data.spice.filetransfer == VIR_TRISTATE_BOOL_NO) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU can't disable file transfers through spice")); - return -1; - } else { - virBufferAddLit(&opt, "disable-agent-file-xfer,"); - } - } + if (graphics->data.spice.filetransfer == VIR_TRISTATE_BOOL_NO) + virBufferAddLit(&opt, "disable-agent-file-xfer,"); if (graphics->data.spice.gl == VIR_TRISTATE_BOOL_YES) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_GL)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU doesn't support spice OpenGL")); - return -1; - } - /* spice.gl is a TristateBool, but qemu expects on/off: use * TristateSwitch helper */ virBufferAsprintf(&opt, "gl=%s,", virTristateSwitchTypeToString(graphics->data.spice.gl)); if (graphics->data.spice.rendernode) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_RENDERNODE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU doesn't support spice OpenGL rendernode")); - return -1; - } - virBufferAddLit(&opt, "rendernode="); virQEMUBuildBufferEscapeComma(&opt, graphics->data.spice.rendernode); virBufferAddLit(&opt, ","); @@ -7821,7 +7782,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: if (qemuBuildGraphicsSPICECommandLine(cfg, cmd, - qemuCaps, graphics) < 0) + graphics) < 0) return -1; break; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c25adbc4f3..9f29d2afbe 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7566,9 +7566,84 @@ qemuDomainDeviceDefValidateTPM(virDomainTPMDef *tpm, } +static int +qemuDomainDeviceDefValidateSPICEGraphics(const virDomainGraphicsDef *graphics, + virQEMUDriverPtr driver, + virQEMUCapsPtr qemuCaps) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + virDomainGraphicsListenDefPtr glisten = NULL; + int tlsPort = graphics->data.spice.tlsPort; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice graphics are not supported with this QEMU")); + return -1; + } + + glisten = virDomainGraphicsGetListen((virDomainGraphicsDefPtr)graphics, 0); + if (!glisten) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing listen element")); + return -1; + } + + switch (glisten->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_UNIX)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unix socket for spice graphics are not supported " + "with this QEMU")); + return -1; + } + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + if (tlsPort > 0 && !cfg->spiceTLS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice TLS port set in XML configuration, " + "but TLS is disabled in qemu.conf")); + return -1; + } + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + break; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: + break; + } + + if (graphics->data.spice.filetransfer == VIR_TRISTATE_BOOL_NO && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU can't disable file transfers through spice")); + return -1; + } + + if (graphics->data.spice.gl == VIR_TRISTATE_BOOL_YES) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_GL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support spice OpenGL")); + return -1; + } + + if (graphics->data.spice.rendernode && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_RENDERNODE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support spice OpenGL rendernode")); + return -1; + } + } + + return 0; +} + + static int qemuDomainDeviceDefValidateGraphics(const virDomainGraphicsDef *graphics, const virDomainDef *def, + virQEMUDriverPtr driver, virQEMUCapsPtr qemuCaps) { bool have_egl_headless = false; @@ -7635,6 +7710,12 @@ qemuDomainDeviceDefValidateGraphics(const virDomainGraphicsDef *graphics, break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + if (qemuDomainDeviceDefValidateSPICEGraphics(graphics, driver, + qemuCaps) < 0) + return -1; + + break; + case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: @@ -8042,7 +8123,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, case VIR_DOMAIN_DEVICE_GRAPHICS: ret = qemuDomainDeviceDefValidateGraphics(dev->data.graphics, def, - qemuCaps); + driver, qemuCaps); break; case VIR_DOMAIN_DEVICE_INPUT: diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 9646a30fb6..ea8fcd92d2 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -85,6 +85,8 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_PIIX_DISABLE_S3); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_PIIX_DISABLE_S4); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_VNC); + virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SPICE); + virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE); if (qemuTestCapsCacheInsert(driver.qemuCapsCache, priv->qemuCaps) < 0) return -1; @@ -592,6 +594,7 @@ mymain(void) struct qemuHotplugTestData data = {0}; struct testQemuHotplugCpuParams cpudata; g_autofree char *fakerootdir = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; fakerootdir = g_strdup(FAKEROOTDIRTEMPLATE); @@ -610,6 +613,8 @@ mymain(void) if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; + cfg = virQEMUDriverGetConfig(&driver); + virEventRegisterDefaultImpl(); VIR_FREE(driver.config->spiceListen); @@ -676,6 +681,7 @@ mymain(void) " }" \ "}\r\n" + cfg->spiceTLS = true; DO_TEST_UPDATE("graphics-spice", "graphics-spice-nochange", false, false, NULL); DO_TEST_UPDATE("graphics-spice-timeout", "graphics-spice-timeout-nochange", false, false, "set_password", QMP_OK, "expire_password", QMP_OK); @@ -684,6 +690,7 @@ mymain(void) DO_TEST_UPDATE("graphics-spice", "graphics-spice-listen", true, false, NULL); DO_TEST_UPDATE("graphics-spice-listen-network", "graphics-spice-listen-network-password", false, false, "set_password", QMP_OK, "expire_password", QMP_OK); + cfg->spiceTLS = false; /* Strange huh? Currently, only graphics can be updated :-P */ DO_TEST_UPDATE("disk-cdrom", "disk-cdrom-nochange", true, false, NULL); diff --git a/tests/qemuxml2argvdata/default-video-type-aarch64.xml b/tests/qemuxml2argvdata/default-video-type-aarch64.xml index 03326d3c9b..f7d2d5d94a 100644 --- a/tests/qemuxml2argvdata/default-video-type-aarch64.xml +++ b/tests/qemuxml2argvdata/default-video-type-aarch64.xml @@ -11,6 +11,6 @@ <emulator>/usr/bin/qemu-system-aarch64</emulator> <controller type='usb' index='0' model='none'/> <memballoon model='none'/> - <graphics type='spice'/> + <graphics type='vnc'/> </devices> </domain> diff --git a/tests/qemuxml2argvdata/default-video-type-ppc64.xml b/tests/qemuxml2argvdata/default-video-type-ppc64.xml index 739e07fc19..ea5b966cfd 100644 --- a/tests/qemuxml2argvdata/default-video-type-ppc64.xml +++ b/tests/qemuxml2argvdata/default-video-type-ppc64.xml @@ -11,6 +11,6 @@ <emulator>/usr/bin/qemu-system-ppc64</emulator> <controller type='usb' index='0' model='none'/> <memballoon model='none'/> - <graphics type='spice'/> + <graphics type='vnc'/> </devices> </domain> diff --git a/tests/qemuxml2argvdata/default-video-type-s390x.xml b/tests/qemuxml2argvdata/default-video-type-s390x.xml index 9eda06a3a1..fe402d2c7f 100644 --- a/tests/qemuxml2argvdata/default-video-type-s390x.xml +++ b/tests/qemuxml2argvdata/default-video-type-s390x.xml @@ -11,6 +11,6 @@ <emulator>/usr/bin/qemu-system-s390x</emulator> <controller type='usb' index='0' model='none'/> <memballoon model='none'/> - <graphics type='spice'/> + <graphics type='vnc'/> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/default-video-type-aarch64.aarch64-latest.xml b/tests/qemuxml2xmloutdata/default-video-type-aarch64.aarch64-latest.xml index 4b660b8d70..1efea62f6f 100644 --- a/tests/qemuxml2xmloutdata/default-video-type-aarch64.aarch64-latest.xml +++ b/tests/qemuxml2xmloutdata/default-video-type-aarch64.aarch64-latest.xml @@ -30,8 +30,8 @@ <target chassis='2' port='0x9'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> - <graphics type='spice'> - <listen type='none'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen type='address'/> </graphics> <video> <model type='virtio' heads='1' primary='yes'/> diff --git a/tests/qemuxml2xmloutdata/default-video-type-ppc64.ppc64-latest.xml b/tests/qemuxml2xmloutdata/default-video-type-ppc64.ppc64-latest.xml index 590a73b456..6c4bd5ef8b 100644 --- a/tests/qemuxml2xmloutdata/default-video-type-ppc64.ppc64-latest.xml +++ b/tests/qemuxml2xmloutdata/default-video-type-ppc64.ppc64-latest.xml @@ -19,8 +19,8 @@ <controller type='pci' index='0' model='pci-root'/> <input type='keyboard' bus='usb'/> <input type='mouse' bus='usb'/> - <graphics type='spice'> - <listen type='none'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen type='address'/> </graphics> <video> <model type='vga' vram='16384' heads='1' primary='yes'/> diff --git a/tests/qemuxml2xmloutdata/default-video-type-s390x.s390x-latest.xml b/tests/qemuxml2xmloutdata/default-video-type-s390x.s390x-latest.xml index 21718db1ca..d4ccf82712 100644 --- a/tests/qemuxml2xmloutdata/default-video-type-s390x.s390x-latest.xml +++ b/tests/qemuxml2xmloutdata/default-video-type-s390x.s390x-latest.xml @@ -17,8 +17,8 @@ <emulator>/usr/bin/qemu-system-s390x</emulator> <controller type='usb' index='0' model='none'/> <controller type='pci' index='0' model='pci-root'/> - <graphics type='spice'> - <listen type='none'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen type='address'/> </graphics> <video> <model type='virtio' heads='1' primary='yes'/> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f22bc0c9e6..d3ef08dcdf 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -382,22 +382,45 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("default-video-type-ppc64", "ppc64"); DO_TEST_CAPS_ARCH_LATEST("default-video-type-riscv64", "riscv64"); DO_TEST_CAPS_ARCH_LATEST("default-video-type-s390x", "s390x"); - DO_TEST("default-video-type-x86_64-caps-test-0", QEMU_CAPS_DEVICE_VGA); - DO_TEST("default-video-type-x86_64-caps-test-1", QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("default-video-type-x86_64-caps-test-0", + QEMU_CAPS_DEVICE_VGA, + QEMU_CAPS_SPICE); + DO_TEST("default-video-type-x86_64-caps-test-1", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_SPICE); DO_TEST("graphics-sdl", QEMU_CAPS_DEVICE_VGA); DO_TEST("graphics-sdl-fullscreen", QEMU_CAPS_DEVICE_CIRRUS_VGA); - DO_TEST("graphics-spice", QEMU_CAPS_DEVICE_QXL); - DO_TEST("graphics-spice-compression", QEMU_CAPS_DEVICE_QXL); - DO_TEST("graphics-spice-qxl-vga", QEMU_CAPS_DEVICE_QXL); - DO_TEST("graphics-spice-socket", QEMU_CAPS_DEVICE_CIRRUS_VGA); - DO_TEST("graphics-spice-auto-socket", QEMU_CAPS_DEVICE_CIRRUS_VGA); + + cfg->spiceTLS = true; + DO_TEST("graphics-spice", + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_SPICE, + QEMU_CAPS_SPICE_FILE_XFER_DISABLE); + DO_TEST("graphics-spice-compression", + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_SPICE); + DO_TEST("graphics-spice-qxl-vga", + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_SPICE); + DO_TEST("graphics-spice-socket", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_SPICE, + QEMU_CAPS_SPICE_UNIX); + DO_TEST("graphics-spice-auto-socket", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_SPICE, + QEMU_CAPS_SPICE_UNIX); cfg->spiceAutoUnixSocket = true; - DO_TEST("graphics-spice-auto-socket-cfg", QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("graphics-spice-auto-socket-cfg", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_SPICE); cfg->spiceAutoUnixSocket = false; + cfg->spiceTLS = false; DO_TEST("graphics-spice-egl-headless", QEMU_CAPS_DEVICE_QXL, - QEMU_CAPS_EGL_HEADLESS); + QEMU_CAPS_EGL_HEADLESS, + QEMU_CAPS_SPICE); DO_TEST("graphics-egl-headless-rendernode", QEMU_CAPS_DEVICE_CIRRUS_VGA, @@ -448,7 +471,13 @@ mymain(void) DO_TEST("serial-tcp-tlsx509-chardev", NONE); DO_TEST("serial-tcp-tlsx509-chardev-notls", NONE); - DO_TEST("serial-spiceport", QEMU_CAPS_DEVICE_QXL); + + cfg->spiceTLS = true; + DO_TEST("serial-spiceport", + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_SPICE); + cfg->spiceTLS = false; + DO_TEST("serial-spiceport-nospice", NONE); DO_TEST("console-compat", NONE); DO_TEST("console-compat2", NONE); @@ -578,7 +607,8 @@ mymain(void) DO_TEST("seclabel-device-multiple", NONE); DO_TEST_FULL("seclabel-dynamic-none-relabel", WHEN_INACTIVE, ARG_QEMU_CAPS, QEMU_CAPS_DEVICE_CIRRUS_VGA, - QEMU_CAPS_OBJECT_MEMORY_FILE, NONE); + QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_SPICE, NONE); DO_TEST("numad-static-vcpu-no-numatune", NONE); DO_TEST("disk-scsi-lun-passthrough-sgio", @@ -698,7 +728,9 @@ mymain(void) DO_TEST("graphics-listen-network2", QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_VNC); - DO_TEST("graphics-spice-timeout", QEMU_CAPS_DEVICE_VGA); + DO_TEST("graphics-spice-timeout", + QEMU_CAPS_DEVICE_VGA, + QEMU_CAPS_SPICE); DO_TEST("numad-auto-vcpu-no-numatune", NONE); DO_TEST("numad-auto-memory-vcpu-no-cpuset-and-placement", NONE); DO_TEST("numad-auto-memory-vcpu-cpuset", NONE); @@ -1202,7 +1234,11 @@ mymain(void) QEMU_CAPS_VIRTIO_GPU_VIRGL); DO_TEST("video-virtio-gpu-spice-gl", QEMU_CAPS_DEVICE_VIRTIO_GPU, - QEMU_CAPS_VIRTIO_GPU_VIRGL); + QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_SPICE, + QEMU_CAPS_SPICE_FILE_XFER_DISABLE, + QEMU_CAPS_SPICE_GL, + QEMU_CAPS_SPICE_RENDERNODE); DO_TEST("video-virtio-gpu-sdl-gl", QEMU_CAPS_DEVICE_VIRTIO_GPU, QEMU_CAPS_VIRTIO_GPU_VIRGL, -- 2.23.0