From nobody Mon Feb 9 01:20:17 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=virtuozzo.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1581951875811809.8657684283164; Mon, 17 Feb 2020 07:04:35 -0800 (PST) Received: from localhost ([::1]:46726 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1j3hwo-00064h-Eh for importer@patchew.org; Mon, 17 Feb 2020 10:04:34 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:39009) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1j3hvO-00044X-0z for qemu-devel@nongnu.org; Mon, 17 Feb 2020 10:03:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1j3hvK-0007OE-D3 for qemu-devel@nongnu.org; Mon, 17 Feb 2020 10:03:05 -0500 Received: from relay.sw.ru ([185.231.240.75]:47408) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1j3hvK-0007L6-1T; Mon, 17 Feb 2020 10:03:02 -0500 Received: from vovaso.qa.sw.ru ([10.94.3.0] helo=kvm.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.92.3) (envelope-from ) id 1j3hvD-0007Zt-Eo; Mon, 17 Feb 2020 18:02:55 +0300 From: Vladimir Sementsov-Ogievskiy To: qemu-devel@nongnu.org Subject: [PATCH v2 05/22] migration/block-dirty-bitmap: refactor state global variables Date: Mon, 17 Feb 2020 18:02:29 +0300 Message-Id: <20200217150246.29180-6-vsementsov@virtuozzo.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20200217150246.29180-1-vsementsov@virtuozzo.com> References: <20200217150246.29180-1-vsementsov@virtuozzo.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 185.231.240.75 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Fam Zheng , vsementsov@virtuozzo.com, qemu-block@nongnu.org, quintela@redhat.com, dgilbert@redhat.com, Stefan Hajnoczi , andrey.shinkevich@virtuozzo.com, John Snow Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" Move all state variables into one global struct. Reduce global variable usage, utilizing opaque pointer where possible. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Andrey Shinkevich --- 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; =20 +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; =20 -static DBMSaveState dirty_bitmap_mig_state; +typedef struct DBMState { + DBMSaveState save; + DBMLoadState load; +} DBMState; =20 -/* 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; =20 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); } =20 -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 =3D dbms->bs; BdrvDirtyBitmap *bitmap =3D dbms->bitmap; uint32_t flags =3D additional_flags; trace_send_bitmap_header_enter(); =20 - if (bs !=3D dirty_bitmap_mig_state.prev_bs) { - dirty_bitmap_mig_state.prev_bs =3D bs; + if (bs !=3D s->prev_bs) { + s->prev_bs =3D bs; flags |=3D DIRTY_BITMAP_MIG_FLAG_DEVICE_NAME; } =20 - if (bitmap !=3D dirty_bitmap_mig_state.prev_bitmap) { - dirty_bitmap_mig_state.prev_bitmap =3D bitmap; + if (bitmap !=3D s->prev_bitmap) { + s->prev_bitmap =3D bitmap; flags |=3D DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME; } =20 @@ -198,19 +203,22 @@ static void send_bitmap_header(QEMUFile *f, SaveBitma= pState *dbms, } } =20 -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); } =20 -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); } =20 -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, SaveBitmapSta= te *dbms, =20 trace_send_bitmap_bits(flags, start_sector, nr_sectors, buf_size); =20 - send_bitmap_header(f, dbms, flags); + send_bitmap_header(f, s, dbms, flags); =20 qemu_put_be64(f, start_sector); qemu_put_be32(f, nr_sectors); @@ -254,12 +262,12 @@ static void send_bitmap_bits(QEMUFile *f, SaveBitmapS= tate *dbms, } =20 /* Called with iothread lock taken. */ -static void dirty_bitmap_do_save_cleanup(void) +static void dirty_bitmap_do_save_cleanup(DBMSaveState *s) { SaveBitmapState *dbms; =20 - while ((dbms =3D QSIMPLEQ_FIRST(&dirty_bitmap_mig_state.dbms_list)) != =3D NULL) { - QSIMPLEQ_REMOVE_HEAD(&dirty_bitmap_mig_state.dbms_list, entry); + while ((dbms =3D QSIMPLEQ_FIRST(&s->dbms_list)) !=3D 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) } =20 /* 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 =3D NULL; =20 - dirty_bitmap_mig_state.bulk_completed =3D false; - dirty_bitmap_mig_state.prev_bs =3D NULL; - dirty_bitmap_mig_state.prev_bitmap =3D NULL; - dirty_bitmap_mig_state.no_bitmaps =3D false; + s->bulk_completed =3D false; + s->prev_bs =3D NULL; + s->prev_bitmap =3D NULL; + s->no_bitmaps =3D false; =20 for (bs =3D bdrv_next_all_states(NULL); bs; bs =3D bdrv_next_all_state= s(bs)) { const char *name =3D bdrv_get_device_or_node_name(bs); @@ -316,35 +324,36 @@ static int init_dirty_bitmap_migration(void) dbms->flags |=3D DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT; } =20 - QSIMPLEQ_INSERT_TAIL(&dirty_bitmap_mig_state.dbms_list, + QSIMPLEQ_INSERT_TAIL(&s->dbms_list, dbms, entry); } } =20 /* 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); } =20 - if (QSIMPLEQ_EMPTY(&dirty_bitmap_mig_state.dbms_list)) { - dirty_bitmap_mig_state.no_bitmaps =3D true; + if (QSIMPLEQ_EMPTY(&s->dbms_list)) { + s->no_bitmaps =3D true; } =20 return 0; =20 fail: - dirty_bitmap_do_save_cleanup(); + dirty_bitmap_do_save_cleanup(s); =20 return -1; } =20 /* 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 =3D MIN(dbms->total_sectors - dbms->cur_sector, dbms->sectors_per_chunk); =20 - send_bitmap_bits(f, dbms, dbms->cur_sector, nr_sectors); + send_bitmap_bits(f, s, dbms, dbms->cur_sector, nr_sectors); =20 dbms->cur_sector +=3D nr_sectors; if (dbms->cur_sector >=3D dbms->total_sectors) { @@ -353,61 +362,66 @@ static void bulk_phase_send_chunk(QEMUFile *f, SaveBi= tmapState *dbms) } =20 /* 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; =20 - 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; } } } =20 - dirty_bitmap_mig_state.bulk_completed =3D true; + s->bulk_completed =3D true; } =20 /* for SaveVMHandlers */ static void dirty_bitmap_save_cleanup(void *opaque) { - dirty_bitmap_do_save_cleanup(); + DBMSaveState *s =3D &((DBMState *)opaque)->save; + + dirty_bitmap_do_save_cleanup(s); } =20 static int dirty_bitmap_save_iterate(QEMUFile *f, void *opaque) { + DBMSaveState *s =3D &((DBMState *)opaque)->save; + trace_dirty_bitmap_save_iterate(migration_in_postcopy()); =20 - 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); } =20 qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS); =20 - return dirty_bitmap_mig_state.bulk_completed; + return s->bulk_completed; } =20 /* Called with iothread lock taken. */ =20 static int dirty_bitmap_save_complete(QEMUFile *f, void *opaque) { + DBMSaveState *s =3D &((DBMState *)opaque)->save; SaveBitmapState *dbms; trace_dirty_bitmap_save_complete_enter(); =20 - if (!dirty_bitmap_mig_state.bulk_completed) { - bulk_phase(f, false); + if (!s->bulk_completed) { + bulk_phase(f, s, false); } =20 - 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); } =20 qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS); =20 trace_dirty_bitmap_save_complete_finish(); =20 - dirty_bitmap_do_save_cleanup(); + dirty_bitmap_save_cleanup(opaque); return 0; } =20 @@ -417,12 +431,13 @@ static void dirty_bitmap_save_pending(QEMUFile *f, vo= id *opaque, uint64_t *res_compatible, uint64_t *res_postcopy_only) { + DBMSaveState *s =3D &((DBMState *)opaque)->save; SaveBitmapState *dbms; uint64_t pending =3D 0; =20 qemu_mutex_lock_iothread(); =20 - QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) { + QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) { uint64_t gran =3D bdrv_dirty_bitmap_granularity(dbms->bitmap); uint64_t sectors =3D dbms->bulk_completed ? 0 : dbms->total_sectors - dbms->cur_sector; @@ -481,7 +496,7 @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLoad= State *s) b->bs =3D s->bs; b->bitmap =3D s->bitmap; b->migrated =3D false; - enabled_bitmaps =3D g_slist_prepend(enabled_bitmaps, b); + s->enabled_bitmaps =3D g_slist_prepend(s->enabled_bitmaps, b); } =20 return 0; @@ -489,11 +504,12 @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLo= adState *s) =20 void dirty_bitmap_mig_before_vm_start(void) { + DBMLoadState *s =3D &dbm_state.load; GSList *item; =20 - qemu_mutex_lock(&finish_lock); + qemu_mutex_lock(&s->finish_lock); =20 - for (item =3D enabled_bitmaps; item; item =3D g_slist_next(item)) { + for (item =3D s->enabled_bitmaps; item; item =3D g_slist_next(item)) { LoadBitmapState *b =3D item->data; =20 if (b->migrated) { @@ -505,10 +521,10 @@ void dirty_bitmap_mig_before_vm_start(void) g_free(b); } =20 - g_slist_free(enabled_bitmaps); - enabled_bitmaps =3D NULL; + g_slist_free(s->enabled_bitmaps); + s->enabled_bitmaps =3D NULL; =20 - qemu_mutex_unlock(&finish_lock); + qemu_mutex_unlock(&s->finish_lock); } =20 static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s) @@ -517,9 +533,9 @@ static void dirty_bitmap_load_complete(QEMUFile *f, DBM= LoadState *s) trace_dirty_bitmap_load_complete(); bdrv_dirty_bitmap_deserialize_finish(s->bitmap); =20 - qemu_mutex_lock(&finish_lock); + qemu_mutex_lock(&s->finish_lock); =20 - for (item =3D enabled_bitmaps; item; item =3D g_slist_next(item)) { + for (item =3D s->enabled_bitmaps; item; item =3D g_slist_next(item)) { LoadBitmapState *b =3D item->data; =20 if (b->bitmap =3D=3D s->bitmap) { @@ -530,7 +546,7 @@ static void dirty_bitmap_load_complete(QEMUFile *f, DBM= LoadState *s) =20 if (bdrv_dirty_bitmap_has_successor(s->bitmap)) { bdrv_dirty_bitmap_lock(s->bitmap); - if (enabled_bitmaps =3D=3D NULL) { + if (s->enabled_bitmaps =3D=3D 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, DBM= LoadState *s) bdrv_dirty_bitmap_unlock(s->bitmap); } =20 - qemu_mutex_unlock(&finish_lock); + qemu_mutex_unlock(&s->finish_lock); } =20 static int dirty_bitmap_load_bits(QEMUFile *f, DBMLoadState *s) @@ -646,7 +662,7 @@ static int dirty_bitmap_load_header(QEMUFile *f, DBMLoa= dState *s) =20 static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id) { - static DBMLoadState s; + DBMLoadState *s =3D &((DBMState *)opaque)->load; int ret =3D 0; =20 trace_dirty_bitmap_load_enter(); @@ -656,17 +672,17 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaqu= e, int version_id) } =20 do { - ret =3D dirty_bitmap_load_header(f, &s); + ret =3D dirty_bitmap_load_header(f, s); if (ret < 0) { return ret; } =20 - if (s.flags & DIRTY_BITMAP_MIG_FLAG_START) { - ret =3D 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 =3D dirty_bitmap_load_bits(f, &s); + if (s->flags & DIRTY_BITMAP_MIG_FLAG_START) { + ret =3D 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 =3D dirty_bitmap_load_bits(f, s); } =20 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)); =20 trace_dirty_bitmap_load_success(); return 0; @@ -684,13 +700,14 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaqu= e, int version_id) =20 static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque) { + DBMSaveState *s =3D &((DBMState *)opaque)->save; SaveBitmapState *dbms =3D NULL; - if (init_dirty_bitmap_migration() < 0) { + if (init_dirty_bitmap_migration(s) < 0) { return -1; } =20 - 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); =20 @@ -699,7 +716,9 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *o= paque) =20 static bool dirty_bitmap_is_active(void *opaque) { - return migrate_dirty_bitmaps() && !dirty_bitmap_mig_state.no_bitmaps; + DBMSaveState *s =3D &((DBMState *)opaque)->save; + + return migrate_dirty_bitmaps() && !s->no_bitmaps; } =20 static bool dirty_bitmap_is_active_iterate(void *opaque) @@ -727,10 +746,10 @@ static SaveVMHandlers savevm_dirty_bitmap_handlers = =3D { =20 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); =20 register_savevm_live("dirty-bitmap", 0, 1, &savevm_dirty_bitmap_handlers, - &dirty_bitmap_mig_state); + &dbm_state); } --=20 2.21.0