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

[Peter Maydell]

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
-- PMM