Re: [PULL 21/40] migration: per-mode blockers

[Steven Sistare <steven.sistare@oracle.com>]

On 11/9/2023 12:10 PM, Peter Maydell wrote:
> On Thu, 2 Nov 2023 at 11:43, Juan Quintela <quintela@redhat.com> wrote:
>>
>> From: Steve Sistare <steven.sistare@oracle.com>
>>
>> Extend the blocker interface so that a blocker can be registered for
>> one or more migration modes.  The existing interfaces register a
>> blocker for all modes, and the new interfaces take a varargs list
>> of modes.
>>
>> Internally, maintain a separate blocker list per mode.  The same Error
>> object may be added to multiple lists.  When a block is deleted, it is
>> removed from every list, and the Error is freed.
>>
>> No functional change until a new mode is added.
> 
> Hi; Coverity worries about this code:
> 
>> -static GSList *migration_blockers;
>> +static GSList *migration_blockers[MIG_MODE__MAX];
>>
>>  static bool migration_object_check(MigrationState *ms, Error **errp);
>>  static int migration_maybe_pause(MigrationState *s,
>> @@ -1043,7 +1043,7 @@ static void fill_source_migration_info(MigrationInfo *info)
>>  {
>>      MigrationState *s = migrate_get_current();
>>      int state = qatomic_read(&s->state);
>> -    GSList *cur_blocker = migration_blockers;
>> +    GSList *cur_blocker = migration_blockers[migrate_mode()];
> 
> because it thinks that migrate_mode() might return a value that's
> too big for the migration_blockers[] array. (CID 1523829, 1523830.)
> 
> I think Coverity complains mostly because it doesn't understand
> that the MIG_MODE__MAX in the enum is not a valid enum value
> that a function returning a MigMode might return. But we can
> help it out by assert()ing in migrate_mode() that the value
> we're about to return is definitely a valid mode.

Thanks Peter, I will submit a fix with suggested-by, unless you want to.
I'll look at all uses of migration_blocker[].
Would coverity be happier if I also increase the size of the array?

- Steve