Hi Peter
On 2025-05-27 17:58, Peter Xu wrote:
> The hook is only defined in two vmstate users ("ram" and "block dirty
> bitmap"), meanwhile both of them define the hook exactly the same as the
> precopy version. Hence, this postcopy version isn't needed.
>
> No functional change intended.
Could be some future users, that would benefit from separate hooks for
precopy and postcopy?
In case we are going to drop it, I think the '_precopy' suffix could be
dropped too, as the handler would be used for postcopy too.
Best regards
Juraj Marcin
>
> Signed-off-by: Peter Xu <[email protected]>
> ---
> include/migration/register.h | 24 ++++++++----------------
> migration/block-dirty-bitmap.c | 1 -
> migration/ram.c | 1 -
> migration/savevm.c | 9 ++++-----
> 4 files changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/include/migration/register.h b/include/migration/register.h
> index b79dc81b8d..e022195785 100644
> --- a/include/migration/register.h
> +++ b/include/migration/register.h
> @@ -77,26 +77,18 @@ typedef struct SaveVMHandlers {
> */
> void (*save_cleanup)(void *opaque);
>
> - /**
> - * @save_live_complete_postcopy
> - *
> - * Called at the end of postcopy for all postcopyable devices.
> - *
> - * @f: QEMUFile where to send the data
> - * @opaque: data pointer passed to register_savevm_live()
> - *
> - * Returns zero to indicate success and negative for error
> - */
> - int (*save_live_complete_postcopy)(QEMUFile *f, void *opaque);
> -
> /**
> * @save_live_complete_precopy
> *
> * Transmits the last section for the device containing any
> - * remaining data at the end of a precopy phase. When postcopy is
> - * enabled, devices that support postcopy will skip this step,
> - * where the final data will be flushed at the end of postcopy via
> - * @save_live_complete_postcopy instead.
> + * remaining data at the end phase of migration.
> + *
> + * For precopy, this will be invoked _during_ the switchover phase
> + * after source VM is stopped.
> + *
> + * For postcopy, this will be invoked _after_ the switchover phase
> + * (except some very unusual cases, like PMEM ramblocks), while
> + * destination VM can be running.
> *
> * @f: QEMUFile where to send the data
> * @opaque: data pointer passed to register_savevm_live()
> diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
> index f2c352d4a7..6ee3c32a76 100644
> --- a/migration/block-dirty-bitmap.c
> +++ b/migration/block-dirty-bitmap.c
> @@ -1248,7 +1248,6 @@ static bool dirty_bitmap_has_postcopy(void *opaque)
>
> static SaveVMHandlers savevm_dirty_bitmap_handlers = {
> .save_setup = dirty_bitmap_save_setup,
> - .save_live_complete_postcopy = dirty_bitmap_save_complete,
> .save_live_complete_precopy = dirty_bitmap_save_complete,
> .has_postcopy = dirty_bitmap_has_postcopy,
> .state_pending_exact = dirty_bitmap_state_pending,
> diff --git a/migration/ram.c b/migration/ram.c
> index fd8d83b63c..8b43b9e1e8 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -4545,7 +4545,6 @@ void postcopy_preempt_shutdown_file(MigrationState *s)
> static SaveVMHandlers savevm_ram_handlers = {
> .save_setup = ram_save_setup,
> .save_live_iterate = ram_save_iterate,
> - .save_live_complete_postcopy = ram_save_complete,
> .save_live_complete_precopy = ram_save_complete,
> .has_postcopy = ram_has_postcopy,
> .state_pending_exact = ram_state_pending_exact,
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 006514c3e3..26d32eb5a7 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1485,9 +1485,8 @@ bool should_send_vmdesc(void)
> }
>
> /*
> - * Calls the save_live_complete_postcopy methods
> - * causing the last few pages to be sent immediately and doing any associated
> - * cleanup.
> + * Complete saving any postcopy-able devices.
> + *
> * Note postcopy also calls qemu_savevm_state_complete_precopy to complete
> * all the other devices, but that happens at the point we switch to
> postcopy.
> */
> @@ -1497,7 +1496,7 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f)
> int ret;
>
> QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> - if (!se->ops || !se->ops->save_live_complete_postcopy) {
> + if (!se->ops || !se->ops->save_live_complete_precopy) {
> continue;
> }
> if (se->ops->is_active) {
> @@ -1510,7 +1509,7 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f)
> qemu_put_byte(f, QEMU_VM_SECTION_END);
> qemu_put_be32(f, se->section_id);
>
> - ret = se->ops->save_live_complete_postcopy(f, se->opaque);
> + ret = se->ops->save_live_complete_precopy(f, se->opaque);
> trace_savevm_section_end(se->idstr, se->section_id, ret);
> save_section_footer(f, se);
> if (ret < 0) {
> --
> 2.49.0
>
