[PATCH] qemu: Add GTK display with OpenGL support
From: Kirill Shchetiniuk <kshcheti@redhat.com> Introduce GTK display support with OpenGL for the QEMU driver. - Add new XML options for GTK display type. - Include capability flags for the QEMU driver. Note: The `QEMU_CAPS_GTK_GL` flag cannot yet be checked, so device definition validation is incomplete. A placeholder is left for future implementation, when the GTK along with OpenGL capability check would be available in QEMU. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/570 Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> --- src/conf/domain_conf.c | 79 ++++++++++++++++++- src/conf/domain_conf.h | 8 ++ src/conf/schemas/domaincommon.rng | 28 +++++++ src/qemu/qemu_capabilities.c | 5 ++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 39 ++++++++- src/qemu/qemu_domain.c | 1 + src/qemu/qemu_driver.c | 2 + src/qemu/qemu_extdevice.c | 2 + src/qemu/qemu_hotplug.c | 1 + src/qemu/qemu_process.c | 8 ++ src/qemu/qemu_validate.c | 16 ++++ src/security/virt-aa-helper.c | 6 ++ src/vmx/vmx.c | 1 + tests/domaincapsdata/qemu_10.0.0.s390x.xml | 1 + tests/domaincapsdata/qemu_7.0.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_7.1.0.ppc64.xml | 1 + tests/domaincapsdata/qemu_7.2.0.ppc.xml | 1 + .../qemu_8.2.0-tcg-virt.loongarch64.xml | 1 + .../qemu_8.2.0-virt.aarch64.xml | 1 + .../qemu_8.2.0-virt.loongarch64.xml | 1 + tests/domaincapsdata/qemu_8.2.0.aarch64.xml | 1 + tests/domaincapsdata/qemu_8.2.0.armv7l.xml | 1 + tests/domaincapsdata/qemu_9.0.0.sparc.xml | 1 + tests/domaincapsdata/qemu_9.1.0.s390x.xml | 1 + tests/domaincapsdata/qemu_9.2.0.s390x.xml | 1 + .../caps_10.0.0_s390x.xml | 1 + .../qemucapabilitiesdata/caps_7.0.0_ppc64.xml | 1 + .../qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 1 + .../caps_8.2.0_aarch64.xml | 1 + .../caps_8.2.0_armv7l.xml | 1 + .../caps_8.2.0_loongarch64.xml | 1 + .../qemucapabilitiesdata/caps_9.0.0_sparc.xml | 1 + .../qemucapabilitiesdata/caps_9.1.0_s390x.xml | 1 + .../qemucapabilitiesdata/caps_9.2.0_s390x.xml | 1 + 36 files changed, 218 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 542d6ade91..73550f7fcf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -960,6 +960,7 @@ VIR_ENUM_IMPL(virDomainGraphics, "spice", "egl-headless", "dbus", + "gtk", ); VIR_ENUM_IMPL(virDomainGraphicsListen, @@ -2054,6 +2055,12 @@ void virDomainGraphicsDefFree(virDomainGraphicsDef *def) g_free(def->data.dbus.rendernode); break; + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: + g_free(def->data.gtk.display); + g_free(def->data.gtk.xauth); + g_free(def->data.gtk.rendernode); + break; + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; } @@ -12199,6 +12206,39 @@ virDomainGraphicsDefParseXMLDBus(virDomainGraphicsDef *def, } +static int +virDomainGraphicsDefParseXMLGTK(virDomainGraphicsDef *def, + xmlNodePtr node, + xmlXPathContextPtr ctxt) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt) + xmlNodePtr glNode; + virTristateBool fullscreen; + + ctxt->node = node; + + if (virXMLPropTristateBool(node, "fullscreen", VIR_XML_PROP_NONE, + &fullscreen) < 0) + return -1; + + virTristateBoolToBool(fullscreen, &def->data.gtk.fullscreen); + def->data.gtk.xauth = virXMLPropString(node, "xauth"); + def->data.gtk.display = virXMLPropString(node, "display"); + + if ((glNode = virXPathNode("./gl", ctxt))) { + def->data.gtk.rendernode = virXMLPropString(glNode, + "rendernode"); + + if (virXMLPropTristateBool(glNode, "enable", + VIR_XML_PROP_REQUIRED, + &def->data.gtk.gl) < 0) + return -1; + } + + return 0; +} + + virDomainGraphicsDef * virDomainGraphicsDefNew(virDomainXMLOption *xmlopt) { @@ -12275,6 +12315,10 @@ virDomainGraphicsDefParseXML(virDomainXMLOption *xmlopt, if (virDomainGraphicsDefParseXMLDBus(def, node, ctxt) < 0) goto error; break; + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: + if (virDomainGraphicsDefParseXMLGTK(def, node, ctxt) < 0) + goto error; + break; case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; } @@ -27118,6 +27162,23 @@ virDomainGraphicsDefFormatDBus(virBuffer *attrBuf, virDomainGraphicsDefFormatAudio(childBuf, def->data.dbus.audioId); } +static void +virDomainGraphicsDefFormatGTK(virBuffer *attrBuf, + virBuffer *childBuf, + virDomainGraphicsDef *def) +{ + virBufferEscapeString(attrBuf, " display='%s'", def->data.gtk.display); + + virBufferEscapeString(attrBuf, " xauth='%s'", def->data.gtk.xauth); + + if (def->data.gtk.fullscreen) + virBufferAddLit(attrBuf, " fullscreen='yes'"); + + virDomainGraphicsDefFormatGL(childBuf, def->data.gtk.gl, + def->data.gtk.rendernode); +} + + static int virDomainGraphicsDefFormat(virBuffer *buf, virDomainGraphicsDef *def, @@ -27166,6 +27227,10 @@ virDomainGraphicsDefFormat(virBuffer *buf, virDomainGraphicsDefFormatDBus(&attrBuf, &childBuf, def); break; + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: + virDomainGraphicsDefFormatGTK(&attrBuf, &childBuf, def); + break; + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; } @@ -31949,6 +32014,11 @@ virDomainGraphicsDefHasOpenGL(const virDomainDef *def) if (graphics->data.dbus.gl == VIR_TRISTATE_BOOL_YES) return true; + continue; + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: + if (graphics->data.gtk.gl == VIR_TRISTATE_BOOL_YES) + return true; + continue; case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; @@ -31966,7 +32036,8 @@ virDomainGraphicsSupportsRenderNode(const virDomainGraphicsDef *graphics) if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE || graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS || - graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_DBUS) + graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_DBUS || + graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_GTK) ret = true; return ret; @@ -31988,6 +32059,9 @@ virDomainGraphicsGetRenderNode(const virDomainGraphicsDef *graphics) case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: ret = graphics->data.dbus.rendernode; break; + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: + ret = graphics->data.gtk.rendernode; + break; case VIR_DOMAIN_GRAPHICS_TYPE_SDL: case VIR_DOMAIN_GRAPHICS_TYPE_VNC: case VIR_DOMAIN_GRAPHICS_TYPE_RDP: @@ -32012,6 +32086,9 @@ virDomainGraphicsNeedsAutoRenderNode(const virDomainGraphicsDef *graphics) if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_DBUS && graphics->data.dbus.gl != VIR_TRISTATE_BOOL_YES) return false; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_GTK && + graphics->data.gtk.gl != VIR_TRISTATE_BOOL_YES) + return false; if (virDomainGraphicsGetRenderNode(graphics)) return false; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 58b97a2b54..f4f06db8a8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1896,6 +1896,7 @@ typedef enum { VIR_DOMAIN_GRAPHICS_TYPE_SPICE, VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS, VIR_DOMAIN_GRAPHICS_TYPE_DBUS, + VIR_DOMAIN_GRAPHICS_TYPE_GTK, VIR_DOMAIN_GRAPHICS_TYPE_LAST } virDomainGraphicsType; @@ -2083,6 +2084,13 @@ struct _virDomainGraphicsDef { unsigned int audioId; bool fromConfig; /* true if the @address is config file originated */ } dbus; + struct { + char *display; + char *xauth; + bool fullscreen; + char *rendernode; + virTristateBool gl; + } gtk; } data; /* nListens, listens, and *port are only useful if type is vnc, * rdp, or spice. They've been extracted from the union only to diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 5597d5a66b..61c1d3ad97 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -4593,6 +4593,34 @@ </element> </optional> </group> + <group> + <attribute name="type"> + <value>gtk</value> + </attribute> + <optional> + <attribute name="display"> + <text/> + </attribute> + </optional> + <optional> + <attribute name="xauth"> + <text/> + </attribute> + </optional> + <optional> + <attribute name="fullscreen"> + <ref name="virYesNo"/> + </attribute> + </optional> + <optional> + <element name="gl"> + <attribute name="enable"> + <ref name="virYesNo"/> + </attribute> + <empty/> + </element> + </optional> + </group> </choice> </element> </define> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a804335c85..de7ce95499 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -732,6 +732,8 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 475 */ "virtio-scsi.iothread-mapping", /* QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING */ + "gtk", /* QEMU_CAPS_GTK */ + "gtk-gl", /* QEMU_CAPS_GTK_GL */ ); @@ -1602,6 +1604,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "set-numa-node/arg-type/+hmat-lb", QEMU_CAPS_NUMA_HMAT }, { "query-cpu-model-expansion/ret-type/deprecated-props", QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_DEPRECATED_PROPS }, { "migrate-incoming/arg-type/exit-on-error", QEMU_CAPS_MIGRATE_INCOMING_EXIT_ON_ERROR }, + { "query-display-options/ret-type/type/^gtk", QEMU_CAPS_GTK }, }; typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps; @@ -6499,6 +6502,8 @@ virQEMUCapsFillDomainDeviceGraphicsCaps(virQEMUDriverConfig *cfg, VIR_DOMAIN_GRAPHICS_TYPE_RDP); } } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_GTK)) + VIR_DOMAIN_CAPS_ENUM_SET(dev->type, VIR_DOMAIN_GRAPHICS_TYPE_GTK); } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index ea7c14daa9..f4ab1a821f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -713,6 +713,8 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 475 */ QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING, /* virtio-scsi supports per-virtqueue iothread mapping */ + QEMU_CAPS_GTK, /* support for GTK graphics is compiled into qemu */ + QEMU_CAPS_GTK_GL, /* support OpenGL for gtk graphics */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e6d308534f..e97992ff56 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8483,6 +8483,34 @@ qemuBuildGraphicsDBusCommandLine(virDomainDef *def, } +static int +qemuBuildGraphicsGTKCommandLine(virQEMUDriverConfig *cfg G_GNUC_UNUSED, + virCommand *cmd, + virDomainGraphicsDef *graphics) +{ + g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER; + + if (graphics->data.gtk.xauth) + virCommandAddEnvPair(cmd, "XAUTHORITY", graphics->data.gtk.xauth); + if (graphics->data.gtk.display) + virCommandAddEnvPair(cmd, "DISPLAY", graphics->data.gtk.display); + if (graphics->data.gtk.fullscreen) + virCommandAddArg(cmd, "-full-screen"); + + virCommandAddArg(cmd, "-display"); + virBufferAddLit(&opt, "gtk"); + + if (graphics->data.gtk.gl != VIR_TRISTATE_BOOL_ABSENT) + virBufferAsprintf(&opt, ",gl=%s", + virTristateSwitchTypeToString(graphics->data.gtk.gl)); + + virCommandAddArgBuffer(cmd, &opt); + + return 0; +} + + + static int qemuBuildGraphicsCommandLine(virQEMUDriverConfig *cfg, virCommand *cmd, @@ -8523,6 +8551,11 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfig *cfg, if (qemuBuildGraphicsDBusCommandLine(def, cmd, graphics) < 0) return -1; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: + if (qemuBuildGraphicsGTKCommandLine(cfg, cmd, graphics) < 0) + return -1; + break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: break; @@ -10076,6 +10109,7 @@ qemuBuildCommandLineValidate(virQEMUDriver *driver, int egl_headless = 0; int dbus = 0; int rdp = 0; + int gtk = 0; if (!driver->privileged) { /* If we have no cgroups then we can have no tunings that @@ -10126,13 +10160,16 @@ qemuBuildCommandLineValidate(virQEMUDriver *driver, case VIR_DOMAIN_GRAPHICS_TYPE_RDP: ++rdp; break; + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: + ++gtk; + break; case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; } } - if (sdl > 1 || vnc > 1 || spice > 1 || egl_headless > 1 || dbus > 1 || rdp > 1) { + if (sdl > 1 || vnc > 1 || spice > 1 || egl_headless > 1 || dbus > 1 || rdp > 1 || gtk > 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("only 1 graphics device of each type (sdl, vnc, spice, headless, dbus, rdp) is supported")); return -1; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 52da234343..f96c926424 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4104,6 +4104,7 @@ qemuDomainDefSuggestDefaultAudioBackend(virQEMUDriver *driver, case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: break; case VIR_DOMAIN_GRAPHICS_TYPE_LAST: default: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a34d6f1437..da22681053 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14837,6 +14837,7 @@ qemuDomainOpenGraphics(virDomainPtr dom, case VIR_DOMAIN_GRAPHICS_TYPE_RDP: case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Can only open VNC, SPICE or D-Bus p2p graphics backends, not %1$s"), virDomainGraphicsTypeToString(vm->def->graphics[idx]->type)); @@ -14908,6 +14909,7 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom, case VIR_DOMAIN_GRAPHICS_TYPE_RDP: case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Can only open VNC, SPICE or D-Bus p2p graphics backends, not %1$s"), virDomainGraphicsTypeToString(vm->def->graphics[idx]->type)); diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 31c7a14156..f6888acf2f 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -256,6 +256,7 @@ qemuExtDevicesStart(virQEMUDriver *driver, case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: case VIR_DOMAIN_GRAPHICS_TYPE_LAST: default: continue; @@ -339,6 +340,7 @@ qemuExtDevicesStop(virQEMUDriver *driver, case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: case VIR_DOMAIN_GRAPHICS_TYPE_LAST: default: continue; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5326aba281..d67c407309 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4688,6 +4688,7 @@ qemuDomainChangeGraphics(virQEMUDriver *driver, case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: virReportError(VIR_ERR_INTERNAL_ERROR, _("unable to change config on '%1$s' graphics type"), type); break; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1af91c5909..b61240b653 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3005,6 +3005,7 @@ qemuProcessInitPasswords(virQEMUDriver *driver, cfg->rdpUsername, cfg->rdpPassword, asyncJob); break; case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: @@ -5065,6 +5066,7 @@ qemuProcessGraphicsReservePorts(virDomainGraphicsDef *graphics, case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; } @@ -5109,6 +5111,7 @@ qemuProcessGraphicsAllocatePorts(virQEMUDriver *driver, case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; } @@ -5280,6 +5283,7 @@ qemuProcessGraphicsSetupListen(virQEMUDriver *driver, case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; } @@ -5359,6 +5363,9 @@ qemuProcessGraphicsSetupRenderNode(virDomainGraphicsDef *graphics, case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: rendernode = &graphics->data.dbus.rendernode; break; + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: + rendernode = &graphics->data.gtk.rendernode; + break; case VIR_DOMAIN_GRAPHICS_TYPE_SDL: case VIR_DOMAIN_GRAPHICS_TYPE_VNC: case VIR_DOMAIN_GRAPHICS_TYPE_RDP: @@ -5571,6 +5578,7 @@ qemuProcessStartValidateGraphics(virDomainObj *vm) case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index b2c3c9e2f6..be92785d71 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4523,6 +4523,17 @@ qemuValidateDomainDeviceDefRDPGraphics(const virDomainGraphicsDef *graphics) } +static int +qemuValidateDomainDeviceDefGTKGraphics(const virDomainGraphicsDef *graphics G_GNUC_UNUSED, + virQEMUCaps *qemuCaps G_GNUC_UNUSED) +{ + /* TODO: OpenGL check */ + /* For now it's not possible to check if gtk-gl capability is available in QEMU */ + /* Add cappability check later when will be possible to gather the capability from QEMU */ + return 0; +} + + static int qemuValidateDomainDeviceDefGraphics(const virDomainGraphicsDef *graphics, const virDomainDef *def, @@ -4600,6 +4611,11 @@ qemuValidateDomainDeviceDefGraphics(const virDomainGraphicsDef *graphics, return -1; break; + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: + if (qemuValidateDomainDeviceDefGTKGraphics(graphics, qemuCaps) < 0) + return -1; + + break; case VIR_DOMAIN_GRAPHICS_TYPE_SDL: case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e3802c18be..d3de4ea4d1 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1037,6 +1037,12 @@ get_files(vahControl * ctl) "r") != 0) goto cleanup; + if (ctl->def->ngraphics == 1 && + ctl->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_GTK) + if (vah_add_file(&buf, ctl->def->graphics[0]->data.gtk.xauth, + "r") != 0) + goto cleanup; + for (i = 0; i < ctl->def->nhostdevs; i++) if (ctl->def->hostdevs[i]) { virDomainHostdevDef *dev = ctl->def->hostdevs[i]; diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 0dd03c1a88..74d90ec77a 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -3527,6 +3527,7 @@ virVMXFormatConfig(virVMXContext *ctx, virDomainXMLOption *xmlopt, virDomainDef case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: + case VIR_DOMAIN_GRAPHICS_TYPE_GTK: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported graphics type '%1$s'"), virDomainGraphicsTypeToString(def->graphics[i]->type)); diff --git a/tests/domaincapsdata/qemu_10.0.0.s390x.xml b/tests/domaincapsdata/qemu_10.0.0.s390x.xml index d66240307e..a81db8e950 100644 --- a/tests/domaincapsdata/qemu_10.0.0.s390x.xml +++ b/tests/domaincapsdata/qemu_10.0.0.s390x.xml @@ -256,6 +256,7 @@ <value>rdp</value> <value>egl-headless</value> <value>dbus</value> + <value>gtk</value> </enum> </graphics> <video supported='yes'> diff --git a/tests/domaincapsdata/qemu_7.0.0.ppc64.xml b/tests/domaincapsdata/qemu_7.0.0.ppc64.xml index 52c73d10a4..e840a0e488 100644 --- a/tests/domaincapsdata/qemu_7.0.0.ppc64.xml +++ b/tests/domaincapsdata/qemu_7.0.0.ppc64.xml @@ -77,6 +77,7 @@ <value>sdl</value> <value>vnc</value> <value>egl-headless</value> + <value>gtk</value> </enum> </graphics> <video supported='yes'> diff --git a/tests/domaincapsdata/qemu_7.1.0.ppc64.xml b/tests/domaincapsdata/qemu_7.1.0.ppc64.xml index ca0bc6f0b5..235e851e86 100644 --- a/tests/domaincapsdata/qemu_7.1.0.ppc64.xml +++ b/tests/domaincapsdata/qemu_7.1.0.ppc64.xml @@ -70,6 +70,7 @@ <graphics supported='yes'> <enum name='type'> <value>vnc</value> + <value>gtk</value> </enum> </graphics> <video supported='yes'> diff --git a/tests/domaincapsdata/qemu_7.2.0.ppc.xml b/tests/domaincapsdata/qemu_7.2.0.ppc.xml index 21dbe730c5..ae379b863e 100644 --- a/tests/domaincapsdata/qemu_7.2.0.ppc.xml +++ b/tests/domaincapsdata/qemu_7.2.0.ppc.xml @@ -69,6 +69,7 @@ <value>spice</value> <value>egl-headless</value> <value>dbus</value> + <value>gtk</value> </enum> </graphics> <video supported='yes'> diff --git a/tests/domaincapsdata/qemu_8.2.0-tcg-virt.loongarch64.xml b/tests/domaincapsdata/qemu_8.2.0-tcg-virt.loongarch64.xml index 18979cf280..3ac273e119 100644 --- a/tests/domaincapsdata/qemu_8.2.0-tcg-virt.loongarch64.xml +++ b/tests/domaincapsdata/qemu_8.2.0-tcg-virt.loongarch64.xml @@ -75,6 +75,7 @@ <value>rdp</value> <value>spice</value> <value>dbus</value> + <value>gtk</value> </enum> </graphics> <video supported='yes'> diff --git a/tests/domaincapsdata/qemu_8.2.0-virt.aarch64.xml b/tests/domaincapsdata/qemu_8.2.0-virt.aarch64.xml index ce17865e24..98346d23d3 100644 --- a/tests/domaincapsdata/qemu_8.2.0-virt.aarch64.xml +++ b/tests/domaincapsdata/qemu_8.2.0-virt.aarch64.xml @@ -123,6 +123,7 @@ <value>rdp</value> <value>egl-headless</value> <value>dbus</value> + <value>gtk</value> </enum> </graphics> <video supported='yes'> diff --git a/tests/domaincapsdata/qemu_8.2.0-virt.loongarch64.xml b/tests/domaincapsdata/qemu_8.2.0-virt.loongarch64.xml index 8f4ebbc107..b6d0f4d455 100644 --- a/tests/domaincapsdata/qemu_8.2.0-virt.loongarch64.xml +++ b/tests/domaincapsdata/qemu_8.2.0-virt.loongarch64.xml @@ -79,6 +79,7 @@ <value>rdp</value> <value>spice</value> <value>dbus</value> + <value>gtk</value> </enum> </graphics> <video supported='yes'> diff --git a/tests/domaincapsdata/qemu_8.2.0.aarch64.xml b/tests/domaincapsdata/qemu_8.2.0.aarch64.xml index ce17865e24..98346d23d3 100644 --- a/tests/domaincapsdata/qemu_8.2.0.aarch64.xml +++ b/tests/domaincapsdata/qemu_8.2.0.aarch64.xml @@ -123,6 +123,7 @@ <value>rdp</value> <value>egl-headless</value> <value>dbus</value> + <value>gtk</value> </enum> </graphics> <video supported='yes'> diff --git a/tests/domaincapsdata/qemu_8.2.0.armv7l.xml b/tests/domaincapsdata/qemu_8.2.0.armv7l.xml index ee653c0c49..77d90635cd 100644 --- a/tests/domaincapsdata/qemu_8.2.0.armv7l.xml +++ b/tests/domaincapsdata/qemu_8.2.0.armv7l.xml @@ -73,6 +73,7 @@ <value>spice</value> <value>egl-headless</value> <value>dbus</value> + <value>gtk</value> </enum> </graphics> <video supported='yes'> diff --git a/tests/domaincapsdata/qemu_9.0.0.sparc.xml b/tests/domaincapsdata/qemu_9.0.0.sparc.xml index c7862f5842..396b40e221 100644 --- a/tests/domaincapsdata/qemu_9.0.0.sparc.xml +++ b/tests/domaincapsdata/qemu_9.0.0.sparc.xml @@ -64,6 +64,7 @@ <value>spice</value> <value>egl-headless</value> <value>dbus</value> + <value>gtk</value> </enum> </graphics> <video supported='yes'> diff --git a/tests/domaincapsdata/qemu_9.1.0.s390x.xml b/tests/domaincapsdata/qemu_9.1.0.s390x.xml index b73e0d0688..e6345ebffb 100644 --- a/tests/domaincapsdata/qemu_9.1.0.s390x.xml +++ b/tests/domaincapsdata/qemu_9.1.0.s390x.xml @@ -208,6 +208,7 @@ <value>rdp</value> <value>egl-headless</value> <value>dbus</value> + <value>gtk</value> </enum> </graphics> <video supported='yes'> diff --git a/tests/domaincapsdata/qemu_9.2.0.s390x.xml b/tests/domaincapsdata/qemu_9.2.0.s390x.xml index 605a3af5c7..3dac7c0462 100644 --- a/tests/domaincapsdata/qemu_9.2.0.s390x.xml +++ b/tests/domaincapsdata/qemu_9.2.0.s390x.xml @@ -208,6 +208,7 @@ <value>rdp</value> <value>egl-headless</value> <value>dbus</value> + <value>gtk</value> </enum> </graphics> <video supported='yes'> diff --git a/tests/qemucapabilitiesdata/caps_10.0.0_s390x.xml b/tests/qemucapabilitiesdata/caps_10.0.0_s390x.xml index be2e91ed92..078f5cea12 100644 --- a/tests/qemucapabilitiesdata/caps_10.0.0_s390x.xml +++ b/tests/qemucapabilitiesdata/caps_10.0.0_s390x.xml @@ -133,6 +133,7 @@ <flag name='migrate-incoming.exit-on-error'/> <flag name='virtio-mem-ccw'/> <flag name='shim'/> + <flag name='gtk'/> <version>9002050</version> <microcodeVersion>39100285</microcodeVersion> <package>v9.2.0-1203-gd6430c17d7</package> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0_ppc64.xml b/tests/qemucapabilitiesdata/caps_7.0.0_ppc64.xml index d78c239372..6dc012bcff 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0_ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0_ppc64.xml @@ -149,6 +149,7 @@ <flag name='usb-mtp'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='gtk'/> <version>7000000</version> <microcodeVersion>42900243</microcodeVersion> <package>v7.0.0</package> diff --git a/tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml b/tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml index d6edb65e96..97728e0141 100644 --- a/tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml @@ -150,6 +150,7 @@ <flag name='usb-mtp'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='gtk'/> <version>7001000</version> <microcodeVersion>42900244</microcodeVersion> <package>v7.1.0</package> diff --git a/tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml b/tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml index fe318e0a52..bf9a38e123 100644 --- a/tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml +++ b/tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml @@ -145,6 +145,7 @@ <flag name='usb-mtp'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='gtk'/> <version>7002000</version> <microcodeVersion>0</microcodeVersion> <package>qemu-7.2.0-6.fc37</package> diff --git a/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml index 837502c336..d21b2cfddb 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml @@ -160,6 +160,7 @@ <flag name='virtio-sound'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='gtk'/> <version>8002000</version> <microcodeVersion>61700246</microcodeVersion> <package>v8.2.0</package> diff --git a/tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml b/tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml index f062f31abc..14013d1c1e 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml @@ -167,6 +167,7 @@ <flag name='virtio-sound'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='gtk'/> <version>8002000</version> <microcodeVersion>0</microcodeVersion> <package>qemu-8.2.0-7.fc39</package> diff --git a/tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.xml b/tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.xml index 2a37631381..37e8dbe371 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.xml @@ -150,6 +150,7 @@ <flag name='virtio-sound'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='gtk'/> <version>8002000</version> <microcodeVersion>106300246</microcodeVersion> <package>v8.2.0</package> diff --git a/tests/qemucapabilitiesdata/caps_9.0.0_sparc.xml b/tests/qemucapabilitiesdata/caps_9.0.0_sparc.xml index 38835ba0cb..703ec9a5a8 100644 --- a/tests/qemucapabilitiesdata/caps_9.0.0_sparc.xml +++ b/tests/qemucapabilitiesdata/caps_9.0.0_sparc.xml @@ -70,6 +70,7 @@ <flag name='blockjob.backing-mask-protocol'/> <flag name='display-reload'/> <flag name='netdev.user'/> + <flag name='gtk'/> <version>9000000</version> <microcodeVersion>0</microcodeVersion> <package>qemu-9.0.0-1.fc40</package> diff --git a/tests/qemucapabilitiesdata/caps_9.1.0_s390x.xml b/tests/qemucapabilitiesdata/caps_9.1.0_s390x.xml index 0d566d13d5..d046779f9a 100644 --- a/tests/qemucapabilitiesdata/caps_9.1.0_s390x.xml +++ b/tests/qemucapabilitiesdata/caps_9.1.0_s390x.xml @@ -125,6 +125,7 @@ <flag name='netdev.user'/> <flag name='query-cpu-model-expansion.deprecated-props'/> <flag name='migrate-incoming.exit-on-error'/> + <flag name='gtk'/> <version>9001000</version> <microcodeVersion>39100246</microcodeVersion> <package>v9.1.0</package> diff --git a/tests/qemucapabilitiesdata/caps_9.2.0_s390x.xml b/tests/qemucapabilitiesdata/caps_9.2.0_s390x.xml index e1323f9b72..9b201aca19 100644 --- a/tests/qemucapabilitiesdata/caps_9.2.0_s390x.xml +++ b/tests/qemucapabilitiesdata/caps_9.2.0_s390x.xml @@ -128,6 +128,7 @@ <flag name='netdev-stream-reconnect-miliseconds'/> <flag name='query-cpu-model-expansion.deprecated-props'/> <flag name='migrate-incoming.exit-on-error'/> + <flag name='gtk'/> <version>9002000</version> <microcodeVersion>39100247</microcodeVersion> <package>v9.2.0</package> -- 2.49.0
On Mon, Apr 28, 2025 at 12:22:22 +0200, Kirill Shchetiniuk via Devel wrote:
From: Kirill Shchetiniuk <kshcheti@redhat.com>
Introduce GTK display support with OpenGL for the QEMU driver.
- Add new XML options for GTK display type. - Include capability flags for the QEMU driver.
Note: The `QEMU_CAPS_GTK_GL` flag cannot yet be checked, so device definition validation is incomplete. A placeholder is left for future implementation, when the GTK along with OpenGL capability check would be available in QEMU.
Can you elaborate? WHy can't it be checked?
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/570
Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> ---
You will need to split this patch. At least the capability flag needs to be a separate commit that only adds the capability flag. I'm also missing qemuxmlconftest test cases and docs/formatdomain.rst documentation changes. [...]
36 files changed, 218 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 542d6ade91..73550f7fcf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -960,6 +960,7 @@ VIR_ENUM_IMPL(virDomainGraphics, "spice", "egl-headless", "dbus", + "gtk", );
VIR_ENUM_IMPL(virDomainGraphicsListen, @@ -2054,6 +2055,12 @@ void virDomainGraphicsDefFree(virDomainGraphicsDef *def) g_free(def->data.dbus.rendernode); break;
+ case VIR_DOMAIN_GRAPHICS_TYPE_GTK: + g_free(def->data.gtk.display); + g_free(def->data.gtk.xauth); + g_free(def->data.gtk.rendernode); + break; + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; } @@ -12199,6 +12206,39 @@ virDomainGraphicsDefParseXMLDBus(virDomainGraphicsDef *def, }
+static int +virDomainGraphicsDefParseXMLGTK(virDomainGraphicsDef *def, + xmlNodePtr node, + xmlXPathContextPtr ctxt) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt) + xmlNodePtr glNode; + virTristateBool fullscreen; + + ctxt->node = node; + + if (virXMLPropTristateBool(node, "fullscreen", VIR_XML_PROP_NONE, + &fullscreen) < 0)
You parse a tristate ...
+ return -1; + + virTristateBoolToBool(fullscreen, &def->data.gtk.fullscreen);
... but store it only as boolean?
+ def->data.gtk.xauth = virXMLPropString(node, "xauth"); + def->data.gtk.display = virXMLPropString(node, "display"); + + if ((glNode = virXPathNode("./gl", ctxt))) { + def->data.gtk.rendernode = virXMLPropString(glNode, + "rendernode"); + + if (virXMLPropTristateBool(glNode, "enable", + VIR_XML_PROP_REQUIRED, + &def->data.gtk.gl) < 0) + return -1; + } + + return 0; +} + + virDomainGraphicsDef * virDomainGraphicsDefNew(virDomainXMLOption *xmlopt) {
[...]
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a804335c85..de7ce95499 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -732,6 +732,8 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 475 */ "virtio-scsi.iothread-mapping", /* QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING */ + "gtk", /* QEMU_CAPS_GTK */ + "gtk-gl", /* QEMU_CAPS_GTK_GL */ );
@@ -1602,6 +1604,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "set-numa-node/arg-type/+hmat-lb", QEMU_CAPS_NUMA_HMAT }, { "query-cpu-model-expansion/ret-type/deprecated-props", QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_DEPRECATED_PROPS }, { "migrate-incoming/arg-type/exit-on-error", QEMU_CAPS_MIGRATE_INCOMING_EXIT_ON_ERROR }, + { "query-display-options/ret-type/type/^gtk", QEMU_CAPS_GTK }, };
typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps; @@ -6499,6 +6502,8 @@ virQEMUCapsFillDomainDeviceGraphicsCaps(virQEMUDriverConfig *cfg, VIR_DOMAIN_GRAPHICS_TYPE_RDP); } } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_GTK)) + VIR_DOMAIN_CAPS_ENUM_SET(dev->type, VIR_DOMAIN_GRAPHICS_TYPE_GTK); }
[...]
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index b2c3c9e2f6..be92785d71 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4523,6 +4523,17 @@ qemuValidateDomainDeviceDefRDPGraphics(const virDomainGraphicsDef *graphics) }
+static int +qemuValidateDomainDeviceDefGTKGraphics(const virDomainGraphicsDef *graphics G_GNUC_UNUSED, + virQEMUCaps *qemuCaps G_GNUC_UNUSED) +{ + /* TODO: OpenGL check */ + /* For now it's not possible to check if gtk-gl capability is available in QEMU */ + /* Add cappability check later when will be possible to gather the capability from QEMU */
So why does the patch add the capability then? Or is this comment no longer applicable?
+ return 0; +} + + static int qemuValidateDomainDeviceDefGraphics(const virDomainGraphicsDef *graphics, const virDomainDef *def,
On Mon, Apr 28, 2025 at 12:33:19 +0200, Peter Krempa via Devel wrote:
On Mon, Apr 28, 2025 at 12:22:22 +0200, Kirill Shchetiniuk via Devel wrote:
From: Kirill Shchetiniuk <kshcheti@redhat.com>
Introduce GTK display support with OpenGL for the QEMU driver.
- Add new XML options for GTK display type. - Include capability flags for the QEMU driver.
Note: The `QEMU_CAPS_GTK_GL` flag cannot yet be checked, so device definition validation is incomplete. A placeholder is left for future implementation, when the GTK along with OpenGL capability check would be available in QEMU.
Can you elaborate? WHy can't it be checked?
Since the 'gtk' backend in support was introduced predating qemu-2.12. I'm guessing what's happening is that you tried to use DO_TEST_CAPS_LATEST which failed. For the above the solution is not to ignore the check but rather one of: 1) update the 'x86_64' test data with a qemu built with gtk support 2) add a variant of the test data from a qemu built with gtk support 3) use DO_TEST_FULL and ARG_FLAGS to add the capability for the test file 4) base the tesst on one of the other arches which were built wit gtk Since I'm the one maintaining the capability dumps, maitaining a variant for this is overkill; adding it to the main build is possible (I can send the update). I was historically building qemu with '--disable-gtk' as it mirrored what fedora was doing but it seems that GTK is enabled in the current fedora build. Option 3) is acceptable for a niche feature and option 4) can theoretically regress in the future.
On Mon, Apr 28, 2025 at 12:57:42 +0200, Peter Krempa via Devel wrote:
On Mon, Apr 28, 2025 at 12:33:19 +0200, Peter Krempa via Devel wrote:
On Mon, Apr 28, 2025 at 12:22:22 +0200, Kirill Shchetiniuk via Devel wrote:
[...]
Since the 'gtk' backend in support was introduced predating qemu-2.12. I'm guessing what's happening is that you tried to use DO_TEST_CAPS_LATEST which failed.
For the above the solution is not to ignore the check but rather one of:
1) update the 'x86_64' test data with a qemu built with gtk support
[...]
Since I'm the one maintaining the capability dumps, maitaining a variant for this is overkill; adding it to the main build is possible (I can send the update).
Update of the test data adding GTK support: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/5LQ7A...
On Mon, Apr 28, 2025 at 12:33:19PM +0200, Peter Krempa wrote:
On Mon, Apr 28, 2025 at 12:22:22 +0200, Kirill Shchetiniuk via Devel wrote:
From: Kirill Shchetiniuk <kshcheti@redhat.com>
Introduce GTK display support with OpenGL for the QEMU driver.
- Add new XML options for GTK display type. - Include capability flags for the QEMU driver.
Note: The `QEMU_CAPS_GTK_GL` flag cannot yet be checked, so device definition validation is incomplete. A placeholder is left for future implementation, when the GTK along with OpenGL capability check would be available in QEMU.
Can you elaborate? WHy can't it be checked?
So I have discussed this with Michal Privoznik, it's possible to directly gather if GTK capability is enable, but there is now straight way to check if OpenGL for GTK is enabled too. Michal purposed a way to check this with a small hack, check if GTK capability is presented along with egl-headless, but we decided better not to check OpenGL for GTK cabability for now, as it's not straightforward approch, and I decided to leave a placeholer for the future check to do not forget to add this later.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/570
Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> ---
You will need to split this patch.
At least the capability flag needs to be a separate commit that only adds the capability flag.
I'm also missing qemuxmlconftest test cases and docs/formatdomain.rst documentation changes.
Sure, will this with next series, thanks!
[...]
36 files changed, 218 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 542d6ade91..73550f7fcf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -960,6 +960,7 @@ VIR_ENUM_IMPL(virDomainGraphics, "spice", "egl-headless", "dbus", + "gtk", );
VIR_ENUM_IMPL(virDomainGraphicsListen, @@ -2054,6 +2055,12 @@ void virDomainGraphicsDefFree(virDomainGraphicsDef *def) g_free(def->data.dbus.rendernode); break;
+ case VIR_DOMAIN_GRAPHICS_TYPE_GTK: + g_free(def->data.gtk.display); + g_free(def->data.gtk.xauth); + g_free(def->data.gtk.rendernode); + break; + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; } @@ -12199,6 +12206,39 @@ virDomainGraphicsDefParseXMLDBus(virDomainGraphicsDef *def, }
+static int +virDomainGraphicsDefParseXMLGTK(virDomainGraphicsDef *def, + xmlNodePtr node, + xmlXPathContextPtr ctxt) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt) + xmlNodePtr glNode; + virTristateBool fullscreen; + + ctxt->node = node; + + if (virXMLPropTristateBool(node, "fullscreen", VIR_XML_PROP_NONE, + &fullscreen) < 0)
You parse a tristate ...
+ return -1; + + virTristateBoolToBool(fullscreen, &def->data.gtk.fullscreen);
... but store it only as boolean?
So I saw the same approach in virDomainGraphicsDefParseXMLSDL and decided to use it too. Do you suggest virXPathBoolean instead or something else?
+ def->data.gtk.xauth = virXMLPropString(node, "xauth"); + def->data.gtk.display = virXMLPropString(node, "display"); + + if ((glNode = virXPathNode("./gl", ctxt))) { + def->data.gtk.rendernode = virXMLPropString(glNode, + "rendernode"); + + if (virXMLPropTristateBool(glNode, "enable", + VIR_XML_PROP_REQUIRED, + &def->data.gtk.gl) < 0) + return -1; + } + + return 0; +} + + virDomainGraphicsDef * virDomainGraphicsDefNew(virDomainXMLOption *xmlopt) {
[...]
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a804335c85..de7ce95499 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -732,6 +732,8 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 475 */ "virtio-scsi.iothread-mapping", /* QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING */ + "gtk", /* QEMU_CAPS_GTK */ + "gtk-gl", /* QEMU_CAPS_GTK_GL */ );
@@ -1602,6 +1604,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "set-numa-node/arg-type/+hmat-lb", QEMU_CAPS_NUMA_HMAT }, { "query-cpu-model-expansion/ret-type/deprecated-props", QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_DEPRECATED_PROPS }, { "migrate-incoming/arg-type/exit-on-error", QEMU_CAPS_MIGRATE_INCOMING_EXIT_ON_ERROR }, + { "query-display-options/ret-type/type/^gtk", QEMU_CAPS_GTK }, };
typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps; @@ -6499,6 +6502,8 @@ virQEMUCapsFillDomainDeviceGraphicsCaps(virQEMUDriverConfig *cfg, VIR_DOMAIN_GRAPHICS_TYPE_RDP); } } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_GTK)) + VIR_DOMAIN_CAPS_ENUM_SET(dev->type, VIR_DOMAIN_GRAPHICS_TYPE_GTK); }
[...]
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index b2c3c9e2f6..be92785d71 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4523,6 +4523,17 @@ qemuValidateDomainDeviceDefRDPGraphics(const virDomainGraphicsDef *graphics) }
+static int +qemuValidateDomainDeviceDefGTKGraphics(const virDomainGraphicsDef *graphics G_GNUC_UNUSED, + virQEMUCaps *qemuCaps G_GNUC_UNUSED) +{ + /* TODO: OpenGL check */ + /* For now it's not possible to check if gtk-gl capability is available in QEMU */ + /* Add cappability check later when will be possible to gather the capability from QEMU */
So why does the patch add the capability then? Or is this comment no longer applicable?
So I didn't know how to do it better, decided to leave a placeholder for future changes. With next series will remove the yet unused gtk-gl capability, and leave the placeholder only.
+ return 0; +} + + static int qemuValidateDomainDeviceDefGraphics(const virDomainGraphicsDef *graphics, const virDomainDef *def,
On Mon, Apr 28, 2025 at 12:22:22 +0200, Kirill Shchetiniuk via Devel wrote:
From: Kirill Shchetiniuk <kshcheti@redhat.com>
Introduce GTK display support with OpenGL for the QEMU driver.
- Add new XML options for GTK display type. - Include capability flags for the QEMU driver.
Note: The `QEMU_CAPS_GTK_GL` flag cannot yet be checked, so device definition validation is incomplete. A placeholder is left for future implementation, when the GTK along with OpenGL capability check would be available in QEMU.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/570
Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> ---
[...]
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 5597d5a66b..61c1d3ad97 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -4593,6 +4593,34 @@ </element> </optional> </group> + <group> + <attribute name="type"> + <value>gtk</value> + </attribute> + <optional> + <attribute name="display"> + <text/> + </attribute> + </optional> + <optional> + <attribute name="xauth"> + <text/>
Since this is a path you want to use: <ref name="absFilePath"/>
+ </attribute> + </optional> + <optional>
[...]
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e6d308534f..e97992ff56 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8483,6 +8483,34 @@ qemuBuildGraphicsDBusCommandLine(virDomainDef *def, }
+static int +qemuBuildGraphicsGTKCommandLine(virQEMUDriverConfig *cfg G_GNUC_UNUSED,
Why are you passing this argument if it's not used?
+ virCommand *cmd, + virDomainGraphicsDef *graphics) +{ + g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER; + + if (graphics->data.gtk.xauth) + virCommandAddEnvPair(cmd, "XAUTHORITY", graphics->data.gtk.xauth);
I also realized that this is passin the path to the '.Xauthority' file to the qemu process. Note that in the default privileged configuration where qemu runs as a different process this will not work as the qemu process will not have access to the cookie inside the file due to permissions. Also you can't simply apply security labelling on that file because it would break other things. I don't have a suggestion how to fix this, besides perhaps copying the authority file at startup somewhere for the qemu process to be able to access it. Either way in the current state it will require mentioning this caveat.
On Mon, Apr 28, 2025 at 02:40:49PM +0200, Peter Krempa wrote:
On Mon, Apr 28, 2025 at 12:22:22 +0200, Kirill Shchetiniuk via Devel wrote:
From: Kirill Shchetiniuk <kshcheti@redhat.com>
Introduce GTK display support with OpenGL for the QEMU driver.
- Add new XML options for GTK display type. - Include capability flags for the QEMU driver.
Note: The `QEMU_CAPS_GTK_GL` flag cannot yet be checked, so device definition validation is incomplete. A placeholder is left for future implementation, when the GTK along with OpenGL capability check would be available in QEMU.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/570
Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> ---
[...]
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 5597d5a66b..61c1d3ad97 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -4593,6 +4593,34 @@ </element> </optional> </group> + <group> + <attribute name="type"> + <value>gtk</value> + </attribute> + <optional> + <attribute name="display"> + <text/> + </attribute> + </optional> + <optional> + <attribute name="xauth"> + <text/>
Since this is a path you want to use:
<ref name="absFilePath"/>
Will fix it with next series, also saw the <text/> approach for xauth in SDL device schema, will fix it there too later with separate patch, to maintain it consistent.
+ </attribute> + </optional> + <optional>
[...]
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e6d308534f..e97992ff56 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8483,6 +8483,34 @@ qemuBuildGraphicsDBusCommandLine(virDomainDef *def, }
+static int +qemuBuildGraphicsGTKCommandLine(virQEMUDriverConfig *cfg G_GNUC_UNUSED,
Why are you passing this argument if it's not used?
+ virCommand *cmd, + virDomainGraphicsDef *graphics) +{ + g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER; + + if (graphics->data.gtk.xauth) + virCommandAddEnvPair(cmd, "XAUTHORITY", graphics->data.gtk.xauth);
I also realized that this is passin the path to the '.Xauthority' file to the qemu process. Note that in the default privileged configuration where qemu runs as a different process this will not work as the qemu process will not have access to the cookie inside the file due to permissions.
Also you can't simply apply security labelling on that file because it would break other things.
I don't have a suggestion how to fix this, besides perhaps copying the authority file at startup somewhere for the qemu process to be able to access it.
Either way in the current state it will require mentioning this caveat.
Yeah, I already noticed that issue with access to xauth file. I generaly thought to make an `xauth` attribute mandatory to force the user to create a readable file with cookie for qemu user, but not sure if it's a good approach.
participants (2)
-
Kirill Shchetiniuk -
Peter Krempa