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,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Fabiano Rosas" <farosas@suse.de>, "Peter Xu" <peterx@redhat.com>
Subject: [PULL 5/9] migration: Rephrase message on failure to save / load Xen device state
Date: Mon, 27 May 2024 13:31:27 +0200	[thread overview]
Message-ID: <20240527113131.2054486-6-armbru@redhat.com> (raw)
In-Reply-To: <20240527113131.2054486-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.

qmp_xen_save_devices_state() and qmp_xen_load_devices_state() violate
this principle: they call qemu_save_device_state() and
qemu_loadvm_state(), which call error_report_err().

I wish I could clean this up now, but migration's error reporting is
too complicated (confused?) for me to mess with it.

Instead, I'm merely improving the error reported by
qmp_xen_load_devices_state() and qmp_xen_load_devices_state() to the
QMP core from

    An IO error has occurred

to
    saving Xen device state failed

and

    loading Xen device state failed

respectively.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20240513141703.549874-6-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Acked-by: Fabiano Rosas <farosas@suse.de>
Acked-by: Peter Xu <peterx@redhat.com>
---
 migration/savevm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 6c789bd54b..c621f2359b 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -45,7 +45,6 @@
 #include "qapi/qapi-commands-migration.h"
 #include "qapi/clone-visitor.h"
 #include "qapi/qapi-builtin-visit.h"
-#include "qapi/qmp/qerror.h"
 #include "qemu/error-report.h"
 #include "sysemu/cpus.h"
 #include "exec/memory.h"
@@ -3203,7 +3202,7 @@ void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,
     object_unref(OBJECT(ioc));
     ret = qemu_save_device_state(f);
     if (ret < 0 || qemu_fclose(f) < 0) {
-        error_setg(errp, QERR_IO_ERROR);
+        error_setg(errp, "saving Xen device state failed");
     } else {
         /* libxl calls the QMP command "stop" before calling
          * "xen-save-devices-state" and in case of migration failure, libxl
@@ -3252,7 +3251,7 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp)
     ret = qemu_loadvm_state(f);
     qemu_fclose(f);
     if (ret < 0) {
-        error_setg(errp, QERR_IO_ERROR);
+        error_setg(errp, "loading Xen device state failed");
     }
     migration_incoming_state_destroy();
 }
-- 
2.45.0



  parent reply	other threads:[~2024-05-27 11:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-27 11:31 [PULL 0/9] Error reporting patches for 2024-05-27 Markus Armbruster
2024-05-27 11:31 ` [PULL 1/9] block: Improve error message when external snapshot can't flush Markus Armbruster
2024-05-27 11:31 ` [PULL 2/9] dump/win_dump: Improve error messages on write error Markus Armbruster
2024-05-27 11:31 ` [PULL 3/9] block/vmdk: Improve error messages on extent " Markus Armbruster
2024-05-27 11:31 ` [PULL 4/9] cpus: Improve error messages on memsave, pmemsave " Markus Armbruster
2024-05-27 11:31 ` Markus Armbruster [this message]
2024-05-27 11:31 ` [PULL 6/9] qerror: QERR_IO_ERROR is no longer used, drop Markus Armbruster
2024-05-27 11:31 ` [PULL 7/9] qga-win32: Improve guest-set-user-password, guest-file-open errors Markus Armbruster
2024-05-27 11:31 ` [PULL 8/9] qga: Shorten several error messages Markus Armbruster
2024-05-27 11:31 ` [PULL 9/9] qerror: QERR_QGA_COMMAND_FAILED is no longer used, drop Markus Armbruster
2024-05-27 17:15 ` [PULL 0/9] Error reporting patches for 2024-05-27 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=20240527113131.2054486-6-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=farosas@suse.de \
    --cc=peterx@redhat.com \
    --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).