[PATCH v3 09/20] ui/vnc: remove use of error_printf_unless_qmp()

Daniel P. Berrangé posted 20 patches 2 weeks, 3 days ago
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, Christian Schoenebeck <qemu_oss@crudebyte.com>, Markus Armbruster <armbru@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Dr. David Alan Gilbert" <dave@treblig.org>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Stefan Weil <sw@weilnetz.de>
There is a newer version of this series
[PATCH v3 09/20] ui/vnc: remove use of error_printf_unless_qmp()
Posted by Daniel P. Berrangé 2 weeks, 3 days ago
The error_printf_unless_qmp() will print to the monitor if the current
one is HMP, if it is QMP nothing will be printed, otherwise stderr
will be used.

This scenario is easily handled by checking !monitor_cur_is_qmp() and
then calling the error_printf() function.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 ui/vnc.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 68ca4a68e7..439d586358 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3530,8 +3530,10 @@ int vnc_display_password(const char *id, const char *password)
         return -EINVAL;
     }
     if (vd->auth == VNC_AUTH_NONE) {
-        error_printf_unless_qmp("If you want use passwords please enable "
-                                "password auth using '-vnc ${dpy},password'.\n");
+        if (!monitor_cur_is_qmp()) {
+            error_printf("If you want use passwords please enable "
+                         "password auth using '-vnc ${dpy},password'.\n");
+        }
         return -EINVAL;
     }
 
@@ -3570,9 +3572,11 @@ static void vnc_display_print_local_addr(VncDisplay *vd)
         qapi_free_SocketAddress(addr);
         return;
     }
-    error_printf_unless_qmp("VNC server running on %s:%s\n",
-                            addr->u.inet.host,
-                            addr->u.inet.port);
+    if (!monitor_cur_is_qmp()) {
+        error_printf("VNC server running on %s:%s\n",
+                     addr->u.inet.host,
+                     addr->u.inet.port);
+    }
     qapi_free_SocketAddress(addr);
 }
 
-- 
2.50.1


Re: [PATCH v3 09/20] ui/vnc: remove use of error_printf_unless_qmp()
Posted by Richard Henderson 2 weeks, 2 days ago
On 9/10/25 18:03, Daniel P. Berrangé wrote:
> The error_printf_unless_qmp() will print to the monitor if the current
> one is HMP, if it is QMP nothing will be printed, otherwise stderr
> will be used.
> 
> This scenario is easily handled by checking !monitor_cur_is_qmp() and
> then calling the error_printf() function.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   ui/vnc.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 68ca4a68e7..439d586358 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3530,8 +3530,10 @@ int vnc_display_password(const char *id, const char *password)
>           return -EINVAL;
>       }
>       if (vd->auth == VNC_AUTH_NONE) {
> -        error_printf_unless_qmp("If you want use passwords please enable "
> -                                "password auth using '-vnc ${dpy},password'.\n");
> +        if (!monitor_cur_is_qmp()) {
> +            error_printf("If you want use passwords please enable "
> +                         "password auth using '-vnc ${dpy},password'.\n");
> +        }
>           return -EINVAL;
>       }
>   
> @@ -3570,9 +3572,11 @@ static void vnc_display_print_local_addr(VncDisplay *vd)
>           qapi_free_SocketAddress(addr);
>           return;
>       }
> -    error_printf_unless_qmp("VNC server running on %s:%s\n",
> -                            addr->u.inet.host,
> -                            addr->u.inet.port);
> +    if (!monitor_cur_is_qmp()) {
> +        error_printf("VNC server running on %s:%s\n",
> +                     addr->u.inet.host,
> +                     addr->u.inet.port);
> +    }
>       qapi_free_SocketAddress(addr);
>   }
>   

With monitor_cur_hmp, you can use monitor_printf directly.


r~

Re: [PATCH v3 09/20] ui/vnc: remove use of error_printf_unless_qmp()
Posted by Daniel P. Berrangé 2 weeks, 2 days ago
On Thu, Sep 11, 2025 at 05:54:48PM +0000, Richard Henderson wrote:
> On 9/10/25 18:03, Daniel P. Berrangé wrote:
> > The error_printf_unless_qmp() will print to the monitor if the current
> > one is HMP, if it is QMP nothing will be printed, otherwise stderr
> > will be used.
> > 
> > This scenario is easily handled by checking !monitor_cur_is_qmp() and
> > then calling the error_printf() function.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >   ui/vnc.c | 14 +++++++++-----
> >   1 file changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/ui/vnc.c b/ui/vnc.c
> > index 68ca4a68e7..439d586358 100644
> > --- a/ui/vnc.c
> > +++ b/ui/vnc.c
> > @@ -3530,8 +3530,10 @@ int vnc_display_password(const char *id, const char *password)
> >           return -EINVAL;
> >       }
> >       if (vd->auth == VNC_AUTH_NONE) {
> > -        error_printf_unless_qmp("If you want use passwords please enable "
> > -                                "password auth using '-vnc ${dpy},password'.\n");
> > +        if (!monitor_cur_is_qmp()) {
> > +            error_printf("If you want use passwords please enable "
> > +                         "password auth using '-vnc ${dpy},password'.\n");
> > +        }
> >           return -EINVAL;
> >       }
> > @@ -3570,9 +3572,11 @@ static void vnc_display_print_local_addr(VncDisplay *vd)
> >           qapi_free_SocketAddress(addr);
> >           return;
> >       }
> > -    error_printf_unless_qmp("VNC server running on %s:%s\n",
> > -                            addr->u.inet.host,
> > -                            addr->u.inet.port);
> > +    if (!monitor_cur_is_qmp()) {
> > +        error_printf("VNC server running on %s:%s\n",
> > +                     addr->u.inet.host,
> > +                     addr->u.inet.port);
> > +    }
> >       qapi_free_SocketAddress(addr);
> >   }
> 
> With monitor_cur_hmp, you can use monitor_printf directly.

