error reporting from pre_load etc callbacks

error reporting from pre_load etc callbacks

[Peter Maydell]

(This was pointed out to me off-list.)

What is the intended API of the pre_load() etc callbacks?
docs/devel/migration.rst doesn't say, and there's no doc comment
in include/migration/vmstate.h. Most actual device implementations
of the hooks seem to return 0 on success and -1 on failure.
But the callers of the hooks like vmstate_load_state() seem to
assume they return negative-errnos: a non-zero return value
is just propagated up to the caller, and can end up for
instance treated as negative-errno in process_incoming_migration_co():
        error_report("load of migration failed: %s", strerror(-ret));
which results in nonsensical error messages to the user like:
 qemu-system-aarch64: load of migration failed: Operation not permitted


More generally, it would be nice if a pre_load/post_load/etc
hook could report an error more precisely than just yes/no
or even than just an errno: in the case of arm CPU loading
we could in theory report more detail like which register the
kernel didn't handle...

thanks
-- PMM

Re: error reporting from pre_load etc callbacks

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> (This was pointed out to me off-list.)
> 
> What is the intended API of the pre_load() etc callbacks?
> docs/devel/migration.rst doesn't say, and there's no doc comment
> in include/migration/vmstate.h. Most actual device implementations
> of the hooks seem to return 0 on success and -1 on failure.
> But the callers of the hooks like vmstate_load_state() seem to
> assume they return negative-errnos: a non-zero return value
> is just propagated up to the caller, and can end up for
> instance treated as negative-errno in process_incoming_migration_co():
>         error_report("load of migration failed: %s", strerror(-ret));
> which results in nonsensical error messages to the user like:
>  qemu-system-aarch64: load of migration failed: Operation not permitted

In theory (pre|post)_(load|save) have the same interface; in practice
they're all rather inconsistent.
Returning 0 on success, -errno on failure is my preference.

> More generally, it would be nice if a pre_load/post_load/etc
> hook could report an error more precisely than just yes/no
> or even than just an errno: in the case of arm CPU loading
> we could in theory report more detail like which register the
> kernel didn't handle...

On the load side, just error_report as tightly as you can; what
you should see on stderr is basically a backtrace:
   'failed to do something in ...'
   'load of migration failed of ...'

Dave

> thanks
> -- PMM
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK