On Mon, May 27, 2024 at 12:53:22PM +0200, Markus Armbruster wrote: > Peter Xu <pet...@redhat.com> writes: > > > On Mon, May 13, 2024 at 04:17:02PM +0200, Markus Armbruster wrote: > >> 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. > > > > :-( > > If I understood how it's *supposed* to work, I might have a chance... > > I can see a mixture of reporting errors directly (with error_report() & > friends), passing them to callers (via Error **errp), and storing them > in / retrieving them from MigrationState member @error. This can't be > right. > > I think a necessary first step towards getting it right is a shared > understanding how errors are to be handled in migration code. This > includes how error data should flow from error source to error sink, and > what the possible sinks are.
True. I think the sink should always be MigrationState.error so far. There's also the other complexity on detecting errors using either qemu_file_get_error() or migrate_get_error().. the error handling in migration is indeed prone to a cleanup. I've added a cleanup entry for migration todo page: https://wiki.qemu.org/ToDo/LiveMigration#Migration_error_detection_and_reporting Thanks, -- Peter Xu
