[v1] migration: assert that the same migration handler is not being added twice

[PATCH] migration: assert that the same migration handler is not being added twice

Posted by Ani Sinha 1 month ago

Currently the code that adds a migration blocker does not check if the same
blocker already exists. Assert that the migration handler being added has
not been added already.

CC: Markus Armbruster <armbru@redhat.com>
CC: Peter Xu <peterx@redhat.com>
CC: Prasad Pandit <pjp@fedoraproject.org>
Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
 migration/migration.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index a5b0465ed3..840cf0dfe2 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1702,6 +1702,8 @@ static int add_blockers(Error **reasonp, unsigned modes, Error **errp)
 {
     for (MigMode mode = 0; mode < MIG_MODE__MAX; mode++) {
         if (modes & BIT(mode)) {
+            assert(g_slist_index(migration_blockers[mode],
+                                     *reasonp) == -1);
             migration_blockers[mode] = g_slist_prepend(migration_blockers[mode],
                                                        *reasonp);
         }
-- 
2.52.0

Re: [PATCH] migration: assert that the same migration handler is not being added twice

Posted by Markus Armbruster 1 month ago

Ani Sinha <anisinha@redhat.com> writes:

> Currently the code that adds a migration blocker does not check if the same
> blocker already exists. Assert that the migration handler being added has
> not been added already.
>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Peter Xu <peterx@redhat.com>
> CC: Prasad Pandit <pjp@fedoraproject.org>
> Signed-off-by: Ani Sinha <anisinha@redhat.com>
> ---
>  migration/migration.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index a5b0465ed3..840cf0dfe2 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1702,6 +1702,8 @@ static int add_blockers(Error **reasonp, unsigned modes, Error **errp)
>  {
>      for (MigMode mode = 0; mode < MIG_MODE__MAX; mode++) {
>          if (modes & BIT(mode)) {
> +            assert(g_slist_index(migration_blockers[mode],
> +                                     *reasonp) == -1);

Indentation is off.

Have we run into such extra additions, or is this just out of caution?

What would happen if such an extra addition went uncaught?

>              migration_blockers[mode] = g_slist_prepend(migration_blockers[mode],
>                                                         *reasonp);
>          }

Re: [PATCH] migration: assert that the same migration handler is not being added twice

Posted by Ani Sinha 1 month ago


> On 10 Mar 2026, at 3:33 PM, Markus Armbruster <armbru@redhat.com> wrote:
> 
> Ani Sinha <anisinha@redhat.com> writes:
> 
>> Currently the code that adds a migration blocker does not check if the same
>> blocker already exists. Assert that the migration handler being added has
>> not been added already.
>> 
>> CC: Markus Armbruster <armbru@redhat.com>
>> CC: Peter Xu <peterx@redhat.com>
>> CC: Prasad Pandit <pjp@fedoraproject.org>
>> Signed-off-by: Ani Sinha <anisinha@redhat.com>
>> ---
>> migration/migration.c | 2 ++
>> 1 file changed, 2 insertions(+)
>> 
>> diff --git a/migration/migration.c b/migration/migration.c
>> index a5b0465ed3..840cf0dfe2 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -1702,6 +1702,8 @@ static int add_blockers(Error **reasonp, unsigned modes, Error **errp)
>> {
>>     for (MigMode mode = 0; mode < MIG_MODE__MAX; mode++) {
>>         if (modes & BIT(mode)) {
>> +            assert(g_slist_index(migration_blockers[mode],
>> +                                     *reasonp) == -1);
> 
> Indentation is off.
> 
> Have we run into such extra additions, or is this just out of caution?

Out of caution I suspect.

> 
> What would happen if such an extra addition went uncaught?
> 
>>             migration_blockers[mode] = g_slist_prepend(migration_blockers[mode],
>>                                                        *reasonp);

I vaguely recall multiple blockers causing some crash.


>>         }

Re: [PATCH] migration: assert that the same migration handler is not being added twice

Posted by Peter Xu 1 month ago

On Sun, Mar 08, 2026 at 03:31:48PM +0530, Ani Sinha wrote:
> Currently the code that adds a migration blocker does not check if the same
> blocker already exists. Assert that the migration handler being added has
> not been added already.
> 
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Peter Xu <peterx@redhat.com>
> CC: Prasad Pandit <pjp@fedoraproject.org>
> Signed-off-by: Ani Sinha <anisinha@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu