Re: [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx

[Peter Xu <peterx@redhat.com>]

On Wed, Mar 25, 2026 at 05:10:51PM -0300, Fabiano Rosas wrote:
> Sure, if there's anything you still find valuable but won't take for the
> repost let me know and I can send a separate series on top if that's the
> case.

I found that there're quite a few cleanups at latter patches, and it caused
some rebase pain if the base series still need frequent update.

So even if they look good in general to me, I want to leave them for later,
reducing scope of dependency.  After the API conslidated, we can do
cleanups on top.

Let me mention what I did and plan to post later:

> >>   vmstate: fixup the use of AUTO_ALLOC flag
> >>   vmstate: Remove vmstate_use_marker_field

I squashed these two.

> >>   vmstate: Stop checking size for nullptr compression

Picked.

> >>   vmstate: Set error inside of vmstate_save_field_with_vmdesc

Squashed.

> >>   vmstate: Remove vmdesc_loop

Starting from this one, I feel a bit uncertain, and maybe we can leave it
for later.

> >>   vmstate: Put array of pointers code together

This is a major cleanup, I left it and a few ones later aside for now.

> >>   vmstate: Create and save ptr marker in same function
> >>   vmstate: Don't recompute size and n_elems in vmstate_size
> >>   vmstate: Increase scope of vmstate_handle_alloc
> >>   vmstate: Remove curr_elem_p

I agree this is good, and..

> >>   vmstate: Introduce vmstate_first
> >>   vmstate: Introduce vmstate_next

.. despite that I feel like the current vmstate_next() impl might be
problematic (see my other reply), I also like this idea of having a _next
helper to hide some details on the ptr markers.  I'll adopt that idea,
thanks.

> >>   vmstate: Drop VMS_ARRAY_OF_POINTER_AUTO_ALLOC

Until I get corrected I think we'll need this flag, as said.  I feel like
it's better to keep VMS_ALLOC semantics as before (when set, it must alloc
the top field object ignoring stream data), then this one for the array
elements separate (meanwhile, this one is "optionally done", based on the
stream data).

We have plenty of space for VMS_* flags and it's not ABI.  IMHO when we're
not certain to merge flags we can always keep them separate until later too.

> >>   vmstate: Move VMS_MUST_EXIST check
> >>   vmstate: Invert exists check
> >>   vmstate: Declare variables at the top
> >>   vmstate: Reduce indentation levels

All these seem to be cleanups.  I plan to leave them separately.

I think it might be good I send another version directly, I'll do it very
soon and keep it RFC.  If I missed something while reading this series,
please then let me know.

-- 
Peter Xu