| Message ID | 20180313211441.5179-1-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20180313211441.5179-1-jsnow@redhat.com
Subject: [Qemu-devel] [PULL 00/13] Bitmaps patches
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
t [tag update] patchew/20180312172124.56461-1-dgilbert@redhat.com -> patchew/20180312172124.56461-1-dgilbert@redhat.com
* [new tag] patchew/20180313211441.5179-1-jsnow@redhat.com -> patchew/20180313211441.5179-1-jsnow@redhat.com
Switched to a new branch 'test'
6dbaff4909 iotests: add dirty bitmap postcopy test
970272a769 iotests: add dirty bitmap migration test
0a4cfd0266 migration: add postcopy migration of dirty bitmaps
39fdbb0e78 migration: allow qmp command migrate-start-postcopy for any postcopy
95be37b4df migration: add is_active_iterate handler
ebce5a11c9 migration/qemu-file: add qemu_put_counted_string()
b2e3a3e0f7 migration: include migrate_dirty_bitmaps in migrate_postcopy
d234483b40 qapi: add dirty-bitmaps migration capability
6b24eebd32 migration: introduce postcopy-only pending
a8ca389528 dirty-bitmap: add locked state
48cfc1176c block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap
8bc9e17e62 block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap
ce2cf17b98 block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor()
=== OUTPUT BEGIN ===
Checking PATCH 1/13: block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor()...
Checking PATCH 2/13: block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap...
Checking PATCH 3/13: block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap...
Checking PATCH 4/13: dirty-bitmap: add locked state...
Checking PATCH 5/13: migration: introduce postcopy-only pending...
Checking PATCH 6/13: qapi: add dirty-bitmaps migration capability...
Checking PATCH 7/13: migration: include migrate_dirty_bitmaps in migrate_postcopy...
Checking PATCH 8/13: migration/qemu-file: add qemu_put_counted_string()...
Checking PATCH 9/13: migration: add is_active_iterate handler...
Checking PATCH 10/13: migration: allow qmp command migrate-start-postcopy for any postcopy...
Checking PATCH 11/13: migration: add postcopy migration of dirty bitmaps...
ERROR: braces {} are necessary for all arms of this statement
#740: FILE: migration/block-dirty-bitmap.c:690:
+ } while (!(s.flags & DIRTY_BITMAP_MIG_FLAG_EOS));
[...]
total: 1 errors, 0 warnings, 816 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 12/13: iotests: add dirty bitmap migration test...
Checking PATCH 13/13: iotests: add dirty bitmap postcopy test...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote: > The following changes since commit 026aaf47c02b79036feb830206cfebb2a726510d: > > Merge remote-tracking branch 'remotes/ehabkost/tags/python-next-pull-request' into staging (2018-03-13 16:26:44 +0000) > > are available in the Git repository at: > > https://github.com/jnsnow/qemu.git tags/bitmaps-pull-request > > for you to fetch changes up to ac8bd439bb7b5dffeb5ff8a17317ca2b192044b6: > > iotests: add dirty bitmap postcopy test (2018-03-13 17:06:32 -0400) > > ---------------------------------------------------------------- > > PR touching migration sent with permission (and reviews) from > David Gilbert. > > ---------------------------------------------------------------- Applied, thanks. -- PMM
Just note here too, that this relates to bug in checkpatch, work on fixing is in progress: https://lists.nongnu.org/archive/html/qemu-devel/2018-03/msg04651.html 14.03.2018 00:36, no-reply@patchew.org wrote: > Hi, > > This series seems to have some coding style problems. See output below for > more information: > > Type: series > Message-id: 20180313211441.5179-1-jsnow@redhat.com > Subject: [Qemu-devel] [PULL 00/13] Bitmaps patches > > === TEST SCRIPT BEGIN === > #!/bin/bash > > BASE=base > n=1 > total=$(git log --oneline $BASE.. | wc -l) > failed=0 > > git config --local diff.renamelimit 0 > git config --local diff.renames True > git config --local diff.algorithm histogram > > commits="$(git log --format=%H --reverse $BASE..)" > for c in $commits; do > echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." > if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then > failed=1 > echo > fi > n=$((n+1)) > done > > exit $failed > === TEST SCRIPT END === > > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 > From https://github.com/patchew-project/qemu > t [tag update] patchew/20180312172124.56461-1-dgilbert@redhat.com -> patchew/20180312172124.56461-1-dgilbert@redhat.com > * [new tag] patchew/20180313211441.5179-1-jsnow@redhat.com -> patchew/20180313211441.5179-1-jsnow@redhat.com > Switched to a new branch 'test' > 6dbaff4909 iotests: add dirty bitmap postcopy test > 970272a769 iotests: add dirty bitmap migration test > 0a4cfd0266 migration: add postcopy migration of dirty bitmaps > 39fdbb0e78 migration: allow qmp command migrate-start-postcopy for any postcopy > 95be37b4df migration: add is_active_iterate handler > ebce5a11c9 migration/qemu-file: add qemu_put_counted_string() > b2e3a3e0f7 migration: include migrate_dirty_bitmaps in migrate_postcopy > d234483b40 qapi: add dirty-bitmaps migration capability > 6b24eebd32 migration: introduce postcopy-only pending > a8ca389528 dirty-bitmap: add locked state > 48cfc1176c block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap > 8bc9e17e62 block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap > ce2cf17b98 block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor() > > === OUTPUT BEGIN === > Checking PATCH 1/13: block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor()... > Checking PATCH 2/13: block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap... > Checking PATCH 3/13: block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap... > Checking PATCH 4/13: dirty-bitmap: add locked state... > Checking PATCH 5/13: migration: introduce postcopy-only pending... > Checking PATCH 6/13: qapi: add dirty-bitmaps migration capability... > Checking PATCH 7/13: migration: include migrate_dirty_bitmaps in migrate_postcopy... > Checking PATCH 8/13: migration/qemu-file: add qemu_put_counted_string()... > Checking PATCH 9/13: migration: add is_active_iterate handler... > Checking PATCH 10/13: migration: allow qmp command migrate-start-postcopy for any postcopy... > Checking PATCH 11/13: migration: add postcopy migration of dirty bitmaps... > ERROR: braces {} are necessary for all arms of this statement > #740: FILE: migration/block-dirty-bitmap.c:690: > + } while (!(s.flags & DIRTY_BITMAP_MIG_FLAG_EOS)); > [...] > > total: 1 errors, 0 warnings, 816 lines checked > > Your patch has style problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. > > Checking PATCH 12/13: iotests: add dirty bitmap migration test... > Checking PATCH 13/13: iotests: add dirty bitmap postcopy test... > === OUTPUT END === > > Test command exited with code: 1 > > > --- > Email generated automatically by Patchew [http://patchew.org/]. > Please send your feedback to patchew-devel@freelists.org
On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote: > From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > > 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 <vsementsov@virtuozzo.com> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > Message-id: 20180313180320.339796-12-vsementsov@virtuozzo.com > [Changed '+' to '*' as per list discussion. --js] > Signed-off-by: John Snow <jsnow@redhat.com> > +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
On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote: > From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > > 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. Hi. Coverity complains (CID1390627) about a resource leak in this function; > +static int dirty_bitmap_load_bits(QEMUFile *f, DirtyBitmapLoadState *s) > +{ > + uint64_t first_byte = qemu_get_be64(f) << BDRV_SECTOR_BITS; > + uint64_t nr_bytes = (uint64_t)qemu_get_be32(f) << BDRV_SECTOR_BITS; > + trace_dirty_bitmap_load_bits_enter(first_byte >> BDRV_SECTOR_BITS, > + nr_bytes >> BDRV_SECTOR_BITS); > + > + if (s->flags & DIRTY_BITMAP_MIG_FLAG_ZEROES) { > + trace_dirty_bitmap_load_bits_zeroes(); > + bdrv_dirty_bitmap_deserialize_zeroes(s->bitmap, first_byte, nr_bytes, > + false); > + } else { > + size_t ret; > + uint8_t *buf; > + uint64_t buf_size = qemu_get_be64(f); > + uint64_t needed_size = > + bdrv_dirty_bitmap_serialization_size(s->bitmap, > + first_byte, nr_bytes); > + > + if (needed_size > buf_size || > + buf_size > QEMU_ALIGN_UP(needed_size, 4 * sizeof(long)) > + /* Here used same alignment as in send_bitmap_bits */ > + ) { > + error_report("Migrated bitmap granularity doesn't " > + "match the destination bitmap '%s' granularity", > + bdrv_dirty_bitmap_name(s->bitmap)); > + return -EINVAL; > + } > + > + buf = g_malloc(buf_size); Here we allocate memory into buf... > + ret = qemu_get_buffer(f, buf, buf_size); > + if (ret != buf_size) { > + error_report("Failed to read bitmap bits"); > + return -EIO; ...but in this error-exit path we do not free it. > + } > + > + bdrv_dirty_bitmap_deserialize_part(s->bitmap, buf, first_byte, nr_bytes, > + false); > + g_free(buf); > + } > + > + return 0; > +} thanks -- PMM
On 04/27/2018 09:22 AM, Peter Maydell wrote: > On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote: >> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> >> 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 <vsementsov@virtuozzo.com> >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >> Message-id: 20180313180320.339796-12-vsementsov@virtuozzo.com >> [Changed '+' to '*' as per list discussion. --js] >> Signed-off-by: John Snow <jsnow@redhat.com> > >> +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.
On 27 April 2018 at 14:22, Peter Maydell <peter.maydell@linaro.org> wrote: > On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote: >> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> >> 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 <vsementsov@virtuozzo.com> >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >> Message-id: 20180313180320.339796-12-vsementsov@virtuozzo.com >> [Changed '+' to '*' as per list discussion. --js] >> Signed-off-by: John Snow <jsnow@redhat.com> > >> +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)) Hi -- just a nudge that Coverity thinks this one is still unfixed. thanks -- PMM
On 06/20/2018 12:43 PM, Peter Maydell wrote: > On 27 April 2018 at 14:22, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote: >>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >>> >>> 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 <vsementsov@virtuozzo.com> >>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >>> Message-id: 20180313180320.339796-12-vsementsov@virtuozzo.com >>> [Changed '+' to '*' as per list discussion. --js] >>> Signed-off-by: John Snow <jsnow@redhat.com> >> >>> +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)) > > Hi -- just a nudge that Coverity thinks this one is still unfixed. > > thanks > -- PMM > Thank you for the reminder, I've been a bit scatter-brained recently.
On 20 June 2018 at 17:58, John Snow <jsnow@redhat.com> wrote: > > > On 06/20/2018 12:43 PM, Peter Maydell wrote: >> On 27 April 2018 at 14:22, Peter Maydell <peter.maydell@linaro.org> wrote: >>> On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote: >>>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >>>> >>>> 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 <vsementsov@virtuozzo.com> >>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >>>> Message-id: 20180313180320.339796-12-vsementsov@virtuozzo.com >>>> [Changed '+' to '*' as per list discussion. --js] >>>> Signed-off-by: John Snow <jsnow@redhat.com> >>> >>>> +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)) >> >> Hi -- just a nudge that Coverity thinks this one is still unfixed. > Thank you for the reminder, I've been a bit scatter-brained recently. Ping? This is still in Coverity's list of unfixed issues. thanks -- PMM
16.10.2018 15:25, Peter Maydell wrote: > On 20 June 2018 at 17:58, John Snow <jsnow@redhat.com> wrote: >> >> On 06/20/2018 12:43 PM, Peter Maydell wrote: >>> On 27 April 2018 at 14:22, Peter Maydell <peter.maydell@linaro.org> wrote: >>>> On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote: >>>>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >>>>> >>>>> 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 <vsementsov@virtuozzo.com> >>>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >>>>> Message-id: 20180313180320.339796-12-vsementsov@virtuozzo.com >>>>> [Changed '+' to '*' as per list discussion. --js] >>>>> Signed-off-by: John Snow <jsnow@redhat.com> >>>>> +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)) >>> Hi -- just a nudge that Coverity thinks this one is still unfixed. >> Thank you for the reminder, I've been a bit scatter-brained recently. > Ping? This is still in Coverity's list of unfixed issues. > > thanks > -- PMM Will send in few seconds, sorry for such a terrible delay :(