Re: [PATCH v2] migration: Ensure vmstate_save() sets errp

[Peter Xu]

On Tue, Oct 15, 2024 at 07:04:37PM +0200, Hanna Czenczek wrote:
> migration/savevm.c contains some calls to vmstate_save() that are
> followed by migrate_set_error() if the integer return value indicates an
> error.  migrate_set_error() requires that the `Error *` object passed to
> it is set.  Therefore, vmstate_save() is assumed to always set *errp on
> error.
> 
> Right now, that assumption is not met: vmstate_save_state_v() (called
> internally by vmstate_save()) will not set *errp if
> vmstate_subsection_save() or vmsd->post_save() fail.  Fix that by adding
> an *errp parameter to vmstate_subsection_save(), and by generating a
> generic error in case post_save() fails (as is already done for
> pre_save()).
> 
> Without this patch, qemu will crash after vmstate_subsection_save() or
> post_save() have failed inside of a vmstate_save() call (unless
> migrate_set_error() then happen to discard the new error because
> s->error is already set).  This happens e.g. when receiving the state
> from a virtio-fs back-end (virtiofsd) fails.
> 
> Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
> ---
> v2: As suggested by Peter, after vmsd->post_save(), change the condition
>     from `if (!ret)` to `if (!ret && ps_ret)` so we will not create an
>     error object in case of success (that would then be leaked, most
>     likely).
> ---
>  migration/vmstate.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)

queued, thanks!

-- 
Peter Xu