[PATCH 12/19] qemu: limit to one <graphics type='rdp'>

marcandre.lureau@redhat.com posted 19 patches 7 months, 1 week ago
There is a newer version of this series
[PATCH 12/19] qemu: limit to one <graphics type='rdp'>
Posted by marcandre.lureau@redhat.com 7 months, 1 week ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 src/qemu/qemu_command.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f08dba7988..dc142b366a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8420,6 +8420,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfig *cfg,
 
             break;
         case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
+            break;
         case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("graphics type '%1$s' not supported"),
@@ -9974,6 +9975,7 @@ qemuBuildCommandLineValidate(virQEMUDriver *driver,
     int spice = 0;
     int egl_headless = 0;
     int dbus = 0;
+    int rdp = 0;
 
     if (!driver->privileged) {
         /* If we have no cgroups then we can have no tunings that
@@ -10022,15 +10024,17 @@ qemuBuildCommandLineValidate(virQEMUDriver *driver,
             ++dbus;
             break;
         case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
+            ++rdp;
+            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) {
+    if (sdl > 1 || vnc > 1 || spice > 1 || egl_headless > 1 || dbus > 1 || rdp > 1) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("only 1 graphics device of each type (sdl, vnc, spice, headless, dbus) is supported"));
+                       _("only 1 graphics device of each type (sdl, vnc, spice, headless, dbus, rdp) is supported"));
         return -1;
     }
 
-- 
2.47.0
Re: [PATCH 12/19] qemu: limit to one <graphics type='rdp'>
Posted by Daniel P. Berrangé 7 months, 1 week ago
On Wed, Jan 29, 2025 at 05:40:34PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  src/qemu/qemu_command.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index f08dba7988..dc142b366a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8420,6 +8420,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfig *cfg,
>  
>              break;
>          case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
> +            break;
>          case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                             _("graphics type '%1$s' not supported"),
> @@ -9974,6 +9975,7 @@ qemuBuildCommandLineValidate(virQEMUDriver *driver,
>      int spice = 0;
>      int egl_headless = 0;
>      int dbus = 0;
> +    int rdp = 0;
>  
>      if (!driver->privileged) {
>          /* If we have no cgroups then we can have no tunings that
> @@ -10022,15 +10024,17 @@ qemuBuildCommandLineValidate(virQEMUDriver *driver,
>              ++dbus;
>              break;
>          case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
> +            ++rdp;
> +            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) {
> +    if (sdl > 1 || vnc > 1 || spice > 1 || egl_headless > 1 || dbus > 1 || rdp > 1) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("only 1 graphics device of each type (sdl, vnc, spice, headless, dbus) is supported"));
> +                       _("only 1 graphics device of each type (sdl, vnc, spice, headless, dbus, rdp) is supported"));
>          return -1;
>      }

The 'rdp' type also uses the 'dbus' backend in QEMU, so checking
dbus > 1 || rdp > 1, allows for two uses of the 'dbus' backend.

Hypothetically we could also have future 'vnc' or 'spice' types
which used the dbus QEMU backend for out of process impls.

IOW, we need to be counting QEMU backends here, rather than libvirt
types.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH 12/19] qemu: limit to one <graphics type='rdp'>
Posted by Marc-André Lureau 6 months, 3 weeks ago
Hi

On Wed, Jan 29, 2025 at 5:58 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Jan 29, 2025 at 05:40:34PM +0400, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  src/qemu/qemu_command.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index f08dba7988..dc142b366a 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -8420,6 +8420,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfig *cfg,
> >
> >              break;
> >          case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
> > +            break;
> >          case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
> >              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> >                             _("graphics type '%1$s' not supported"),
> > @@ -9974,6 +9975,7 @@ qemuBuildCommandLineValidate(virQEMUDriver *driver,
> >      int spice = 0;
> >      int egl_headless = 0;
> >      int dbus = 0;
> > +    int rdp = 0;
> >
> >      if (!driver->privileged) {
> >          /* If we have no cgroups then we can have no tunings that
> > @@ -10022,15 +10024,17 @@ qemuBuildCommandLineValidate(virQEMUDriver *driver,
> >              ++dbus;
> >              break;
> >          case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
> > +            ++rdp;
> > +            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) {
> > +    if (sdl > 1 || vnc > 1 || spice > 1 || egl_headless > 1 || dbus > 1 || rdp > 1) {
> >          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > -                       _("only 1 graphics device of each type (sdl, vnc, spice, headless, dbus) is supported"));
> > +                       _("only 1 graphics device of each type (sdl, vnc, spice, headless, dbus, rdp) is supported"));
> >          return -1;
> >      }
>
> The 'rdp' type also uses the 'dbus' backend in QEMU, so checking
> dbus > 1 || rdp > 1, allows for two uses of the 'dbus' backend.
>
> Hypothetically we could also have future 'vnc' or 'spice' types
> which used the dbus QEMU backend for out of process impls.

I don't think qemu supports multiple instances of the same graphics
backends, this is true for -display dbus.

If we were to have vnc or spice server out of process (I doubt someone
will do one for spice, but I started one for vnc, fwiw), they would
use the same dbus graphics instance. This check is thus still valid,
only one of each kind is supported atm. Or am I missing something?

>
> IOW, we need to be counting QEMU backends here, rather than libvirt
> types.
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>