Re: [PULL 02/45] migration: push Error **errp into vmstate_load_state()

[Peter Maydell <peter.maydell@linaro.org>]

On Tue, 21 Oct 2025 at 17:46, Peter Xu <peterx@redhat.com> wrote:
>
> On Tue, Oct 21, 2025 at 05:21:44PM +0100, Peter Maydell wrote:
> > On Tue, 21 Oct 2025 at 17:16, Peter Xu <peterx@redhat.com> wrote:
> > >
> > > On Tue, Oct 21, 2025 at 04:43:52PM +0100, Peter Maydell wrote:
> > > > Do you have plans for further cleanup/extension of the
> > > > use of Error here that would let these functions pass
> > > > the Error back up the chain ?
> > >
> > > It would be non-trivial though as we'll need to change VMStateInfo.get()
> > > API and that'll be another lot of churns.
> >
> > We could at least do it in stages, so we add new fields
> > .get_err and .put_err that have the new API with Error*;
> > the calling code in migration/ uses the new functions if
> > they're non-NULL, otherwise falling back to the old ones.
> > Then we only need to update the implementations which
> > want to be able to return an Error. (This is the same sort
> > of thing we have with MemoryRegionOps and its read/write
> > vs read_with_attrs/write_with_attrs methods.)
> >
> > The downside is we end up with another "there's two ways
> > you can do this" API.
>
> Right, the other thing is get()/put() logically should only be used for
> primitives...  I wished they're never used in complicated ways.
>
> IOW, in an ideal world, get()/put() should only fail because of qemufile
> channel errors, rather than anything else.

I suppose so, but that seems like something in practice
we make a lot of use of. It's really handy to be able to
say "this is how you obtain the integer you wanted to put
in the migration stream" -- we do a fair amount of that
in target/arm/machine.c for instance. But those don't
need to return a failure, I suppose.

> Then, descriptive errors for get()/put() may not even be needed..  OTOH,
> complicated structs (like virtio_gpu_load...) should really be done in
> VMSDs already, and anything can fail outside -EIO should be done only in
> pre_save() / post_load() / ...

You can't return an Error from a post_load callback either :-)

-- PMM