On 2025-05-29 16:42, Juraj Marcin wrote:
> 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.
Never mind, just noticed it's in the next patch...
>
> 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
> >
