qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, qemu-devel@nongnu.org
Subject: [PULL 04/18] block/monitor: Use hmp_handle_error to report error
Date: Wed, 29 Oct 2025 13:06:20 +0100	[thread overview]
Message-ID: <20251029120634.288467-5-kwolf@redhat.com> (raw)
In-Reply-To: <20251029120634.288467-1-kwolf@redhat.com>

From: Bin Guo <guobin@linux.alibaba.com>

According to writing-monitor-commands.rst, best practice is to
use the 'hmp_handle_error' function, which ensures that the
message gets an 'Error: ' prefix.

Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
Message-ID: <20250916054850.40963-1-guobin@linux.alibaba.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
[kwolf: Fixed up iotests reference output]
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/monitor/block-hmp-cmds.c | 45 +++++++++++++++++-----------------
 tests/qemu-iotests/267.out     |  8 +++---
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 282d1c386e..3640d1f3dc 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -62,7 +62,7 @@ static void hmp_drive_add_node(Monitor *mon, const char *optstr)
 {
     QemuOpts *opts;
     QDict *qdict;
-    Error *local_err = NULL;
+    Error *err = NULL;
 
     opts = qemu_opts_parse_noisily(&qemu_drive_opts, optstr, false);
     if (!opts) {
@@ -73,19 +73,19 @@ static void hmp_drive_add_node(Monitor *mon, const char *optstr)
 
     if (!qdict_get_try_str(qdict, "node-name")) {
         qobject_unref(qdict);
-        error_report("'node-name' needs to be specified");
+        error_setg(&err, "'node-name' needs to be specified");
         goto out;
     }
 
-    BlockDriverState *bs = bds_tree_init(qdict, &local_err);
+    BlockDriverState *bs = bds_tree_init(qdict, &err);
     if (!bs) {
-        error_report_err(local_err);
         goto out;
     }
 
     bdrv_set_monitor_owned(bs);
 out:
     qemu_opts_del(opts);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_drive_add(Monitor *mon, const QDict *qdict)
@@ -109,7 +109,6 @@ void hmp_drive_add(Monitor *mon, const QDict *qdict)
     mc = MACHINE_GET_CLASS(current_machine);
     dinfo = drive_new(opts, mc->block_default_type, &err);
     if (err) {
-        error_report_err(err);
         qemu_opts_del(opts);
         goto err;
     }
@@ -123,7 +122,7 @@ void hmp_drive_add(Monitor *mon, const QDict *qdict)
         monitor_printf(mon, "OK\n");
         break;
     default:
-        monitor_printf(mon, "Can't hot-add drive to type %d\n", dinfo->type);
+        error_setg(&err, "Can't hot-add drive to type %d", dinfo->type);
         goto err;
     }
     return;
@@ -134,6 +133,7 @@ err:
         monitor_remove_blk(blk);
         blk_unref(blk);
     }
+    hmp_handle_error(mon, err);
 }
 
 void hmp_drive_del(Monitor *mon, const QDict *qdict)
@@ -141,36 +141,32 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
     const char *id = qdict_get_str(qdict, "id");
     BlockBackend *blk;
     BlockDriverState *bs;
-    Error *local_err = NULL;
+    Error *err = NULL;
 
     GLOBAL_STATE_CODE();
     bdrv_graph_rdlock_main_loop();
 
     bs = bdrv_find_node(id);
     if (bs) {
-        qmp_blockdev_del(id, &local_err);
-        if (local_err) {
-            error_report_err(local_err);
-        }
+        qmp_blockdev_del(id, &err);
         goto unlock;
     }
 
     blk = blk_by_name(id);
     if (!blk) {
-        error_report("Device '%s' not found", id);
+        error_setg(&err, "Device '%s' not found", id);
         goto unlock;
     }
 
     if (!blk_legacy_dinfo(blk)) {
-        error_report("Deleting device added with blockdev-add"
-                     " is not supported");
+        error_setg(&err, "Deleting device added with blockdev-add"
+                   " is not supported");
         goto unlock;
     }
 
     bs = blk_bs(blk);
     if (bs) {
-        if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, &local_err)) {
-            error_report_err(local_err);
+        if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, &err)) {
             goto unlock;
         }
 
@@ -196,6 +192,7 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
 
 unlock:
     bdrv_graph_rdunlock_main_loop();
+    hmp_handle_error(mon, err);
 }
 
 void hmp_commit(Monitor *mon, const QDict *qdict)
@@ -203,6 +200,7 @@ void hmp_commit(Monitor *mon, const QDict *qdict)
     const char *device = qdict_get_str(qdict, "device");
     BlockBackend *blk;
     int ret;
+    Error *err = NULL;
 
     GLOBAL_STATE_CODE();
     GRAPH_RDLOCK_GUARD_MAINLOOP();
