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 <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.
> >
> > Resolves: https://gitlab.com/libvirt/libvirt/-/issues/570
> >
> > Signed-off-by: Kirill Shchetiniuk <kshch...@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.