We still need to be able to print to stderr when there is neither HMP
nor QMP, which error_printf gets us, but monitor_printf would not.


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 v3 09/20] ui/vnc: remove use of error_printf_unless_qmp()
Posted by Markus Armbruster 1 week, 2 days ago
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Sep 11, 2025 at 05:54:48PM +0000, Richard Henderson wrote:
>> On 9/10/25 18:03, Daniel P. Berrangé wrote:
>> > The error_printf_unless_qmp() will print to the monitor if the current
>> > one is HMP, if it is QMP nothing will be printed, otherwise stderr
>> > will be used.
>> > 
>> > This scenario is easily handled by checking !monitor_cur_is_qmp() and
>> > then calling the error_printf() function.
>> > 
>> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> > ---
>> >   ui/vnc.c | 14 +++++++++-----
>> >   1 file changed, 9 insertions(+), 5 deletions(-)
>> > 
>> > diff --git a/ui/vnc.c b/ui/vnc.c
>> > index 68ca4a68e7..439d586358 100644
>> > --- a/ui/vnc.c
>> > +++ b/ui/vnc.c
>> > @@ -3530,8 +3530,10 @@ int vnc_display_password(const char *id, const char *password)
>> >           return -EINVAL;
>> >       }
>> >       if (vd->auth == VNC_AUTH_NONE) {
>> > -        error_printf_unless_qmp("If you want use passwords please enable "
>> > -                                "password auth using '-vnc ${dpy},password'.\n");
>> > +        if (!monitor_cur_is_qmp()) {
>> > +            error_printf("If you want use passwords please enable "

Maybe use the opportunity to put a comma after passwords?

>> > +                         "password auth using '-vnc ${dpy},password'.\n");
>> > +        }
>> >           return -EINVAL;
>> >       }
>> > @@ -3570,9 +3572,11 @@ static void vnc_display_print_local_addr(VncDisplay *vd)
>> >           qapi_free_SocketAddress(addr);
>> >           return;
>> >       }
>> > -    error_printf_unless_qmp("VNC server running on %s:%s\n",
>> > -                            addr->u.inet.host,
>> > -                            addr->u.inet.port);
>> > +    if (!monitor_cur_is_qmp()) {
>> > +        error_printf("VNC server running on %s:%s\n",
>> > +                     addr->u.inet.host,
>> > +                     addr->u.inet.port);
>> > +    }
>> >       qapi_free_SocketAddress(addr);
>> >   }
>> 
>> With monitor_cur_hmp, you can use monitor_printf directly.
>
> We still need to be able to print to stderr when there is neither HMP
> nor QMP, which error_printf gets us, but monitor_printf would not.

monitor_printf() is fine when it's obvious that we're in HMP context.
It isn't here.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Re: [PATCH v3 09/20] ui/vnc: remove use of error_printf_unless_qmp()
Posted by Richard Henderson 2 weeks, 3 days ago
On 9/10/25 18:03, Daniel P. Berrangé wrote:
> The error_printf_unless_qmp() will print to the monitor if the current
> one is HMP, if it is QMP nothing will be printed, otherwise stderr
> will be used.
> 
> This scenario is easily handled by checking !monitor_cur_is_qmp() and
> then calling the error_printf() function.
> 
> Signed-off-by: Daniel P. Berrangé<berrange@redhat.com>
> ---
>   ui/vnc.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v3 09/20] ui/vnc: remove use of error_printf_unless_qmp()
Posted by Dr. David Alan Gilbert 2 weeks, 3 days ago
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> The error_printf_unless_qmp() will print to the monitor if the current
> one is HMP, if it is QMP nothing will be printed, otherwise stderr
> will be used.
> 
> This scenario is easily handled by checking !monitor_cur_is_qmp() and
> then calling the error_printf() function.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  ui/vnc.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 68ca4a68e7..439d586358 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3530,8 +3530,10 @@ int vnc_display_password(const char *id, const char *password)
>          return -EINVAL;
>      }
>      if (vd->auth == VNC_AUTH_NONE) {
> -        error_printf_unless_qmp("If you want use passwords please enable "
> -                                "password auth using '-vnc ${dpy},password'.\n");
> +        if (!monitor_cur_is_qmp()) {
> +            error_printf("If you want use passwords please enable "
> +                         "password auth using '-vnc ${dpy},password'.\n");
> +        }

OK, but while you're here, could you add the missing 'to' please.

Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>

>          return -EINVAL;
>      }
>  
> @@ -3570,9 +3572,11 @@ static void vnc_display_print_local_addr(VncDisplay *vd)
>          qapi_free_SocketAddress(addr);
>          return;
>      }
> -    error_printf_unless_qmp("VNC server running on %s:%s\n",
> -                            addr->u.inet.host,
> -                            addr->u.inet.port);
> +    if (!monitor_cur_is_qmp()) {
> +        error_printf("VNC server running on %s:%s\n",
> +                     addr->u.inet.host,
> +                     addr->u.inet.port);
> +    }
>      qapi_free_SocketAddress(addr);
>  }
>  
> -- 
> 2.50.1
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/