On Mon, Apr 28, 2025 at 12:22:22 +0200, Kirill Shchetiniuk via Devel wrote: > From: Kirill Shchetiniuk <kshch...@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 <kshch...@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,
