[PATCH v1] tests/qtest/migration: Fix slow test dirty_limit

[PATCH v1] tests/qtest/migration: Fix slow test dirty_limit

[Fabiano Rosas]

After the referenced commit, the incoming side doesn't exit
automatically after a failure. Tests that expect the destination to
fail should use -incoming defer, issue QMP migrate-incoming, wait for
the failure event and issue QMP quit.

Fix the dirty_limit test which wasn't updated properly.

Fixes: 4e8c4dda97 ("tests/qtest/migration: Force exit-on-error=false")
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/migration/precopy-tests.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c
index c6c8ae3004..a0e3ff0547 100644
--- a/tests/qtest/migration/precopy-tests.c
+++ b/tests/qtest/migration/precopy-tests.c
@@ -1070,11 +1070,10 @@ static void test_dirty_limit(char *name, MigrateCommon *args)
     args->start.hide_stderr = true;
     args->start.use_dirty_ring = true;
 
-    args->listen_uri = uri;
     args->connect_uri = uri;
 
     /* Start src, dst vm */
-    if (migrate_start(&from, &to, args->listen_uri, &args->start)) {
+    if (migrate_start(&from, &to, "defer", &args->start)) {
         return;
     }
 
@@ -1082,6 +1081,7 @@ static void test_dirty_limit(char *name, MigrateCommon *args)
     migrate_dirty_limit_wait_showup(from, dirtylimit_period, dirtylimit_value);
 
     /* Start migrate */
+    migrate_incoming_qmp(to, args->connect_uri, NULL, "{}");
     migrate_qmp(from, to, args->connect_uri, NULL, "{}");
 
     /* Wait for dirty limit throttle begin */
-- 
2.51.0

Re: [PATCH v1] tests/qtest/migration: Fix slow test dirty_limit

[Peter Xu]

On Thu, Mar 26, 2026 at 01:44:05PM -0300, Fabiano Rosas wrote:
> After the referenced commit, the incoming side doesn't exit
> automatically after a failure. Tests that expect the destination to
> fail should use -incoming defer, issue QMP migrate-incoming, wait for
> the failure event and issue QMP quit.
> 
> Fix the dirty_limit test which wasn't updated properly.
> 
> Fixes: 4e8c4dda97 ("tests/qtest/migration: Force exit-on-error=false")
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

Highly likely I missed something, but could you remind me what is the
problem before the change, and why "defer" fixes it?

The current failure path in this test is:

    /* Now cancel migrate and wait for dirty limit throttle switch off */
    migrate_cancel(from);
    wait_for_migration_status(from, "cancelled", NULL);

    /* destination always fails after cancel */
    migration_event_wait(to, "failed");
    qtest_set_expected_status(to, EXIT_FAILURE);
    qtest_quit(to);

It waits for "failed", so IIUC after switching to exit-on-error it'll make
sure things got gracefully shutdown before qtest_quit().  So I'm a bit lost
on what we're fixing.

PS: a nitpick is if we could always attach a "Link:" to the commit message,
it might help a bit on knowing the original report too.

> ---
>  tests/qtest/migration/precopy-tests.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c
> index c6c8ae3004..a0e3ff0547 100644
> --- a/tests/qtest/migration/precopy-tests.c
> +++ b/tests/qtest/migration/precopy-tests.c
> @@ -1070,11 +1070,10 @@ static void test_dirty_limit(char *name, MigrateCommon *args)
>      args->start.hide_stderr = true;
>      args->start.use_dirty_ring = true;
>  
> -    args->listen_uri = uri;
>      args->connect_uri = uri;
>  
>      /* Start src, dst vm */
> -    if (migrate_start(&from, &to, args->listen_uri, &args->start)) {
> +    if (migrate_start(&from, &to, "defer", &args->start)) {
>          return;
>      }
>  
> @@ -1082,6 +1081,7 @@ static void test_dirty_limit(char *name, MigrateCommon *args)
>      migrate_dirty_limit_wait_showup(from, dirtylimit_period, dirtylimit_value);
>  
>      /* Start migrate */
> +    migrate_incoming_qmp(to, args->connect_uri, NULL, "{}");
>      migrate_qmp(from, to, args->connect_uri, NULL, "{}");
>  
>      /* Wait for dirty limit throttle begin */
> -- 
> 2.51.0
> 

-- 
Peter Xu

Re: [PATCH v1] tests/qtest/migration: Fix slow test dirty_limit

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> On Thu, Mar 26, 2026 at 01:44:05PM -0300, Fabiano Rosas wrote:
>> After the referenced commit, the incoming side doesn't exit
>> automatically after a failure. Tests that expect the destination to
>> fail should use -incoming defer, issue QMP migrate-incoming, wait for
>> the failure event and issue QMP quit.
>> 
>> Fix the dirty_limit test which wasn't updated properly.
>> 
>> Fixes: 4e8c4dda97 ("tests/qtest/migration: Force exit-on-error=false")
>> Reported-by: Thomas Huth <thuth@redhat.com>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>
> Highly likely I missed something, but could you remind me what is the
> problem before the change, and why "defer" fixes it?
>

The original issue was that on failure the incoming side exits abruptly
and leaves resources in the io/channel.c and io/task.c without freeing
and that triggers ASAN (when in use). The fix was to set
exit-on-error=false always. -incoming defer is needed because -incoming
uri has no means of setting the exit-on-error flag, we need an explicit
call to migrate-incoming to be able to set the flag.

The dirty_limit test was not updated properly because it's under g_slow
and I forgot to test it. I removed the qtest_set_expected_status(to,
EXIT_FAILURE) line, but didn't make sure exit-on-error is set to false
for this test. So it's now aborting with "expected 0, got 1".

> The current failure path in this test is:
>
>     /* Now cancel migrate and wait for dirty limit throttle switch off */
>     migrate_cancel(from);
>     wait_for_migration_status(from, "cancelled", NULL);
>
>     /* destination always fails after cancel */
>     migration_event_wait(to, "failed");
>     qtest_set_expected_status(to, EXIT_FAILURE);
>     qtest_quit(to);
>
> It waits for "failed", so IIUC after switching to exit-on-error it'll make
> sure things got gracefully shutdown before qtest_quit().  So I'm a bit lost
> on what we're fixing.
>
> PS: a nitpick is if we could always attach a "Link:" to the commit message,
> it might help a bit on knowing the original report too.
>
>> ---
>>  tests/qtest/migration/precopy-tests.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c
>> index c6c8ae3004..a0e3ff0547 100644
>> --- a/tests/qtest/migration/precopy-tests.c
>> +++ b/tests/qtest/migration/precopy-tests.c
>> @@ -1070,11 +1070,10 @@ static void test_dirty_limit(char *name, MigrateCommon *args)
>>      args->start.hide_stderr = true;
>>      args->start.use_dirty_ring = true;
>>  
>> -    args->listen_uri = uri;
>>      args->connect_uri = uri;
>>  
>>      /* Start src, dst vm */
>> -    if (migrate_start(&from, &to, args->listen_uri, &args->start)) {
>> +    if (migrate_start(&from, &to, "defer", &args->start)) {
>>          return;
>>      }
>>  
>> @@ -1082,6 +1081,7 @@ static void test_dirty_limit(char *name, MigrateCommon *args)
>>      migrate_dirty_limit_wait_showup(from, dirtylimit_period, dirtylimit_value);
>>  
>>      /* Start migrate */
>> +    migrate_incoming_qmp(to, args->connect_uri, NULL, "{}");
>>      migrate_qmp(from, to, args->connect_uri, NULL, "{}");
>>  
>>      /* Wait for dirty limit throttle begin */
>> -- 
>> 2.51.0
>> 

Re: [PATCH v1] tests/qtest/migration: Fix slow test dirty_limit

[Peter Xu]

On Thu, Mar 26, 2026 at 06:13:14PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Thu, Mar 26, 2026 at 01:44:05PM -0300, Fabiano Rosas wrote:
> >> After the referenced commit, the incoming side doesn't exit
> >> automatically after a failure. Tests that expect the destination to
> >> fail should use -incoming defer, issue QMP migrate-incoming, wait for
> >> the failure event and issue QMP quit.
> >> 
> >> Fix the dirty_limit test which wasn't updated properly.
> >> 
> >> Fixes: 4e8c4dda97 ("tests/qtest/migration: Force exit-on-error=false")
> >> Reported-by: Thomas Huth <thuth@redhat.com>
> >> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> >
> > Highly likely I missed something, but could you remind me what is the
> > problem before the change, and why "defer" fixes it?
> >
> 
> The original issue was that on failure the incoming side exits abruptly
> and leaves resources in the io/channel.c and io/task.c without freeing
> and that triggers ASAN (when in use). The fix was to set
> exit-on-error=false always. -incoming defer is needed because -incoming
> uri has no means of setting the exit-on-error flag, we need an explicit
> call to migrate-incoming to be able to set the flag.
> 
> The dirty_limit test was not updated properly because it's under g_slow
> and I forgot to test it. I removed the qtest_set_expected_status(to,
> EXIT_FAILURE) line, but didn't make sure exit-on-error is set to false
> for this test. So it's now aborting with "expected 0, got 1".

Mistery resolved after I git pull looking at the new code..

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

-- 
Peter Xu