qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: richard.henderson@linaro.org,
	"Jonathan Cameron" <Jonathan.Cameron@huawei.com>,
	"Jonathan Cameron" <jonathan.cameron@huawei.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Akihiko Odaki" <odaki@rsg.ci.i.u-tokyo.ac.jp>
Subject: [PULL 03/13] hw/cxl: Convert cxl_fmws_link() to Error
Date: Tue, 30 Sep 2025 14:46:43 +0200	[thread overview]
Message-ID: <20250930124653.321609-4-armbru@redhat.com> (raw)
In-Reply-To: <20250930124653.321609-1-armbru@redhat.com>

Functions that use an Error **errp parameter to return errors should
not also report them to the user, because reporting is the caller's
job.  When the caller does, the error is reported twice.  When it
doesn't (because it recovered from the error), there is no error to
report, i.e. the report is bogus.

cxl_fmws_link_targets() violates this principle: it calls
error_setg(&error_fatal, ...) via cxl_fmws_link().  Goes back to
commit 584f722eb3ab (hw/cxl: Make the CXL fixed memory windows
devices.)  Currently harmless, because cxl_fmws_link_targets()'s
callers always pass &error_fatal.  Clean this up by converting
cxl_fmws_link() to Error.

Also change its return value on error from 1 to -1 to conform to the
rules laid in qapi/error.h.  It's call chain cxl_fmws_link_targets()
via object_child_foreach_recursive() is fine with that.

Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20250923091000.3180122-4-armbru@redhat.com>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/cxl/cxl-host.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index 5c2ce25a19..0d891c651d 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -72,6 +72,7 @@ static void cxl_fixed_memory_window_config(CXLFixedMemoryWindowOptions *object,
 
 static int cxl_fmws_link(Object *obj, void *opaque)
 {
+    Error **errp = opaque;
     struct CXLFixedWindow *fw;
     int i;
 
@@ -87,9 +88,9 @@ static int cxl_fmws_link(Object *obj, void *opaque)
         o = object_resolve_path_type(fw->targets[i], TYPE_PXB_CXL_DEV,
                                      &ambig);
         if (!o) {
-            error_setg(&error_fatal, "Could not resolve CXLFM target %s",
+            error_setg(errp, "Could not resolve CXLFM target %s",
                        fw->targets[i]);
-            return 1;
+            return -1;
         }
         fw->target_hbs[i] = PXB_CXL_DEV(o);
     }
@@ -99,7 +100,7 @@ static int cxl_fmws_link(Object *obj, void *opaque)
 void cxl_fmws_link_targets(Error **errp)
 {
     /* Order doesn't matter for this, so no need to build list */
-    object_child_foreach_recursive(object_get_root(), cxl_fmws_link, NULL);
+    object_child_foreach_recursive(object_get_root(), cxl_fmws_link, errp);
 }
 
 static bool cxl_hdm_find_target(uint32_t *cache_mem, hwaddr addr,
-- 
2.49.0



  parent reply	other threads:[~2025-09-30 12:50 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-30 12:46 [PULL 00/13] Error reporting patches for 2025-09-30 Markus Armbruster
2025-09-30 12:46 ` [PULL 01/13] monitor: Clean up HMP gdbserver error reporting Markus Armbruster
2025-09-30 12:46 ` [PULL 02/13] tcg: Fix error reporting on mprotect() failure in tcg_region_init() Markus Armbruster
2025-09-30 12:46 ` Markus Armbruster [this message]
2025-09-30 12:46 ` [PULL 04/13] migration/cpr: Clean up error reporting in cpr_resave_fd() Markus Armbruster
2025-09-30 12:46 ` [PULL 05/13] hw/remote/vfio-user: Clean up error reporting Markus Armbruster
2025-09-30 12:46 ` [PULL 06/13] net/slirp: " Markus Armbruster
2025-09-30 12:46 ` [PULL 07/13] ui/spice-core: " Markus Armbruster
2025-09-30 12:46 ` [PULL 08/13] util/oslib-win32: Do not treat null @errp as &error_warn Markus Armbruster
2025-09-30 12:46 ` [PULL 09/13] ui/pixman: Consistent error handling in qemu_pixman_shareable_free() Markus Armbruster
2025-10-13  9:55   ` Bernhard Beschow
2025-10-13 11:11     ` Markus Armbruster
2025-09-30 12:46 ` [PULL 10/13] ui/dbus: Clean up dbus_update_gl_cb() error checking Markus Armbruster
2025-09-30 12:46 ` [PULL 11/13] ui/dbus: Consistent handling of texture mutex failure Markus Armbruster
2025-09-30 16:27   ` Richard Henderson
2025-10-01  6:30     ` Markus Armbruster
2025-09-30 12:46 ` [PULL 12/13] ivshmem-flat: Mark an instance of missing error handling FIXME Markus Armbruster
2025-09-30 12:46 ` [PULL 13/13] error: Kill @error_warn Markus Armbruster

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=20250930124653.321609-4-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=odaki@rsg.ci.i.u-tokyo.ac.jp \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.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).