On 04/27/2018 09:22 AM, Peter Maydell wrote: > On 13 March 2018 at 21:14, John Snow <[email protected]> wrote: >> From: Vladimir Sementsov-Ogievskiy <[email protected]> >> >> Postcopy migration of dirty bitmaps. Only named dirty bitmaps are migrated. >> >> If destination qemu is already containing a dirty bitmap with the same name >> as a migrated bitmap (for the same node), then, if their granularities are >> the same the migration will be done, otherwise the error will be generated. >> >> If destination qemu doesn't contain such bitmap it will be created. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]> >> Reviewed-by: Dr. David Alan Gilbert <[email protected]> >> Message-id: [email protected] >> [Changed '+' to '*' as per list discussion. --js] >> Signed-off-by: John Snow <[email protected]> > >> +static int init_dirty_bitmap_migration(void) >> +{ > > Hi; Coverity (CID1390625) complains about a possible dereference > after NULL check in this function: > >> + BlockDriverState *bs; >> + BdrvDirtyBitmap *bitmap; >> + DirtyBitmapMigBitmapState *dbms; >> + BdrvNextIterator it; >> + >> + 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; >> + >> + 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); >> + } > > The 'bs' test in this while() loop implies that we might > leave the loop because bs == NULL... > >> + >> + for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap; > > ...but this call to bdrv_dirty_bitmap_next() will always > dereference bs, so if it's NULL we'll crash. > >> + bitmap = bdrv_dirty_bitmap_next(bs, bitmap)) > > thanks > -- PMM >
I have some patches on the way to clean up this file. Sorry for the delay.
