On Wed, Jan 28, 2026 at 04:35:32PM -0300, Fabiano Rosas wrote:
> Peter Xu <[email protected]> writes:
>
> > Split it into two smaller chunks:
> >
> > - Dump of early_setup VMSDs
> > - Dump of save_setup() sections
> >
> > They're mutual exclusive, hence we can run two loops and do them
> > sequentially. This will cause migration thread to loop one more time, but
> > it should be fine when migration just started and only do it once. It's
> > needed because we will need to reuse the early_vmsd helper later to
> > deduplicate code elsewhere.
> >
> > QEMU almost sticks with qemu_savevm_state_XXX() to represent the dump of
> > vmstates's section XXX.
> > With that in mind, this patch renamed the original
> > qemu_savevm_state_setup() to qemu_savevm_state_do_setup() instead.
> >
> > So after this patch:
> >
> > - qemu_savevm_state_non_iterable_early() dumps early_vmsds only,
> > - qemu_savevm_state_setup() dumps save_setup() sections only,
> > - qemu_savevm_state_do_setup() does all things needed during setup
> > phase (including migration SETUP notifies)
> >
> > Signed-off-by: Peter Xu <[email protected]>
>
> Reviewed-by: Fabiano Rosas <[email protected]>
>
> Not for this series, but I think we could try to standardize the naming
> in savevm.c a bit. Here's a list of functions from savevm.c plus the
> main function called by them. You'll see that there's room from
> improvement.
>
> Removing this "qemu" prefix and choosing when to put "state" in the name
> would already be a win.
True.
IMHO if we choose it again we can use prefix vmstate_. The current
savevm_state_ is pretty vague on its own when loadvm needs to use it. Then
it can be vmstate_save_* on save side, and vmstate_load_* on load side (and
vmstate_XXX) when valid on both.
>
> Also maybe making more clear what calls vmstate code and what deals with
> se->ops only.
Yes we can try, but there'll be some tricky ones, inline below (assuming we
can shorten the prefix to "vmstate_").
>
> ---
> qemu_loadvm_state_setup { se->ops->load_setup }
Taking this one as example.
We could rename this to vmstate_ops_load_setup(), however "load_setup" is
actually a concept that is higher than the impl. IOW it'll be also
reasonable if we want to add a load_setup() hook to VMSDs some day. Maybe
still good to rename it to vmstate_load_setup().
> qemu_savevm_state_active { se->ops->is_active }
Similarly, here I'd think vmstate_active() (it applies to both save/load)
better than vmstate_ops_active() because active describes the vmstate
instance, and it's fine one day we want to introduce VMSD.active.
...
> qemu_savevm_state_blocked { se->vmsd->unmigratable }
> qemu_savevm_state_cleanup { se->ops->save_cleanup }
> qemu_savevm_state_guest_unplug_pending { se->vmsd->dev_unplug_pending }
> qemu_savevm_state_iterate { se->ops->save_live_iterate }
> qemu_savevm_state_non_iterable_early { se->vmsd->early_setup;
> vmstate_save }
> qemu_savevm_state_pending_estimate { se->ops->state_pending_estimate }
> qemu_savevm_state_pending_exact { se->ops->state_pending_exact }
> qemu_savevm_state_postcopy_prepare { se->ops->save_postcopy_prepare }
> qemu_savevm_state_prepare { se->ops->save_prepare }
> qemu_savevm_state_resume_prepare { se->ops->resume_prepare }
> loadvm_postcopy_handle_switchover_start { se->ops->switchover_start }
> qemu_loadvm_load_state_buffer { se->ops->load_state_buffer }
> qemu_loadvm_state_switchover_ack_needed { se->ops->switchover_ack_needed }
> qemu_savevm_complete { se->ops->save_complete }
> qemu_savevm_complete_exists { se->ops->save_complete }
> qemu_savevm_non_migratable_list { se->vmsd->unmigratable }
> qemu_savevm_se_iterable { se->ops->save_setup }
> save_state_priority { se->vmsd->priority }
> vmstate_load { se->ops->load_state }
This one (and vmstate_save()) is special because it handles both impls
(VMSD and OPS). The current names are actually suitable.
> vmstate_save_old_style { se->ops->save_state }
>
> loadvm_handle_cmd_packaged { qemu_loadvm_state_main }
> qemu_load_device_state { qemu_loadvm_state_main }
> qemu_savevm_maybe_send_switchover_start {
> qemu_savevm_send_switchover_start }
> qemu_savevm_send_* { qemu_savevm_command_send }
> qemu_savevm_state_complete_postcopy { qemu_savevm_complete }
> qemu_savevm_state_complete_precopy_iterable { qemu_savevm_complete }
> qemu_savevm_state_end_precopy { qemu_savevm_state_end }
> qemu_savevm_state_header { qemu_savevm_send_header }
...
>
> qemu_savevm_state_do_setup {
> qemu_savevm_state_non_iterable_early;
> qemu_savevm_state_setup }
>
> qemu_loadvm_state { qemu_loadvm_state_header;
> qemu_loadvm_state_setup;
>
> qemu_loadvm_state_switchover_ack_needed;
> qemu_loadvm_state_main;
> qemu_loadvm_thread_pool_wait }
>
> qemu_loadvm_state_main { qemu_loadvm_section_start_full;
> qemu_loadvm_section_part_end;
> loadvm_process_command }
>
> qemu_savevm_state { qemu_savevm_state_header;
> qemu_savevm_state_do_setup;
> qemu_savevm_state_iterate;
> qemu_savevm_state_complete_precopy;
> qemu_savevm_state_cleanup }
>
> qemu_savevm_state_complete_precopy {
> qemu_savevm_state_complete_precopy_iterable;
> qemu_savevm_state_non_iterable;
> qemu_savevm_state_end_precopy }
>
> qemu_save_device_state {
> qemu_savevm_state_non_iterable_early;
> qemu_savevm_state_non_iterable;
> qemu_savevm_state_end }
Most of above are special functions higher than vmstate alone, hence IMHO
not vmstate_*() APIs. We can name it whatever way we like, or leave them
be.
>
> loadvm_handle_recv_bitmap { migrate_send_rp_recv_bitmap }
> loadvm_postcopy_handle_advise { ram_postcopy_incoming_init }
> loadvm_postcopy_handle_listen { postcopy_incoming_setup }
> loadvm_postcopy_handle_resume { mis->postcopy_pause_sem_fault }
> loadvm_postcopy_handle_run { loadvm_postcopy_handle_run_bh }
> loadvm_postcopy_handle_run_bh { vm_start }
> loadvm_postcopy_ram_handle_discard { postcopy_ram_prepare_discard }
> loadvm_process_command { switch (cmd) }
> loadvm_process_enable_colo { colo_init_ram_cache }
These are almost not vmstate relevant but about MIG_CMD*. We can likely
leave them alone. Currently the loadvm_ prefix actually sounds not too
bad.
...
[will skip most of below; we could start with vmstate API first]
> qemu_loadvm_approve_switchover { migrate_send_rp_switchover_ack }
> qemu_loadvm_load_thread { data->function }
> qemu_loadvm_section_part_end { vmstate_load }
> qemu_loadvm_section_start_full { vmstate_load }
> qemu_loadvm_start_load_thread { thread_pool_submit }
> qemu_loadvm_state_cleanup { se->ops->load_cleanup }
> qemu_loadvm_state_header { QEMU_VM_FILE_MAGIC }
> qemu_loadvm_thread_pool_create { thread_pool_new }
> qemu_loadvm_thread_pool_destroy { thread_pool_free }
> qemu_loadvm_thread_pool_wait { thread_pool_wait }
> qemu_savevm_command_send { QEMU_VM_COMMAND }
> qemu_savevm_send_colo_enable { MIG_CMD_ENABLE_COLO }
> qemu_savevm_send_configuration {
> vmstate_save_state(&vmstate_configuration) }
> qemu_savevm_send_header { QEMU_VM_FILE_MAGIC }
> qemu_savevm_state_end { QEMU_VM_EOF }
> qemu_savevm_state_non_iterable { vmstate_save }
> qemu_savevm_state_setup { se->ops->save_setup }
> qemu_savevm_state_vm_desc { QEMU_VM_VMDESCRIPTION }
> register_savevm_live { savevm_state_handler_insert }
> save_section_footer { QEMU_VM_SECTION_FOOTER }
> save_section_header { section_type }
> savevm_state_handler_insert { QTAILQ_INSERT_TAIL }
> savevm_state_handler_remove { QTAILQ_REMOVE }
> unregister_savevm { savevm_state_handler_remove }
> vmstate_register_with_alias_id { savevm_state_handler_insert }
> vmstate_save { vmstate_save_old_style;
> vmstate_save_state }
> vmstate_unregister { savevm_state_handler_remove }
>
--
Peter Xu
