HMP command gdbserver used to emit two error messages for certain
errors.  For instance, with -M none:

    (qemu) gdbserver
    gdbstub: meaningless to attach gdb to a machine without any CPU.
    Could not open gdbserver on device 'tcp::1234'

The first message is the specific error, and the second one a generic
additional message that feels superfluous to me.

Commit c0e6b8b798b (system: propagate Error to gdbserver_start (and
other device setups)) turned the first message into a warning:

    warning: gdbstub: meaningless to attach gdb to a machine without any CPU.
    Could not open gdbserver on device 'tcp::1234'

This is arguably worse.

hmp_gdbserver() passes &error_warn to gdbserver_start(), so that
failure gets reported as warning, and then additionally emits the
generic error on failure.  This is a misuse of &error_warn.

Instead, receive the error in &err and report it, as usual.  With
this, gdbserver reports just the error:

    gdbstub: meaningless to attach gdb to a machine without any CPU.

Cc: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/exec/gdbstub.h | 3 ---
 monitor/hmp-cmds.c     | 7 ++++---
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index a16c0051ce..bd7182c4d3 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -55,9 +55,6 @@ void gdb_unregister_coprocessor_all(CPUState *cpu);
  * system emulation you can use a full chardev spec for your gdbserver
  * port.
  *
