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);
}
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);
}
--
2.21.0
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>
--
With the best regards,
Andrey Shinkevich
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>
--
Best regards,
Vladimir
© 2016 - 2026 Red Hat, Inc.