[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] migration/dirty-bitmaps: change bitmap enumeration method
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH v2] migration/dirty-bitmaps: change bitmap enumeration method |
Date: |
Fri, 6 Dec 2019 22:31:11 +0000 |
14.05.2019 23:19, John Snow wrote:
> Shift from looking at every root BDS to *every* BDS. This will migrate
> bitmaps that are attached to blockdev created nodes instead of just ones
> attached to emulated storage devices.
>
> Note that this will not migrate anonymous or internal-use bitmaps, as
> those are defined as having no name.
>
> This will also fix the Coverity issues Peter Maydell has been asking
> about for the past several releases, as well as fixing a real bug.
>
> Reported-by: Peter Maydell <address@hidden>
> Reported-by: Coverity 😅
What was the coverity number (I don't believe that it was smile:)?
Do someone know, that this patch fixes very-very-very terrible bug?
Before this patch, here were bdrv_next-based loop, with exists from it,
but not using bdrv_next_cleanup(). This leads to leaked (incremented) refcnt of
bds on any failure during this loop!
Now we faced this bug, in Rhel-based Qemu, so I strongly recommend to fix it in
Rhel.
> Reported-by: aihua liang <address@hidden>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1652490
> Fixes: Coverity CID 1390625
> CC: Stefan Hajnoczi <address@hidden>
> Signed-off-by: John Snow <address@hidden>
> ---
> migration/block-dirty-bitmap.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
> index d1bb863cb6..4a896a09eb 100644
> --- a/migration/block-dirty-bitmap.c
> +++ b/migration/block-dirty-bitmap.c
> @@ -273,7 +273,6 @@ static int init_dirty_bitmap_migration(void)
> BlockDriverState *bs;
> BdrvDirtyBitmap *bitmap;
> DirtyBitmapMigBitmapState *dbms;
> - BdrvNextIterator it;
> Error *local_err = NULL;
>
> dirty_bitmap_mig_state.bulk_completed = false;
> @@ -281,13 +280,8 @@ static int init_dirty_bitmap_migration(void)
> dirty_bitmap_mig_state.prev_bitmap = NULL;
> dirty_bitmap_mig_state.no_bitmaps = false;
>
> - for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
> - const char *drive_name = bdrv_get_device_or_node_name(bs);
> -
> - /* skip automatically inserted nodes */
> - while (bs && bs->drv && bs->implicit) {
> - bs = backing_bs(bs);
> - }
> + for (bs = bdrv_next_all_states(NULL); bs; bs = bdrv_next_all_states(bs))
> {
> + const char *name = bdrv_get_device_or_node_name(bs);
>
> for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap;
> bitmap = bdrv_dirty_bitmap_next(bs, bitmap))
> @@ -296,7 +290,7 @@ static int init_dirty_bitmap_migration(void)
> continue;
> }
>
> - if (drive_name == NULL) {
> + if (!name || strcmp(name, "") == 0) {
> error_report("Found bitmap '%s' in unnamed node %p. It
> can't "
> "be migrated", bdrv_dirty_bitmap_name(bitmap),
> bs);
> goto fail;
> @@ -313,7 +307,7 @@ static int init_dirty_bitmap_migration(void)
>
> dbms = g_new0(DirtyBitmapMigBitmapState, 1);
> dbms->bs = bs;
> - dbms->node_name = drive_name;
> + dbms->node_name = name;
> dbms->bitmap = bitmap;
> dbms->total_sectors = bdrv_nb_sectors(bs);
> dbms->sectors_per_chunk = CHUNK_SIZE * 8 *
>
--
Best regards,
Vladimir
- Re: [PATCH v2] migration/dirty-bitmaps: change bitmap enumeration method,
Vladimir Sementsov-Ogievskiy <=