@@ -214,22 +212,25 @@ void hmp_commit(Monitor *mon, const QDict *qdict)
 
         blk = blk_by_name(device);
         if (!blk) {
-            error_report("Device '%s' not found", device);
-            return;
+            error_setg(&err, "Device '%s' not found", device);
+            goto end;
         }
 
         bs = bdrv_skip_implicit_filters(blk_bs(blk));
 
         if (!blk_is_available(blk)) {
-            error_report("Device '%s' has no medium", device);
-            return;
+            error_setg(&err, "Device '%s' has no medium", device);
+            goto end;
         }
 
         ret = bdrv_commit(bs);
     }
     if (ret < 0) {
-        error_report("'commit' error for '%s': %s", device, strerror(-ret));
+        error_setg(&err, "'commit' error for '%s': %s", device, strerror(-ret));
     }
+
+end:
+    hmp_handle_error(mon, err);
 }
 
 void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
@@ -890,7 +891,7 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
 
     bs = bdrv_all_find_vmstate_bs(NULL, false, NULL, &err);
     if (!bs) {
-        error_report_err(err);
+        hmp_handle_error(mon, err);
         return;
     }
 
diff --git a/tests/qemu-iotests/267.out b/tests/qemu-iotests/267.out
index f6f5d8715a..37b7ebd280 100644
--- a/tests/qemu-iotests/267.out
+++ b/tests/qemu-iotests/267.out
@@ -8,7 +8,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm snap0
 Error: no block device can store vmstate for snapshot
 (qemu) info snapshots
-no block device can store vmstate for snapshot
+Error: no block device can store vmstate for snapshot
 (qemu) loadvm snap0
 Error: no block device can store vmstate for snapshot
 (qemu) quit
@@ -22,7 +22,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm snap0
 Error: Device 'none0' is writable but does not support snapshots
 (qemu) info snapshots
-no block device can store vmstate for snapshot
+Error: no block device can store vmstate for snapshot
 (qemu) loadvm snap0
 Error: Device 'none0' is writable but does not support snapshots
 (qemu) quit
@@ -58,7 +58,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm snap0
 Error: Device 'virtio0' is writable but does not support snapshots
 (qemu) info snapshots
-no block device can store vmstate for snapshot
+Error: no block device can store vmstate for snapshot
 (qemu) loadvm snap0
 Error: Device 'virtio0' is writable but does not support snapshots
 (qemu) quit
@@ -83,7 +83,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm snap0
 Error: Device 'file' is writable but does not support snapshots
 (qemu) info snapshots
-no block device can store vmstate for snapshot
+Error: no block device can store vmstate for snapshot
 (qemu) loadvm snap0
 Error: Device 'file' is writable but does not support snapshots
 (qemu) quit
-- 
2.51.0



  parent reply	other threads:[~2025-10-29 12:08 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-29 12:06 [PULL 00/18] Block layer patches Kevin Wolf
2025-10-29 12:06 ` [PULL 01/18] tests/qemu-iotests: Mark the 'inactive-node-nbd' as unsupported with -luks Kevin Wolf
2025-10-29 12:06 ` [PULL 02/18] block: remove 'detached-header' option from opts after use Kevin Wolf
2025-10-29 12:06 ` [PULL 03/18] block: fix luks 'amend' when run in coroutine Kevin Wolf
2025-10-31 10:18   ` Michael Tokarev
2025-10-31 11:05     ` Daniel P. Berrangé
2025-10-29 12:06 ` Kevin Wolf [this message]
2025-10-29 12:06 ` [PULL 05/18] block/curl.c: Fix CURLOPT_VERBOSE parameter type Kevin Wolf
2025-10-29 12:06 ` [PULL 06/18] iotests: Adjust nbd expected outputs to match current behavior Kevin Wolf
2025-10-29 12:06 ` [PULL 07/18] iotests: Adjust fuse-allow-other expected output Kevin Wolf
2025-10-29 12:06 ` [PULL 08/18] block: enable stats-intervals for storage devices Kevin Wolf
2025-10-29 12:06 ` [PULL 09/18] MAINTAINERS: Mark VHDX block driver as "Odd Fixes" Kevin Wolf
2025-10-29 12:06 ` [PULL 10/18] include/block/block_int-common: document when resize callback is used Kevin Wolf
2025-10-29 12:06 ` [PULL 11/18] block: make bdrv_co_parent_cb_resize() a proper IO API function Kevin Wolf
2025-10-29 12:06 ` [PULL 12/18] block: implement 'resize' callback for child_of_bds class Kevin Wolf
2025-10-29 12:06 ` [PULL 13/18] iotests: add test for resizing a node below filters Kevin Wolf
2025-10-29 12:06 ` [PULL 14/18] iotests: add test for resizing a 'file' node below a 'raw' node Kevin Wolf
2025-10-29 12:06 ` [PULL 15/18] block: Improve comments in BlockLimits Kevin Wolf
2025-10-29 12:06 ` [PULL 16/18] block: Expose block limits for images in QMP Kevin Wolf
2025-10-29 12:06 ` [PULL 17/18] qemu-img info: Optionally show block limits Kevin Wolf
2025-10-29 12:06 ` [PULL 18/18] qemu-img info: Add cache mode option Kevin Wolf
2025-10-31  9:25 ` [PULL 00/18] Block layer patches Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251029120634.288467-5-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).