Re: [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps

[Vladimir Sementsov-Ogievskiy]

16.10.2018 15:25, Peter Maydell wrote:
> On 20 June 2018 at 17:58, John Snow <address@hidden> wrote:
>>
>> 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.
>> 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 :(

-- 
Best regards,
Vladimir