On Sat, Feb 21, 2026 at 12:02:04AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Simplify vmstate_save_state() which is rather big, and simplify further
> refactoring.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>

Reviewed-by: Peter Xu <[email protected]>

One trivial thing..

> ---
>  migration/vmstate.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index b07bbdd366..810b131f18 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -430,6 +430,26 @@ bool vmstate_section_needed(const VMStateDescription 
> *vmsd, void *opaque)
>      return true;
>  }
>  
> +static bool vmstate_pre_save(const VMStateDescription *vmsd, void *opaque,
> +                             Error **errp)
> +{
> +    ERRP_GUARD();

I can come up with no reason a caller should pass in NULL for errp..  I
don't know why we assert(errp) so rarely in qemu for similar cases, but
iiuc that's what we really want here..

> +
> +    if (vmsd->pre_save_errp) {
> +        if (!vmsd->pre_save_errp(opaque, errp)) {
> +            error_prepend(errp, "pre-save for %s failed: ", vmsd->name);
> +            return false;
> +        }
> +    } else if (vmsd->pre_save) {
> +        if (vmsd->pre_save(opaque) < 0) {
> +            error_setg(errp, "pre-save failed: %s", vmsd->name);
> +            return false;
> +        }
> +    }
> +
> +    return true;
> +}
> +
>  static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
>                                  void *opaque, JSONWriter *vmdesc,
>                                  int version_id, Error **errp)
> @@ -440,18 +460,8 @@ static int vmstate_save_state_v(QEMUFile *f, const 
> VMStateDescription *vmsd,
>  
>      trace_vmstate_save_state_top(vmsd->name);
>  
> -    if (vmsd->pre_save_errp) {
> -        ret = vmsd->pre_save_errp(opaque, errp) ? 0 : -EINVAL;
> -        if (ret < 0) {
> -            error_prepend(errp, "pre-save for %s failed: ", vmsd->name);
> -            return ret;
> -        }
> -    } else if (vmsd->pre_save) {
> -        ret = vmsd->pre_save(opaque);
> -        if (ret) {
> -            error_setg(errp, "pre-save failed: %s", vmsd->name);
> -            return ret;
> -        }
> +    if (!vmstate_pre_save(vmsd, opaque, errp)) {
> +        return -EINVAL;
>      }
>  
>      trace_vmstate_save_state_pre_save_done(vmsd->name);
> -- 
> 2.52.0
> 

-- 
Peter Xu


Reply via email to