[v1] Add qemu RDP server support

[PATCH 05/19] qemu: report an error for unsupported graphics

Posted by marcandre.lureau@redhat.com 7 months, 1 week ago

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Without an error message, it can be tedious to figure out failure to start.

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

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1f20189e89..f08dba7988 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8421,6 +8421,9 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfig *cfg,
             break;
         case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
         case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("graphics type '%1$s' not supported"),
+                           virDomainGraphicsTypeToString(graphics->type));
             return -1;
         case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
         default:
-- 
2.47.0

Re: [PATCH 05/19] qemu: report an error for unsupported graphics

Posted by Daniel P. Berrangé 6 months, 4 weeks ago

On Wed, Jan 29, 2025 at 05:40:27PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Without an error message, it can be tedious to figure out failure to start.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  src/qemu/qemu_command.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 1f20189e89..f08dba7988 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8421,6 +8421,9 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfig *cfg,
>              break;
>          case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
>          case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("graphics type '%1$s' not supported"),
> +                           virDomainGraphicsTypeToString(graphics->type));
>              return -1;

We don't report such errors when building the command line as they should
have already been caught & reported by qemuValidateDomainDef, in this
case we have qemuValidateDomainDeviceDefGraphics which reports any
unsupported graphics types, based on the capabilities we probe. If that
check is wrong/incomplete, we should fix the validate method

>          case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
>          default:

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 05/19] qemu: report an error for unsupported graphics

Posted by Marc-André Lureau 6 months, 3 weeks ago

Hi

On Wed, Feb 12, 2025 at 7:44 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Jan 29, 2025 at 05:40:27PM +0400, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Without an error message, it can be tedious to figure out failure to start.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  src/qemu/qemu_command.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 1f20189e89..f08dba7988 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -8421,6 +8421,9 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfig *cfg,
> >              break;
> >          case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
> >          case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
> > +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > +                           _("graphics type '%1$s' not supported"),
> > +                           virDomainGraphicsTypeToString(graphics->type));
> >              return -1;
>
> We don't report such errors when building the command line as they should
> have already been caught & reported by qemuValidateDomainDef, in this
> case we have qemuValidateDomainDeviceDefGraphics which reports any
> unsupported graphics types, based on the capabilities we probe. If that
> check is wrong/incomplete, we should fix the validate method

I suppose I added it to help me figure out what were the missing
pieces. It's a bit tedious when you have a flow that returns an error
without any readable trace. I am fine to drop it..

> >          case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
> >          default:
>
> 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 :|
>