[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps |
Date: |
Wed, 20 Jun 2018 12:58:10 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
On 06/20/2018 12:43 PM, Peter Maydell wrote:
> On 27 April 2018 at 14:22, Peter Maydell <address@hidden> wrote:
>> On 13 March 2018 at 21:14, John Snow <address@hidden> wrote:
>>> From: Vladimir Sementsov-Ogievskiy <address@hidden>
>>>
>>> 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 <address@hidden>
>>> Reviewed-by: Dr. David Alan Gilbert <address@hidden>
>>> Message-id: address@hidden
>>> [Changed '+' to '*' as per list discussion. --js]
>>> Signed-off-by: John Snow <address@hidden>
>>
>>> +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.