Re: [PATCH 2/2] migration: Trivial cleanups for postcopy

[Peter Xu <peterx@redhat.com>]

On Wed, Apr 16, 2025 at 10:43:56AM -0300, Fabiano Rosas wrote:
> Some general cleanups of silly things that were left behind when
> refactoring code.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  migration/migration.c | 28 ++++++++++------------------
>  1 file changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index d46e776e24..89b1de0ab5 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2732,19 +2732,15 @@ static int postcopy_start(MigrationState *ms, Error **errp)
>      }
>  
>      /*
> -     * in Finish migrate and with the io-lock held everything should
> +     * in FINISH_MIGRATE and with the BQL held everything should
>       * be quiet, but we've potentially still got dirty pages and we
>       * need to tell the destination to throw any pages it's already received
>       * that are dirty
>       */
> -    if (migrate_postcopy_ram()) {
> -        ram_postcopy_send_discard_bitmap(ms);
> -    }
> +    ram_postcopy_send_discard_bitmap(ms);

IIUC it was from:

commit 58110f0acb1a33e2bc60a2f1b26d2690a92e8a14
Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Date:   Mon Jul 10 19:30:16 2017 +0300

    migration: split common postcopy out of ram postcopy

I need to confess I know nothing about migrate_dirty_bitmaps().. and I
still think the name is not well chosen - migration definitely has its own
dirty bitmap, and it has nothing to do with block layer..

Obviously, it was for postcopy support of block layer stuff, but I don't
know who is using it, how, and when..

Is people using this feature?  Maybe we should have an unit test, and a
doc?  Copying Vladimir.

>  
> -    if (migrate_postcopy_ram()) {
> -        /* Ping just for debugging, helps line traces up */
> -        qemu_savevm_send_ping(ms->to_dst_file, 2);
> -    }
> +    /* Ping just for debugging, helps line traces up */
> +    qemu_savevm_send_ping(ms->to_dst_file, 2);
>  
>      /*
>       * While loading the device state we may trigger page transfer
> @@ -2774,9 +2770,7 @@ static int postcopy_start(MigrationState *ms, Error **errp)
>          goto fail_closefb;
>      }
>  
> -    if (migrate_postcopy_ram()) {
> -        qemu_savevm_send_ping(fb, 3);
> -    }
> +    qemu_savevm_send_ping(fb, 3);
>  
>      qemu_savevm_send_postcopy_run(fb);
>  
> @@ -2807,13 +2801,11 @@ static int postcopy_start(MigrationState *ms, Error **errp)
>  
>      migration_downtime_end(ms);
>  
> -    if (migrate_postcopy_ram()) {
> -        /*
> -         * Although this ping is just for debug, it could potentially be
> -         * used for getting a better measurement of downtime at the source.
> -         */
> -        qemu_savevm_send_ping(ms->to_dst_file, 4);
> -    }
> +    /*
> +     * Although this ping is just for debug, it could potentially be
> +     * used for getting a better measurement of downtime at the source.
> +     */
> +    qemu_savevm_send_ping(ms->to_dst_file, 4);
>  
>      if (migrate_release_ram()) {
>          ram_postcopy_migrated_memory_release(ms);
> -- 
> 2.35.3
> 

-- 
Peter Xu