diff mbox series

[v2,05/22] migration/block-dirty-bitmap: refactor state global variables

Message ID 20200217150246.29180-6-vsementsov@virtuozzo.com
State New
Headers show
Series Fix error handling during bitmap postcopy | expand

Commit Message

Vladimir Sementsov-Ogievskiy Feb. 17, 2020, 3:02 p.m. UTC
Move all state variables into one global struct. Reduce global
variable usage, utilizing opaque pointer where possible.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 migration/block-dirty-bitmap.c | 171 ++++++++++++++++++---------------
 1 file changed, 95 insertions(+), 76 deletions(-)

Comments

Andrey Shinkevich Feb. 18, 2020, 1:05 p.m. UTC | #1
On 17/02/2020 18:02, Vladimir Sementsov-Ogievskiy wrote:
> Move all state variables into one global struct. Reduce global
> variable usage, utilizing opaque pointer where possible.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   migration/block-dirty-bitmap.c | 171 ++++++++++++++++++---------------
>   1 file changed, 95 insertions(+), 76 deletions(-)
> 
> diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
> index 49d4cf8810..7a82b76809 100644
> --- a/migration/block-dirty-bitmap.c
> +++ b/migration/block-dirty-bitmap.c
> @@ -128,6 +128,12 @@ typedef struct DBMSaveState {
>       BdrvDirtyBitmap *prev_bitmap;
>   } DBMSaveState;
>   
> +typedef struct LoadBitmapState {
> +    BlockDriverState *bs;
> +    BdrvDirtyBitmap *bitmap;
> +    bool migrated;
> +} LoadBitmapState;
> +
>   /* State of the dirty bitmap migration (DBM) during load process */
>   typedef struct DBMLoadState {
>       uint32_t flags;
> @@ -135,18 +141,17 @@ typedef struct DBMLoadState {
>       char bitmap_name[256];
>       BlockDriverState *bs;
>       BdrvDirtyBitmap *bitmap;
> +
> +    GSList *enabled_bitmaps;
> +    QemuMutex finish_lock;
>   } DBMLoadState;
>   
> -static DBMSaveState dirty_bitmap_mig_state;
> +typedef struct DBMState {
> +    DBMSaveState save;
> +    DBMLoadState load;
> +} DBMState;
>   
> -/* State of one bitmap during load process */
> -typedef struct LoadBitmapState {
> -    BlockDriverState *bs;
> -    BdrvDirtyBitmap *bitmap;
> -    bool migrated;
> -} LoadBitmapState;
> -static GSList *enabled_bitmaps;
> -QemuMutex finish_lock;
> +static DBMState dbm_state;
>   
>   static uint32_t qemu_get_bitmap_flags(QEMUFile *f)
>   {
> @@ -169,21 +174,21 @@ static void qemu_put_bitmap_flags(QEMUFile *f, uint32_t flags)
>       qemu_put_byte(f, flags);
>   }
>   
> -static void send_bitmap_header(QEMUFile *f, SaveBitmapState *dbms,
> -                               uint32_t additional_flags)
> +static void send_bitmap_header(QEMUFile *f, DBMSaveState *s,
> +                               SaveBitmapState *dbms, uint32_t additional_flags)
>   {
>       BlockDriverState *bs = dbms->bs;
>       BdrvDirtyBitmap *bitmap = dbms->bitmap;
>       uint32_t flags = additional_flags;
>       trace_send_bitmap_header_enter();
>   
> -    if (bs != dirty_bitmap_mig_state.prev_bs) {
> -        dirty_bitmap_mig_state.prev_bs = bs;
> +    if (bs != s->prev_bs) {
> +        s->prev_bs = bs;
>           flags |= DIRTY_BITMAP_MIG_FLAG_DEVICE_NAME;
>       }
>   
> -    if (bitmap != dirty_bitmap_mig_state.prev_bitmap) {
> -        dirty_bitmap_mig_state.prev_bitmap = bitmap;
> +    if (bitmap != s->prev_bitmap) {
> +        s->prev_bitmap = bitmap;
>           flags |= DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME;
>       }
>   
> @@ -198,19 +203,22 @@ static void send_bitmap_header(QEMUFile *f, SaveBitmapState *dbms,
>       }
>   }
>   
> -static void send_bitmap_start(QEMUFile *f, SaveBitmapState *dbms)
> +static void send_bitmap_start(QEMUFile *f, DBMSaveState *s,
> +                              SaveBitmapState *dbms)
>   {
> -    send_bitmap_header(f, dbms, DIRTY_BITMAP_MIG_FLAG_START);
> +    send_bitmap_header(f, s, dbms, DIRTY_BITMAP_MIG_FLAG_START);
>       qemu_put_be32(f, bdrv_dirty_bitmap_granularity(dbms->bitmap));
>       qemu_put_byte(f, dbms->flags);
>   }
>   
> -static void send_bitmap_complete(QEMUFile *f, SaveBitmapState *dbms)
> +static void send_bitmap_complete(QEMUFile *f, DBMSaveState *s,
> +                                 SaveBitmapState *dbms)
>   {
> -    send_bitmap_header(f, dbms, DIRTY_BITMAP_MIG_FLAG_COMPLETE);
> +    send_bitmap_header(f, s, dbms, DIRTY_BITMAP_MIG_FLAG_COMPLETE);
>   }
>   
> -static void send_bitmap_bits(QEMUFile *f, SaveBitmapState *dbms,
> +static void send_bitmap_bits(QEMUFile *f, DBMSaveState *s,
> +                             SaveBitmapState *dbms,
>                                uint64_t start_sector, uint32_t nr_sectors)
>   {
>       /* align for buffer_is_zero() */
> @@ -235,7 +243,7 @@ static void send_bitmap_bits(QEMUFile *f, SaveBitmapState *dbms,
>   
>       trace_send_bitmap_bits(flags, start_sector, nr_sectors, buf_size);
>   
> -    send_bitmap_header(f, dbms, flags);
> +    send_bitmap_header(f, s, dbms, flags);
>   
>       qemu_put_be64(f, start_sector);
>       qemu_put_be32(f, nr_sectors);
> @@ -254,12 +262,12 @@ static void send_bitmap_bits(QEMUFile *f, SaveBitmapState *dbms,
>   }
>   
>   /* Called with iothread lock taken.  */
> -static void dirty_bitmap_do_save_cleanup(void)
> +static void dirty_bitmap_do_save_cleanup(DBMSaveState *s)
>   {
>       SaveBitmapState *dbms;
>   
> -    while ((dbms = QSIMPLEQ_FIRST(&dirty_bitmap_mig_state.dbms_list)) != NULL) {
> -        QSIMPLEQ_REMOVE_HEAD(&dirty_bitmap_mig_state.dbms_list, entry);
> +    while ((dbms = QSIMPLEQ_FIRST(&s->dbms_list)) != NULL) {
> +        QSIMPLEQ_REMOVE_HEAD(&s->dbms_list, entry);
>           bdrv_dirty_bitmap_set_busy(dbms->bitmap, false);
>           bdrv_unref(dbms->bs);
>           g_free(dbms);
> @@ -267,17 +275,17 @@ static void dirty_bitmap_do_save_cleanup(void)
>   }
>   
>   /* Called with iothread lock taken. */
> -static int init_dirty_bitmap_migration(void)
> +static int init_dirty_bitmap_migration(DBMSaveState *s)
>   {
>       BlockDriverState *bs;
>       BdrvDirtyBitmap *bitmap;
>       SaveBitmapState *dbms;
>       Error *local_err = NULL;
>   
> -    dirty_bitmap_mig_state.bulk_completed = false;
> -    dirty_bitmap_mig_state.prev_bs = NULL;
> -    dirty_bitmap_mig_state.prev_bitmap = NULL;
> -    dirty_bitmap_mig_state.no_bitmaps = false;
> +    s->bulk_completed = false;
> +    s->prev_bs = NULL;
> +    s->prev_bitmap = NULL;
> +    s->no_bitmaps = false;
>   
>       for (bs = bdrv_next_all_states(NULL); bs; bs = bdrv_next_all_states(bs)) {
>           const char *name = bdrv_get_device_or_node_name(bs);
> @@ -316,35 +324,36 @@ static int init_dirty_bitmap_migration(void)
>                   dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT;
>               }
>   
> -            QSIMPLEQ_INSERT_TAIL(&dirty_bitmap_mig_state.dbms_list,
> +            QSIMPLEQ_INSERT_TAIL(&s->dbms_list,
>                                    dbms, entry);
>           }
>       }
>   
>       /* unset migration flags here, to not roll back it */
> -    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
> +    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
>           bdrv_dirty_bitmap_skip_store(dbms->bitmap, true);
>       }
>   
> -    if (QSIMPLEQ_EMPTY(&dirty_bitmap_mig_state.dbms_list)) {
> -        dirty_bitmap_mig_state.no_bitmaps = true;
> +    if (QSIMPLEQ_EMPTY(&s->dbms_list)) {
> +        s->no_bitmaps = true;
>       }
>   
>       return 0;
>   
>   fail:
> -    dirty_bitmap_do_save_cleanup();
> +    dirty_bitmap_do_save_cleanup(s);
>   
>       return -1;
>   }
>   
>   /* Called with no lock taken.  */
> -static void bulk_phase_send_chunk(QEMUFile *f, SaveBitmapState *dbms)
> +static void bulk_phase_send_chunk(QEMUFile *f, DBMSaveState *s,
> +                                  SaveBitmapState *dbms)
>   {
>       uint32_t nr_sectors = MIN(dbms->total_sectors - dbms->cur_sector,
>                                dbms->sectors_per_chunk);
>   
> -    send_bitmap_bits(f, dbms, dbms->cur_sector, nr_sectors);
> +    send_bitmap_bits(f, s, dbms, dbms->cur_sector, nr_sectors);
>   
>       dbms->cur_sector += nr_sectors;
>       if (dbms->cur_sector >= dbms->total_sectors) {
> @@ -353,61 +362,66 @@ static void bulk_phase_send_chunk(QEMUFile *f, SaveBitmapState *dbms)
>   }
>   
>   /* Called with no lock taken.  */
> -static void bulk_phase(QEMUFile *f, bool limit)
> +static void bulk_phase(QEMUFile *f, DBMSaveState *s, bool limit)
>   {
>       SaveBitmapState *dbms;
>   
> -    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
> +    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
>           while (!dbms->bulk_completed) {
> -            bulk_phase_send_chunk(f, dbms);
> +            bulk_phase_send_chunk(f, s, dbms);
>               if (limit && qemu_file_rate_limit(f)) {
>                   return;
>               }
>           }
>       }
>   
> -    dirty_bitmap_mig_state.bulk_completed = true;
> +    s->bulk_completed = true;
>   }
>   
>   /* for SaveVMHandlers */
>   static void dirty_bitmap_save_cleanup(void *opaque)
>   {
> -    dirty_bitmap_do_save_cleanup();
> +    DBMSaveState *s = &((DBMState *)opaque)->save;
> +
> +    dirty_bitmap_do_save_cleanup(s);
>   }

Why do one need the extra nested "do" function?

>   
>   static int dirty_bitmap_save_iterate(QEMUFile *f, void *opaque)
>   {
> +    DBMSaveState *s = &((DBMState *)opaque)->save;
> +
>       trace_dirty_bitmap_save_iterate(migration_in_postcopy());
>   
> -    if (migration_in_postcopy() && !dirty_bitmap_mig_state.bulk_completed) {
> -        bulk_phase(f, true);
> +    if (migration_in_postcopy() && !s->bulk_completed) {
> +        bulk_phase(f, s, true);
>       }
>   
>       qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS);
>   
> -    return dirty_bitmap_mig_state.bulk_completed;
> +    return s->bulk_completed;
>   }
>   
>   /* Called with iothread lock taken.  */
>   
>   static int dirty_bitmap_save_complete(QEMUFile *f, void *opaque)
>   {
> +    DBMSaveState *s = &((DBMState *)opaque)->save;
>       SaveBitmapState *dbms;
>       trace_dirty_bitmap_save_complete_enter();
>   
> -    if (!dirty_bitmap_mig_state.bulk_completed) {
> -        bulk_phase(f, false);
> +    if (!s->bulk_completed) {
> +        bulk_phase(f, s, false);
>       }
>   
> -    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
> -        send_bitmap_complete(f, dbms);
> +    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
> +        send_bitmap_complete(f, s, dbms);
>       }
>   
>       qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS);
>   
>       trace_dirty_bitmap_save_complete_finish();
>   
> -    dirty_bitmap_do_save_cleanup();
> +    dirty_bitmap_save_cleanup(opaque);
>       return 0;
>   }
>   
> @@ -417,12 +431,13 @@ static void dirty_bitmap_save_pending(QEMUFile *f, void *opaque,
>                                         uint64_t *res_compatible,
>                                         uint64_t *res_postcopy_only)
>   {
> +    DBMSaveState *s = &((DBMState *)opaque)->save;
>       SaveBitmapState *dbms;
>       uint64_t pending = 0;
>   
>       qemu_mutex_lock_iothread();
>   
> -    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
> +    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
>           uint64_t gran = bdrv_dirty_bitmap_granularity(dbms->bitmap);
>           uint64_t sectors = dbms->bulk_completed ? 0 :
>                              dbms->total_sectors - dbms->cur_sector;
> @@ -481,7 +496,7 @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s)
>           b->bs = s->bs;
>           b->bitmap = s->bitmap;
>           b->migrated = false;
> -        enabled_bitmaps = g_slist_prepend(enabled_bitmaps, b);
> +        s->enabled_bitmaps = g_slist_prepend(s->enabled_bitmaps, b);
>       }
>   
>       return 0;
> @@ -489,11 +504,12 @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s)
>   
>   void dirty_bitmap_mig_before_vm_start(void)
>   {
> +    DBMLoadState *s = &dbm_state.load;
>       GSList *item;
>   
> -    qemu_mutex_lock(&finish_lock);
> +    qemu_mutex_lock(&s->finish_lock);
>   
> -    for (item = enabled_bitmaps; item; item = g_slist_next(item)) {
> +    for (item = s->enabled_bitmaps; item; item = g_slist_next(item)) {
>           LoadBitmapState *b = item->data;
>   
>           if (b->migrated) {
> @@ -505,10 +521,10 @@ void dirty_bitmap_mig_before_vm_start(void)
>           g_free(b);
>       }
>   
> -    g_slist_free(enabled_bitmaps);
> -    enabled_bitmaps = NULL;
> +    g_slist_free(s->enabled_bitmaps);
> +    s->enabled_bitmaps = NULL;
>   
> -    qemu_mutex_unlock(&finish_lock);
> +    qemu_mutex_unlock(&s->finish_lock);
>   }
>   
>   static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s)
> @@ -517,9 +533,9 @@ static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s)
>       trace_dirty_bitmap_load_complete();
>       bdrv_dirty_bitmap_deserialize_finish(s->bitmap);
>   
> -    qemu_mutex_lock(&finish_lock);
> +    qemu_mutex_lock(&s->finish_lock);
>   
> -    for (item = enabled_bitmaps; item; item = g_slist_next(item)) {
> +    for (item = s->enabled_bitmaps; item; item = g_slist_next(item)) {
>           LoadBitmapState *b = item->data;
>   
>           if (b->bitmap == s->bitmap) {
> @@ -530,7 +546,7 @@ static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s)
>   
>       if (bdrv_dirty_bitmap_has_successor(s->bitmap)) {
>           bdrv_dirty_bitmap_lock(s->bitmap);
> -        if (enabled_bitmaps == NULL) {
> +        if (s->enabled_bitmaps == NULL) {
>               /* in postcopy */
>               bdrv_reclaim_dirty_bitmap_locked(s->bitmap, &error_abort);
>               bdrv_enable_dirty_bitmap_locked(s->bitmap);
> @@ -549,7 +565,7 @@ static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s)
>           bdrv_dirty_bitmap_unlock(s->bitmap);
>       }
>   
> -    qemu_mutex_unlock(&finish_lock);
> +    qemu_mutex_unlock(&s->finish_lock);
>   }
>   
>   static int dirty_bitmap_load_bits(QEMUFile *f, DBMLoadState *s)
> @@ -646,7 +662,7 @@ static int dirty_bitmap_load_header(QEMUFile *f, DBMLoadState *s)
>   
>   static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
>   {
> -    static DBMLoadState s;
> +    DBMLoadState *s = &((DBMState *)opaque)->load;
>       int ret = 0;
>   
>       trace_dirty_bitmap_load_enter();
> @@ -656,17 +672,17 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
>       }
>   
>       do {
> -        ret = dirty_bitmap_load_header(f, &s);
> +        ret = dirty_bitmap_load_header(f, s);
>           if (ret < 0) {
>               return ret;
>           }
>   
> -        if (s.flags & DIRTY_BITMAP_MIG_FLAG_START) {
> -            ret = dirty_bitmap_load_start(f, &s);
> -        } else if (s.flags & DIRTY_BITMAP_MIG_FLAG_COMPLETE) {
> -            dirty_bitmap_load_complete(f, &s);
> -        } else if (s.flags & DIRTY_BITMAP_MIG_FLAG_BITS) {
> -            ret = dirty_bitmap_load_bits(f, &s);
> +        if (s->flags & DIRTY_BITMAP_MIG_FLAG_START) {
> +            ret = dirty_bitmap_load_start(f, s);
> +        } else if (s->flags & DIRTY_BITMAP_MIG_FLAG_COMPLETE) {
> +            dirty_bitmap_load_complete(f, s);
> +        } else if (s->flags & DIRTY_BITMAP_MIG_FLAG_BITS) {
> +            ret = dirty_bitmap_load_bits(f, s);
>           }
>   
>           if (!ret) {
> @@ -676,7 +692,7 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
>           if (ret) {
>               return ret;
>           }
> -    } while (!(s.flags & DIRTY_BITMAP_MIG_FLAG_EOS));
> +    } while (!(s->flags & DIRTY_BITMAP_MIG_FLAG_EOS));
>   
>       trace_dirty_bitmap_load_success();
>       return 0;
> @@ -684,13 +700,14 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
>   
>   static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque)
>   {
> +    DBMSaveState *s = &((DBMState *)opaque)->save;
>       SaveBitmapState *dbms = NULL;
> -    if (init_dirty_bitmap_migration() < 0) {
> +    if (init_dirty_bitmap_migration(s) < 0) {
>           return -1;
>       }
>   
> -    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
> -        send_bitmap_start(f, dbms);
> +    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
> +        send_bitmap_start(f, s, dbms);
>       }
>       qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS);
>   
> @@ -699,7 +716,9 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque)
>   
>   static bool dirty_bitmap_is_active(void *opaque)
>   {
> -    return migrate_dirty_bitmaps() && !dirty_bitmap_mig_state.no_bitmaps;
> +    DBMSaveState *s = &((DBMState *)opaque)->save;
> +
> +    return migrate_dirty_bitmaps() && !s->no_bitmaps;
>   }
>   
>   static bool dirty_bitmap_is_active_iterate(void *opaque)
> @@ -727,10 +746,10 @@ static SaveVMHandlers savevm_dirty_bitmap_handlers = {
>   
>   void dirty_bitmap_mig_init(void)
>   {
> -    QSIMPLEQ_INIT(&dirty_bitmap_mig_state.dbms_list);
> -    qemu_mutex_init(&finish_lock);
> +    QSIMPLEQ_INIT(&dbm_state.save.dbms_list);
> +    qemu_mutex_init(&dbm_state.load.finish_lock);
>   
>       register_savevm_live("dirty-bitmap", 0, 1,
>                            &savevm_dirty_bitmap_handlers,
> -                         &dirty_bitmap_mig_state);
> +                         &dbm_state);
>   }
> 

Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Vladimir Sementsov-Ogievskiy Feb. 19, 2020, 3:29 p.m. UTC | #2
18.02.2020 16:05, Andrey Shinkevich wrote:
> 
> 
> On 17/02/2020 18:02, Vladimir Sementsov-Ogievskiy wrote:
>> Move all state variables into one global struct. Reduce global
>> variable usage, utilizing opaque pointer where possible.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   migration/block-dirty-bitmap.c | 171 ++++++++++++++++++---------------
>>   1 file changed, 95 insertions(+), 76 deletions(-)
>>
>> diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
>> index 49d4cf8810..7a82b76809 100644
>> --- a/migration/block-dirty-bitmap.c
>> +++ b/migration/block-dirty-bitmap.c
>> @@ -128,6 +128,12 @@ typedef struct DBMSaveState {
>>       BdrvDirtyBitmap *prev_bitmap;
>>   } DBMSaveState;
>> +typedef struct LoadBitmapState {
>> +    BlockDriverState *bs;
>> +    BdrvDirtyBitmap *bitmap;
>> +    bool migrated;
>> +} LoadBitmapState;
>> +
>>   /* State of the dirty bitmap migration (DBM) during load process */
>>   typedef struct DBMLoadState {
>>       uint32_t flags;
>> @@ -135,18 +141,17 @@ typedef struct DBMLoadState {
>>       char bitmap_name[256];
>>       BlockDriverState *bs;
>>       BdrvDirtyBitmap *bitmap;
>> +
>> +    GSList *enabled_bitmaps;
>> +    QemuMutex finish_lock;
>>   } DBMLoadState;
>> -static DBMSaveState dirty_bitmap_mig_state;
>> +typedef struct DBMState {
>> +    DBMSaveState save;
>> +    DBMLoadState load;
>> +} DBMState;
>> -/* State of one bitmap during load process */
>> -typedef struct LoadBitmapState {
>> -    BlockDriverState *bs;
>> -    BdrvDirtyBitmap *bitmap;
>> -    bool migrated;
>> -} LoadBitmapState;
>> -static GSList *enabled_bitmaps;
>> -QemuMutex finish_lock;
>> +static DBMState dbm_state;
>>   static uint32_t qemu_get_bitmap_flags(QEMUFile *f)
>>   {
>> @@ -169,21 +174,21 @@ static void qemu_put_bitmap_flags(QEMUFile *f, uint32_t flags)
>>       qemu_put_byte(f, flags);
>>   }
>> -static void send_bitmap_header(QEMUFile *f, SaveBitmapState *dbms,
>> -                               uint32_t additional_flags)
>> +static void send_bitmap_header(QEMUFile *f, DBMSaveState *s,
>> +                               SaveBitmapState *dbms, uint32_t additional_flags)
>>   {
>>       BlockDriverState *bs = dbms->bs;
>>       BdrvDirtyBitmap *bitmap = dbms->bitmap;
>>       uint32_t flags = additional_flags;
>>       trace_send_bitmap_header_enter();
>> -    if (bs != dirty_bitmap_mig_state.prev_bs) {
>> -        dirty_bitmap_mig_state.prev_bs = bs;
>> +    if (bs != s->prev_bs) {
>> +        s->prev_bs = bs;
>>           flags |= DIRTY_BITMAP_MIG_FLAG_DEVICE_NAME;
>>       }
>> -    if (bitmap != dirty_bitmap_mig_state.prev_bitmap) {
>> -        dirty_bitmap_mig_state.prev_bitmap = bitmap;
>> +    if (bitmap != s->prev_bitmap) {
>> +        s->prev_bitmap = bitmap;
>>           flags |= DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME;
>>       }
>> @@ -198,19 +203,22 @@ static void send_bitmap_header(QEMUFile *f, SaveBitmapState *dbms,
>>       }
>>   }
>> -static void send_bitmap_start(QEMUFile *f, SaveBitmapState *dbms)
>> +static void send_bitmap_start(QEMUFile *f, DBMSaveState *s,
>> +                              SaveBitmapState *dbms)
>>   {
>> -    send_bitmap_header(f, dbms, DIRTY_BITMAP_MIG_FLAG_START);
>> +    send_bitmap_header(f, s, dbms, DIRTY_BITMAP_MIG_FLAG_START);
>>       qemu_put_be32(f, bdrv_dirty_bitmap_granularity(dbms->bitmap));
>>       qemu_put_byte(f, dbms->flags);
>>   }
>> -static void send_bitmap_complete(QEMUFile *f, SaveBitmapState *dbms)
>> +static void send_bitmap_complete(QEMUFile *f, DBMSaveState *s,
>> +                                 SaveBitmapState *dbms)
>>   {
>> -    send_bitmap_header(f, dbms, DIRTY_BITMAP_MIG_FLAG_COMPLETE);
>> +    send_bitmap_header(f, s, dbms, DIRTY_BITMAP_MIG_FLAG_COMPLETE);
>>   }
>> -static void send_bitmap_bits(QEMUFile *f, SaveBitmapState *dbms,
>> +static void send_bitmap_bits(QEMUFile *f, DBMSaveState *s,
>> +                             SaveBitmapState *dbms,
>>                                uint64_t start_sector, uint32_t nr_sectors)
>>   {
>>       /* align for buffer_is_zero() */
>> @@ -235,7 +243,7 @@ static void send_bitmap_bits(QEMUFile *f, SaveBitmapState *dbms,
>>       trace_send_bitmap_bits(flags, start_sector, nr_sectors, buf_size);
>> -    send_bitmap_header(f, dbms, flags);
>> +    send_bitmap_header(f, s, dbms, flags);
>>       qemu_put_be64(f, start_sector);
>>       qemu_put_be32(f, nr_sectors);
>> @@ -254,12 +262,12 @@ static void send_bitmap_bits(QEMUFile *f, SaveBitmapState *dbms,
>>   }
>>   /* Called with iothread lock taken.  */
>> -static void dirty_bitmap_do_save_cleanup(void)
>> +static void dirty_bitmap_do_save_cleanup(DBMSaveState *s)
>>   {
>>       SaveBitmapState *dbms;
>> -    while ((dbms = QSIMPLEQ_FIRST(&dirty_bitmap_mig_state.dbms_list)) != NULL) {
>> -        QSIMPLEQ_REMOVE_HEAD(&dirty_bitmap_mig_state.dbms_list, entry);
>> +    while ((dbms = QSIMPLEQ_FIRST(&s->dbms_list)) != NULL) {
>> +        QSIMPLEQ_REMOVE_HEAD(&s->dbms_list, entry);
>>           bdrv_dirty_bitmap_set_busy(dbms->bitmap, false);
>>           bdrv_unref(dbms->bs);
>>           g_free(dbms);
>> @@ -267,17 +275,17 @@ static void dirty_bitmap_do_save_cleanup(void)
>>   }
>>   /* Called with iothread lock taken. */
>> -static int init_dirty_bitmap_migration(void)
>> +static int init_dirty_bitmap_migration(DBMSaveState *s)
>>   {
>>       BlockDriverState *bs;
>>       BdrvDirtyBitmap *bitmap;
>>       SaveBitmapState *dbms;
>>       Error *local_err = NULL;
>> -    dirty_bitmap_mig_state.bulk_completed = false;
>> -    dirty_bitmap_mig_state.prev_bs = NULL;
>> -    dirty_bitmap_mig_state.prev_bitmap = NULL;
>> -    dirty_bitmap_mig_state.no_bitmaps = false;
>> +    s->bulk_completed = false;
>> +    s->prev_bs = NULL;
>> +    s->prev_bitmap = NULL;
>> +    s->no_bitmaps = false;
>>       for (bs = bdrv_next_all_states(NULL); bs; bs = bdrv_next_all_states(bs)) {
>>           const char *name = bdrv_get_device_or_node_name(bs);
>> @@ -316,35 +324,36 @@ static int init_dirty_bitmap_migration(void)
>>                   dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT;
>>               }
>> -            QSIMPLEQ_INSERT_TAIL(&dirty_bitmap_mig_state.dbms_list,
>> +            QSIMPLEQ_INSERT_TAIL(&s->dbms_list,
>>                                    dbms, entry);
>>           }
>>       }
>>       /* unset migration flags here, to not roll back it */
>> -    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
>> +    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
>>           bdrv_dirty_bitmap_skip_store(dbms->bitmap, true);
>>       }
>> -    if (QSIMPLEQ_EMPTY(&dirty_bitmap_mig_state.dbms_list)) {
>> -        dirty_bitmap_mig_state.no_bitmaps = true;
>> +    if (QSIMPLEQ_EMPTY(&s->dbms_list)) {
>> +        s->no_bitmaps = true;
>>       }
>>       return 0;
>>   fail:
>> -    dirty_bitmap_do_save_cleanup();
>> +    dirty_bitmap_do_save_cleanup(s);
>>       return -1;
>>   }
>>   /* Called with no lock taken.  */
>> -static void bulk_phase_send_chunk(QEMUFile *f, SaveBitmapState *dbms)
>> +static void bulk_phase_send_chunk(QEMUFile *f, DBMSaveState *s,
>> +                                  SaveBitmapState *dbms)
>>   {
>>       uint32_t nr_sectors = MIN(dbms->total_sectors - dbms->cur_sector,
>>                                dbms->sectors_per_chunk);
>> -    send_bitmap_bits(f, dbms, dbms->cur_sector, nr_sectors);
>> +    send_bitmap_bits(f, s, dbms, dbms->cur_sector, nr_sectors);
>>       dbms->cur_sector += nr_sectors;
>>       if (dbms->cur_sector >= dbms->total_sectors) {
>> @@ -353,61 +362,66 @@ static void bulk_phase_send_chunk(QEMUFile *f, SaveBitmapState *dbms)
>>   }
>>   /* Called with no lock taken.  */
>> -static void bulk_phase(QEMUFile *f, bool limit)
>> +static void bulk_phase(QEMUFile *f, DBMSaveState *s, bool limit)
>>   {
>>       SaveBitmapState *dbms;
>> -    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
>> +    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
>>           while (!dbms->bulk_completed) {
>> -            bulk_phase_send_chunk(f, dbms);
>> +            bulk_phase_send_chunk(f, s, dbms);
>>               if (limit && qemu_file_rate_limit(f)) {
>>                   return;
>>               }
>>           }
>>       }
>> -    dirty_bitmap_mig_state.bulk_completed = true;
>> +    s->bulk_completed = true;
>>   }
>>   /* for SaveVMHandlers */
>>   static void dirty_bitmap_save_cleanup(void *opaque)
>>   {
>> -    dirty_bitmap_do_save_cleanup();
>> +    DBMSaveState *s = &((DBMState *)opaque)->save;
>> +
>> +    dirty_bitmap_do_save_cleanup(s);
>>   }
> 
> Why do one need the extra nested "do" function?

Because "_do_" has sub-structure argument, and I don't have pointer to the
whole structure in another "_do_" caller..

> 
>>   static int dirty_bitmap_save_iterate(QEMUFile *f, void *opaque)
>>   {
>> +    DBMSaveState *s = &((DBMState *)opaque)->save;
>> +
>>       trace_dirty_bitmap_save_iterate(migration_in_postcopy());
>> -    if (migration_in_postcopy() && !dirty_bitmap_mig_state.bulk_completed) {
>> -        bulk_phase(f, true);
>> +    if (migration_in_postcopy() && !s->bulk_completed) {
>> +        bulk_phase(f, s, true);
>>       }
>>       qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS);
>> -    return dirty_bitmap_mig_state.bulk_completed;
>> +    return s->bulk_completed;
>>   }
>>   /* Called with iothread lock taken.  */
>>   static int dirty_bitmap_save_complete(QEMUFile *f, void *opaque)
>>   {
>> +    DBMSaveState *s = &((DBMState *)opaque)->save;
>>       SaveBitmapState *dbms;
>>       trace_dirty_bitmap_save_complete_enter();
>> -    if (!dirty_bitmap_mig_state.bulk_completed) {
>> -        bulk_phase(f, false);
>> +    if (!s->bulk_completed) {
>> +        bulk_phase(f, s, false);
>>       }
>> -    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
>> -        send_bitmap_complete(f, dbms);
>> +    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
>> +        send_bitmap_complete(f, s, dbms);
>>       }
>>       qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS);
>>       trace_dirty_bitmap_save_complete_finish();
>> -    dirty_bitmap_do_save_cleanup();
>> +    dirty_bitmap_save_cleanup(opaque);
>>       return 0;
>>   }
>> @@ -417,12 +431,13 @@ static void dirty_bitmap_save_pending(QEMUFile *f, void *opaque,
>>                                         uint64_t *res_compatible,
>>                                         uint64_t *res_postcopy_only)
>>   {
>> +    DBMSaveState *s = &((DBMState *)opaque)->save;
>>       SaveBitmapState *dbms;
>>       uint64_t pending = 0;
>>       qemu_mutex_lock_iothread();
>> -    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
>> +    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
>>           uint64_t gran = bdrv_dirty_bitmap_granularity(dbms->bitmap);
>>           uint64_t sectors = dbms->bulk_completed ? 0 :
>>                              dbms->total_sectors - dbms->cur_sector;
>> @@ -481,7 +496,7 @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s)
>>           b->bs = s->bs;
>>           b->bitmap = s->bitmap;
>>           b->migrated = false;
>> -        enabled_bitmaps = g_slist_prepend(enabled_bitmaps, b);
>> +        s->enabled_bitmaps = g_slist_prepend(s->enabled_bitmaps, b);
>>       }
>>       return 0;
>> @@ -489,11 +504,12 @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s)
>>   void dirty_bitmap_mig_before_vm_start(void)
>>   {
>> +    DBMLoadState *s = &dbm_state.load;
>>       GSList *item;
>> -    qemu_mutex_lock(&finish_lock);
>> +    qemu_mutex_lock(&s->finish_lock);
>> -    for (item = enabled_bitmaps; item; item = g_slist_next(item)) {
>> +    for (item = s->enabled_bitmaps; item; item = g_slist_next(item)) {
>>           LoadBitmapState *b = item->data;
>>           if (b->migrated) {
>> @@ -505,10 +521,10 @@ void dirty_bitmap_mig_before_vm_start(void)
>>           g_free(b);
>>       }
>> -    g_slist_free(enabled_bitmaps);
>> -    enabled_bitmaps = NULL;
>> +    g_slist_free(s->enabled_bitmaps);
>> +    s->enabled_bitmaps = NULL;
>> -    qemu_mutex_unlock(&finish_lock);
>> +    qemu_mutex_unlock(&s->finish_lock);
>>   }
>>   static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s)
>> @@ -517,9 +533,9 @@ static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s)
>>       trace_dirty_bitmap_load_complete();
>>       bdrv_dirty_bitmap_deserialize_finish(s->bitmap);
>> -    qemu_mutex_lock(&finish_lock);
>> +    qemu_mutex_lock(&s->finish_lock);
>> -    for (item = enabled_bitmaps; item; item = g_slist_next(item)) {
>> +    for (item = s->enabled_bitmaps; item; item = g_slist_next(item)) {
>>           LoadBitmapState *b = item->data;
>>           if (b->bitmap == s->bitmap) {
>> @@ -530,7 +546,7 @@ static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s)
>>       if (bdrv_dirty_bitmap_has_successor(s->bitmap)) {
>>           bdrv_dirty_bitmap_lock(s->bitmap);
>> -        if (enabled_bitmaps == NULL) {
>> +        if (s->enabled_bitmaps == NULL) {
>>               /* in postcopy */
>>               bdrv_reclaim_dirty_bitmap_locked(s->bitmap, &error_abort);
>>               bdrv_enable_dirty_bitmap_locked(s->bitmap);
>> @@ -549,7 +565,7 @@ static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s)
>>           bdrv_dirty_bitmap_unlock(s->bitmap);
>>       }
>> -    qemu_mutex_unlock(&finish_lock);
>> +    qemu_mutex_unlock(&s->finish_lock);
>>   }
>>   static int dirty_bitmap_load_bits(QEMUFile *f, DBMLoadState *s)
>> @@ -646,7 +662,7 @@ static int dirty_bitmap_load_header(QEMUFile *f, DBMLoadState *s)
>>   static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
>>   {
>> -    static DBMLoadState s;
>> +    DBMLoadState *s = &((DBMState *)opaque)->load;
>>       int ret = 0;
>>       trace_dirty_bitmap_load_enter();
>> @@ -656,17 +672,17 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
>>       }
>>       do {
>> -        ret = dirty_bitmap_load_header(f, &s);
>> +        ret = dirty_bitmap_load_header(f, s);
>>           if (ret < 0) {
>>               return ret;
>>           }
>> -        if (s.flags & DIRTY_BITMAP_MIG_FLAG_START) {
>> -            ret = dirty_bitmap_load_start(f, &s);
>> -        } else if (s.flags & DIRTY_BITMAP_MIG_FLAG_COMPLETE) {
>> -            dirty_bitmap_load_complete(f, &s);
>> -        } else if (s.flags & DIRTY_BITMAP_MIG_FLAG_BITS) {
>> -            ret = dirty_bitmap_load_bits(f, &s);
>> +        if (s->flags & DIRTY_BITMAP_MIG_FLAG_START) {
>> +            ret = dirty_bitmap_load_start(f, s);
>> +        } else if (s->flags & DIRTY_BITMAP_MIG_FLAG_COMPLETE) {
>> +            dirty_bitmap_load_complete(f, s);
>> +        } else if (s->flags & DIRTY_BITMAP_MIG_FLAG_BITS) {
>> +            ret = dirty_bitmap_load_bits(f, s);
>>           }
>>           if (!ret) {
>> @@ -676,7 +692,7 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
>>           if (ret) {
>>               return ret;
>>           }
>> -    } while (!(s.flags & DIRTY_BITMAP_MIG_FLAG_EOS));
>> +    } while (!(s->flags & DIRTY_BITMAP_MIG_FLAG_EOS));
>>       trace_dirty_bitmap_load_success();
>>       return 0;
>> @@ -684,13 +700,14 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
>>   static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque)
>>   {
>> +    DBMSaveState *s = &((DBMState *)opaque)->save;
>>       SaveBitmapState *dbms = NULL;
>> -    if (init_dirty_bitmap_migration() < 0) {
>> +    if (init_dirty_bitmap_migration(s) < 0) {
>>           return -1;
>>       }
>> -    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
>> -        send_bitmap_start(f, dbms);
>> +    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
>> +        send_bitmap_start(f, s, dbms);
>>       }
>>       qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS);
>> @@ -699,7 +716,9 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque)
>>   static bool dirty_bitmap_is_active(void *opaque)
>>   {
>> -    return migrate_dirty_bitmaps() && !dirty_bitmap_mig_state.no_bitmaps;
>> +    DBMSaveState *s = &((DBMState *)opaque)->save;
>> +
>> +    return migrate_dirty_bitmaps() && !s->no_bitmaps;
>>   }
>>   static bool dirty_bitmap_is_active_iterate(void *opaque)
>> @@ -727,10 +746,10 @@ static SaveVMHandlers savevm_dirty_bitmap_handlers = {
>>   void dirty_bitmap_mig_init(void)
>>   {
>> -    QSIMPLEQ_INIT(&dirty_bitmap_mig_state.dbms_list);
>> -    qemu_mutex_init(&finish_lock);
>> +    QSIMPLEQ_INIT(&dbm_state.save.dbms_list);
>> +    qemu_mutex_init(&dbm_state.load.finish_lock);
>>       register_savevm_live("dirty-bitmap", 0, 1,
>>                            &savevm_dirty_bitmap_handlers,
>> -                         &dirty_bitmap_mig_state);
>> +                         &dbm_state);
>>   }
>>
> 
> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
diff mbox series

Patch

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 49d4cf8810..7a82b76809 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -128,6 +128,12 @@  typedef struct DBMSaveState {
     BdrvDirtyBitmap *prev_bitmap;
 } DBMSaveState;
 
+typedef struct LoadBitmapState {
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *bitmap;
+    bool migrated;
+} LoadBitmapState;
+
 /* State of the dirty bitmap migration (DBM) during load process */
 typedef struct DBMLoadState {
     uint32_t flags;
@@ -135,18 +141,17 @@  typedef struct DBMLoadState {
     char bitmap_name[256];
     BlockDriverState *bs;
     BdrvDirtyBitmap *bitmap;
+
+    GSList *enabled_bitmaps;
+    QemuMutex finish_lock;
 } DBMLoadState;
 
-static DBMSaveState dirty_bitmap_mig_state;
+typedef struct DBMState {
+    DBMSaveState save;
+    DBMLoadState load;
+} DBMState;
 
-/* State of one bitmap during load process */
-typedef struct LoadBitmapState {
-    BlockDriverState *bs;
-    BdrvDirtyBitmap *bitmap;
-    bool migrated;
-} LoadBitmapState;
-static GSList *enabled_bitmaps;
-QemuMutex finish_lock;
+static DBMState dbm_state;
 
 static uint32_t qemu_get_bitmap_flags(QEMUFile *f)
 {
@@ -169,21 +174,21 @@  static void qemu_put_bitmap_flags(QEMUFile *f, uint32_t flags)
     qemu_put_byte(f, flags);
 }
 
-static void send_bitmap_header(QEMUFile *f, SaveBitmapState *dbms,
-                               uint32_t additional_flags)
+static void send_bitmap_header(QEMUFile *f, DBMSaveState *s,
+                               SaveBitmapState *dbms, uint32_t additional_flags)
 {
     BlockDriverState *bs = dbms->bs;
     BdrvDirtyBitmap *bitmap = dbms->bitmap;
     uint32_t flags = additional_flags;
     trace_send_bitmap_header_enter();
 
-    if (bs != dirty_bitmap_mig_state.prev_bs) {
-        dirty_bitmap_mig_state.prev_bs = bs;
+    if (bs != s->prev_bs) {
+        s->prev_bs = bs;
         flags |= DIRTY_BITMAP_MIG_FLAG_DEVICE_NAME;
     }
 
-    if (bitmap != dirty_bitmap_mig_state.prev_bitmap) {
-        dirty_bitmap_mig_state.prev_bitmap = bitmap;
+    if (bitmap != s->prev_bitmap) {
+        s->prev_bitmap = bitmap;
         flags |= DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME;
     }
 
@@ -198,19 +203,22 @@  static void send_bitmap_header(QEMUFile *f, SaveBitmapState *dbms,
     }
 }
 
-static void send_bitmap_start(QEMUFile *f, SaveBitmapState *dbms)
+static void send_bitmap_start(QEMUFile *f, DBMSaveState *s,
+                              SaveBitmapState *dbms)
 {
-    send_bitmap_header(f, dbms, DIRTY_BITMAP_MIG_FLAG_START);
+    send_bitmap_header(f, s, dbms, DIRTY_BITMAP_MIG_FLAG_START);
     qemu_put_be32(f, bdrv_dirty_bitmap_granularity(dbms->bitmap));
     qemu_put_byte(f, dbms->flags);
 }
 
-static void send_bitmap_complete(QEMUFile *f, SaveBitmapState *dbms)
+static void send_bitmap_complete(QEMUFile *f, DBMSaveState *s,
+                                 SaveBitmapState *dbms)
 {
-    send_bitmap_header(f, dbms, DIRTY_BITMAP_MIG_FLAG_COMPLETE);
+    send_bitmap_header(f, s, dbms, DIRTY_BITMAP_MIG_FLAG_COMPLETE);
 }
 
-static void send_bitmap_bits(QEMUFile *f, SaveBitmapState *dbms,
+static void send_bitmap_bits(QEMUFile *f, DBMSaveState *s,
+                             SaveBitmapState *dbms,
                              uint64_t start_sector, uint32_t nr_sectors)
 {
     /* align for buffer_is_zero() */
@@ -235,7 +243,7 @@  static void send_bitmap_bits(QEMUFile *f, SaveBitmapState *dbms,
 
     trace_send_bitmap_bits(flags, start_sector, nr_sectors, buf_size);
 
-    send_bitmap_header(f, dbms, flags);
+    send_bitmap_header(f, s, dbms, flags);
 
     qemu_put_be64(f, start_sector);
     qemu_put_be32(f, nr_sectors);
@@ -254,12 +262,12 @@  static void send_bitmap_bits(QEMUFile *f, SaveBitmapState *dbms,
 }
 
 /* Called with iothread lock taken.  */
-static void dirty_bitmap_do_save_cleanup(void)
+static void dirty_bitmap_do_save_cleanup(DBMSaveState *s)
 {
     SaveBitmapState *dbms;
 
-    while ((dbms = QSIMPLEQ_FIRST(&dirty_bitmap_mig_state.dbms_list)) != NULL) {
-        QSIMPLEQ_REMOVE_HEAD(&dirty_bitmap_mig_state.dbms_list, entry);
+    while ((dbms = QSIMPLEQ_FIRST(&s->dbms_list)) != NULL) {
+        QSIMPLEQ_REMOVE_HEAD(&s->dbms_list, entry);
         bdrv_dirty_bitmap_set_busy(dbms->bitmap, false);
         bdrv_unref(dbms->bs);
         g_free(dbms);
@@ -267,17 +275,17 @@  static void dirty_bitmap_do_save_cleanup(void)
 }
 
 /* Called with iothread lock taken. */
-static int init_dirty_bitmap_migration(void)
+static int init_dirty_bitmap_migration(DBMSaveState *s)
 {
     BlockDriverState *bs;
     BdrvDirtyBitmap *bitmap;
     SaveBitmapState *dbms;
     Error *local_err = NULL;
 
-    dirty_bitmap_mig_state.bulk_completed = false;
-    dirty_bitmap_mig_state.prev_bs = NULL;
-    dirty_bitmap_mig_state.prev_bitmap = NULL;
-    dirty_bitmap_mig_state.no_bitmaps = false;
+    s->bulk_completed = false;
+    s->prev_bs = NULL;
+    s->prev_bitmap = NULL;
+    s->no_bitmaps = false;
 
     for (bs = bdrv_next_all_states(NULL); bs; bs = bdrv_next_all_states(bs)) {
         const char *name = bdrv_get_device_or_node_name(bs);
@@ -316,35 +324,36 @@  static int init_dirty_bitmap_migration(void)
                 dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT;
             }
 
-            QSIMPLEQ_INSERT_TAIL(&dirty_bitmap_mig_state.dbms_list,
+            QSIMPLEQ_INSERT_TAIL(&s->dbms_list,
                                  dbms, entry);
         }
     }
 
     /* unset migration flags here, to not roll back it */
-    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
+    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
         bdrv_dirty_bitmap_skip_store(dbms->bitmap, true);
     }
 
-    if (QSIMPLEQ_EMPTY(&dirty_bitmap_mig_state.dbms_list)) {
-        dirty_bitmap_mig_state.no_bitmaps = true;
+    if (QSIMPLEQ_EMPTY(&s->dbms_list)) {
+        s->no_bitmaps = true;
     }
 
     return 0;
 
 fail:
-    dirty_bitmap_do_save_cleanup();
+    dirty_bitmap_do_save_cleanup(s);
 
     return -1;
 }
 
 /* Called with no lock taken.  */
-static void bulk_phase_send_chunk(QEMUFile *f, SaveBitmapState *dbms)
+static void bulk_phase_send_chunk(QEMUFile *f, DBMSaveState *s,
+                                  SaveBitmapState *dbms)
 {
     uint32_t nr_sectors = MIN(dbms->total_sectors - dbms->cur_sector,
                              dbms->sectors_per_chunk);
 
-    send_bitmap_bits(f, dbms, dbms->cur_sector, nr_sectors);
+    send_bitmap_bits(f, s, dbms, dbms->cur_sector, nr_sectors);
 
     dbms->cur_sector += nr_sectors;
     if (dbms->cur_sector >= dbms->total_sectors) {
@@ -353,61 +362,66 @@  static void bulk_phase_send_chunk(QEMUFile *f, SaveBitmapState *dbms)
 }
 
 /* Called with no lock taken.  */
-static void bulk_phase(QEMUFile *f, bool limit)
+static void bulk_phase(QEMUFile *f, DBMSaveState *s, bool limit)
 {
     SaveBitmapState *dbms;
 
-    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
+    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
         while (!dbms->bulk_completed) {
-            bulk_phase_send_chunk(f, dbms);
+            bulk_phase_send_chunk(f, s, dbms);
             if (limit && qemu_file_rate_limit(f)) {
                 return;
             }
         }
     }
 
-    dirty_bitmap_mig_state.bulk_completed = true;
+    s->bulk_completed = true;
 }
 
 /* for SaveVMHandlers */
 static void dirty_bitmap_save_cleanup(void *opaque)
 {
-    dirty_bitmap_do_save_cleanup();
+    DBMSaveState *s = &((DBMState *)opaque)->save;
+
+    dirty_bitmap_do_save_cleanup(s);
 }
 
 static int dirty_bitmap_save_iterate(QEMUFile *f, void *opaque)
 {
+    DBMSaveState *s = &((DBMState *)opaque)->save;
+
     trace_dirty_bitmap_save_iterate(migration_in_postcopy());
 
-    if (migration_in_postcopy() && !dirty_bitmap_mig_state.bulk_completed) {
-        bulk_phase(f, true);
+    if (migration_in_postcopy() && !s->bulk_completed) {
+        bulk_phase(f, s, true);
     }
 
     qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS);
 
-    return dirty_bitmap_mig_state.bulk_completed;
+    return s->bulk_completed;
 }
 
 /* Called with iothread lock taken.  */
 
 static int dirty_bitmap_save_complete(QEMUFile *f, void *opaque)
 {
+    DBMSaveState *s = &((DBMState *)opaque)->save;
     SaveBitmapState *dbms;
     trace_dirty_bitmap_save_complete_enter();
 
-    if (!dirty_bitmap_mig_state.bulk_completed) {
-        bulk_phase(f, false);
+    if (!s->bulk_completed) {
+        bulk_phase(f, s, false);
     }
 
-    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
-        send_bitmap_complete(f, dbms);
+    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
+        send_bitmap_complete(f, s, dbms);
     }
 
     qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS);
 
     trace_dirty_bitmap_save_complete_finish();
 
-    dirty_bitmap_do_save_cleanup();
+    dirty_bitmap_save_cleanup(opaque);
     return 0;
 }
 
@@ -417,12 +431,13 @@  static void dirty_bitmap_save_pending(QEMUFile *f, void *opaque,
                                       uint64_t *res_compatible,
                                       uint64_t *res_postcopy_only)
 {
+    DBMSaveState *s = &((DBMState *)opaque)->save;
     SaveBitmapState *dbms;
     uint64_t pending = 0;
 
     qemu_mutex_lock_iothread();
 
-    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
+    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
         uint64_t gran = bdrv_dirty_bitmap_granularity(dbms->bitmap);
         uint64_t sectors = dbms->bulk_completed ? 0 :
                            dbms->total_sectors - dbms->cur_sector;
@@ -481,7 +496,7 @@  static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s)
         b->bs = s->bs;
         b->bitmap = s->bitmap;
         b->migrated = false;
-        enabled_bitmaps = g_slist_prepend(enabled_bitmaps, b);
+        s->enabled_bitmaps = g_slist_prepend(s->enabled_bitmaps, b);
     }
 
     return 0;
@@ -489,11 +504,12 @@  static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s)
 
 void dirty_bitmap_mig_before_vm_start(void)
 {
+    DBMLoadState *s = &dbm_state.load;
     GSList *item;
 
-    qemu_mutex_lock(&finish_lock);
+    qemu_mutex_lock(&s->finish_lock);
 
-    for (item = enabled_bitmaps; item; item = g_slist_next(item)) {
+    for (item = s->enabled_bitmaps; item; item = g_slist_next(item)) {
         LoadBitmapState *b = item->data;
 
         if (b->migrated) {
@@ -505,10 +521,10 @@  void dirty_bitmap_mig_before_vm_start(void)
         g_free(b);
     }
 
-    g_slist_free(enabled_bitmaps);
-    enabled_bitmaps = NULL;
+    g_slist_free(s->enabled_bitmaps);
+    s->enabled_bitmaps = NULL;
 
-    qemu_mutex_unlock(&finish_lock);
+    qemu_mutex_unlock(&s->finish_lock);
 }
 
 static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s)
@@ -517,9 +533,9 @@  static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s)
     trace_dirty_bitmap_load_complete();
     bdrv_dirty_bitmap_deserialize_finish(s->bitmap);
 
-    qemu_mutex_lock(&finish_lock);
+    qemu_mutex_lock(&s->finish_lock);
 
-    for (item = enabled_bitmaps; item; item = g_slist_next(item)) {
+    for (item = s->enabled_bitmaps; item; item = g_slist_next(item)) {
         LoadBitmapState *b = item->data;
 
         if (b->bitmap == s->bitmap) {
@@ -530,7 +546,7 @@  static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s)
 
     if (bdrv_dirty_bitmap_has_successor(s->bitmap)) {
         bdrv_dirty_bitmap_lock(s->bitmap);
-        if (enabled_bitmaps == NULL) {
+        if (s->enabled_bitmaps == NULL) {
             /* in postcopy */
             bdrv_reclaim_dirty_bitmap_locked(s->bitmap, &error_abort);
             bdrv_enable_dirty_bitmap_locked(s->bitmap);
@@ -549,7 +565,7 @@  static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s)
         bdrv_dirty_bitmap_unlock(s->bitmap);
     }
 
-    qemu_mutex_unlock(&finish_lock);
+    qemu_mutex_unlock(&s->finish_lock);
 }
 
 static int dirty_bitmap_load_bits(QEMUFile *f, DBMLoadState *s)
@@ -646,7 +662,7 @@  static int dirty_bitmap_load_header(QEMUFile *f, DBMLoadState *s)
 
 static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
 {
-    static DBMLoadState s;
+    DBMLoadState *s = &((DBMState *)opaque)->load;
     int ret = 0;
 
     trace_dirty_bitmap_load_enter();
@@ -656,17 +672,17 @@  static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
     }
 
     do {
-        ret = dirty_bitmap_load_header(f, &s);
+        ret = dirty_bitmap_load_header(f, s);
         if (ret < 0) {
             return ret;
         }
 
-        if (s.flags & DIRTY_BITMAP_MIG_FLAG_START) {
-            ret = dirty_bitmap_load_start(f, &s);
-        } else if (s.flags & DIRTY_BITMAP_MIG_FLAG_COMPLETE) {
-            dirty_bitmap_load_complete(f, &s);
-        } else if (s.flags & DIRTY_BITMAP_MIG_FLAG_BITS) {
-            ret = dirty_bitmap_load_bits(f, &s);
+        if (s->flags & DIRTY_BITMAP_MIG_FLAG_START) {
+            ret = dirty_bitmap_load_start(f, s);
+        } else if (s->flags & DIRTY_BITMAP_MIG_FLAG_COMPLETE) {
+            dirty_bitmap_load_complete(f, s);
+        } else if (s->flags & DIRTY_BITMAP_MIG_FLAG_BITS) {
+            ret = dirty_bitmap_load_bits(f, s);
         }
 
         if (!ret) {
@@ -676,7 +692,7 @@  static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
         if (ret) {
             return ret;
         }
-    } while (!(s.flags & DIRTY_BITMAP_MIG_FLAG_EOS));
+    } while (!(s->flags & DIRTY_BITMAP_MIG_FLAG_EOS));
 
     trace_dirty_bitmap_load_success();
     return 0;
@@ -684,13 +700,14 @@  static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
 
 static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque)
 {
+    DBMSaveState *s = &((DBMState *)opaque)->save;
     SaveBitmapState *dbms = NULL;
-    if (init_dirty_bitmap_migration() < 0) {
+    if (init_dirty_bitmap_migration(s) < 0) {
         return -1;
     }
 
-    QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
-        send_bitmap_start(f, dbms);
+    QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
+        send_bitmap_start(f, s, dbms);
     }
     qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS);
 
@@ -699,7 +716,9 @@  static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque)
 
 static bool dirty_bitmap_is_active(void *opaque)
 {
-    return migrate_dirty_bitmaps() && !dirty_bitmap_mig_state.no_bitmaps;
+    DBMSaveState *s = &((DBMState *)opaque)->save;
+
+    return migrate_dirty_bitmaps() && !s->no_bitmaps;
 }
 
 static bool dirty_bitmap_is_active_iterate(void *opaque)
@@ -727,10 +746,10 @@  static SaveVMHandlers savevm_dirty_bitmap_handlers = {
 
 void dirty_bitmap_mig_init(void)
 {
-    QSIMPLEQ_INIT(&dirty_bitmap_mig_state.dbms_list);
-    qemu_mutex_init(&finish_lock);
+    QSIMPLEQ_INIT(&dbm_state.save.dbms_list);
+    qemu_mutex_init(&dbm_state.load.finish_lock);
 
     register_savevm_live("dirty-bitmap", 0, 1,
                          &savevm_dirty_bitmap_handlers,
-                         &dirty_bitmap_mig_state);
+                         &dbm_state);
 }