- * The error handle should be either &error_fatal (for start-up) or
- * &error_warn (for QMP/HMP initiated sessions).
- *
  * Returns true when server successfully started.
  */
 bool gdbserver_start(const char *port_or_device, Error **errp);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 74a0f56566..33a88ce205 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -280,14 +280,15 @@ void hmp_log(Monitor *mon, const QDict *qdict)
 
 void hmp_gdbserver(Monitor *mon, const QDict *qdict)
 {
+    Error *err = NULL;
     const char *device = qdict_get_try_str(qdict, "device");
+
     if (!device) {
         device = "tcp::" DEFAULT_GDBSTUB_PORT;
     }
 
-    if (!gdbserver_start(device, &error_warn)) {
-        monitor_printf(mon, "Could not open gdbserver on device '%s'\n",
-                       device);
+    if (!gdbserver_start(device, &err)) {
+        error_report_err(err);
     } else if (strcmp(device, "none") == 0) {
         monitor_printf(mon, "Disabled gdbserver\n");
     } else {
-- 
2.49.0

On Fri, Aug 08, 2025 at 10:08:12AM +0200, Markus Armbruster wrote:
> HMP command gdbserver used to emit two error messages for certain
> errors.  For instance, with -M none:
> 
>     (qemu) gdbserver
>     gdbstub: meaningless to attach gdb to a machine without any CPU.
>     Could not open gdbserver on device 'tcp::1234'
> 
> The first message is the specific error, and the second one a generic
> additional message that feels superfluous to me.
> 
> Commit c0e6b8b798b (system: propagate Error to gdbserver_start (and
> other device setups)) turned the first message into a warning:
> 
>     warning: gdbstub: meaningless to attach gdb to a machine without any CPU.
>     Could not open gdbserver on device 'tcp::1234'
> 
> This is arguably worse.
> 
> hmp_gdbserver() passes &error_warn to gdbserver_start(), so that
> failure gets reported as warning, and then additionally emits the
> generic error on failure.  This is a misuse of &error_warn.
> 
> Instead, receive the error in &err and report it, as usual.  With
> this, gdbserver reports just the error:
> 
>     gdbstub: meaningless to attach gdb to a machine without any CPU.

What do you think about an alternative of removing "gdbstub: "
from all the errors raised in  'gdbserver_start' & similar, and
then using

  error_prepend(err, "Could not open gdbserver on device '%s'", device);

in hmp_gdbserver ?

I don't feel too strongly, so in any case for the patch as is,

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


> 
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/exec/gdbstub.h | 3 ---
>  monitor/hmp-cmds.c     | 7 ++++---
>  2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
> index a16c0051ce..bd7182c4d3 100644
> --- a/include/exec/gdbstub.h
> +++ b/include/exec/gdbstub.h
> @@ -55,9 +55,6 @@ void gdb_unregister_coprocessor_all(CPUState *cpu);
>   * system emulation you can use a full chardev spec for your gdbserver
>   * port.
>   *
> - * The error handle should be either &error_fatal (for start-up) or
> - * &error_warn (for QMP/HMP initiated sessions).
> - *
>   * Returns true when server successfully started.
>   */
>  bool gdbserver_start(const char *port_or_device, Error **errp);
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 74a0f56566..33a88ce205 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -280,14 +280,15 @@ void hmp_log(Monitor *mon, const QDict *qdict)
>  
>  void hmp_gdbserver(Monitor *mon, const QDict *qdict)
>  {
> +    Error *err = NULL;
>      const char *device = qdict_get_try_str(qdict, "device");
> +
>      if (!device) {
>          device = "tcp::" DEFAULT_GDBSTUB_PORT;
>      }
>  
> -    if (!gdbserver_start(device, &error_warn)) {
> -        monitor_printf(mon, "Could not open gdbserver on device '%s'\n",
> -                       device);
> +    if (!gdbserver_start(device, &err)) {
> +        error_report_err(err);
>      } else if (strcmp(device, "none") == 0) {
>          monitor_printf(mon, "Disabled gdbserver\n");
>      } else {
> -- 
> 2.49.0
> 
> 

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 :|

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Fri, Aug 08, 2025 at 10:08:12AM +0200, Markus Armbruster wrote:
>> HMP command gdbserver used to emit two error messages for certain
>> errors.  For instance, with -M none:
>> 
>>     (qemu) gdbserver
>>     gdbstub: meaningless to attach gdb to a machine without any CPU.
>>     Could not open gdbserver on device 'tcp::1234'
>> 
>> The first message is the specific error, and the second one a generic
>> additional message that feels superfluous to me.
>> 
>> Commit c0e6b8b798b (system: propagate Error to gdbserver_start (and
>> other device setups)) turned the first message into a warning:
>> 
>>     warning: gdbstub: meaningless to attach gdb to a machine without any CPU.
>>     Could not open gdbserver on device 'tcp::1234'
>> 
>> This is arguably worse.
>> 
>> hmp_gdbserver() passes &error_warn to gdbserver_start(), so that
>> failure gets reported as warning, and then additionally emits the
>> generic error on failure.  This is a misuse of &error_warn.
>> 
>> Instead, receive the error in &err and report it, as usual.  With
>> this, gdbserver reports just the error:
>> 
>>     gdbstub: meaningless to attach gdb to a machine without any CPU.
>
> What do you think about an alternative of removing "gdbstub: "
> from all the errors raised in  'gdbserver_start' & similar, and
> then using
>
>   error_prepend(err, "Could not open gdbserver on device '%s'", device);
>
> in hmp_gdbserver ?

This would change the error message from

    (qemu) gdbserver 
    gdbstub: meaningless to attach gdb to a machine without any CPU.

to

    (qemu) gdbserver 
    Could not open gdbserver on device 'tcp::1234': meaningless to attach gdb to a machine without any CPU.

Longer, but doesn't tell me anything new beyond reminding me that
gdbserver's argument defaults to "tcp::1234".

This instance of gdbserver_start() is additionally called by
qemu_machine_creation_done() in system/vl.c on behalf of command line
option gdb.  The error message would change from

    $ qemu-system-x86_64 -M none -gdb ""
    qemu-system-x86_64: -gdb : gdbstub: meaningless to attach gdb to a machine without any CPU.

to

    $ qemu-system-x86_64 -M none -gdb ""
    qemu-system-x86_64: -gdb : meaningless to attach gdb to a machine without any CPU.

Improvement.  Howver, we better drop the prefix from all error messages
emitted here, not just this one.

There's another instance of gdbserver_start() in gdbstub/user.c, called
by main() in bsduser/main.c and in linux-user/main.c on behalf of
command line option -g / -gdb.  Same silly prefix:

    $ qemu-x86_64 -g x,y /bin/ls
    qemu-x86_64: gdbstub: unknown option "y"
    Usage: -g {port|path}[,suspend={y|n}]

> I don't feel too strongly, so in any case for the patch as is,

Separate patch to strip "gdbstub: " from all error messages?

> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Thanks!