1. Patchew
  2. QEMU
  3. View series

[PATCH] tests/qtest: failover: fix infinite loop

Laurent Vivier posted 1 patch 3 years, 10 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220329124259.355995-1-lvivier@redhat.com
Maintainers: Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
tests/qtest/virtio-net-failover.c | 29 +++++++++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)
[PATCH] tests/qtest: failover: fix infinite loop
Posted by Laurent Vivier 3 years, 10 months ago 
If the migration is over before we cancel it, we are
waiting in a loop a state that never comes because the state
is already "completed".

To avoid an infinite loop, skip the test if the migration
is "completed" before we were able to cancel it.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 tests/qtest/virtio-net-failover.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c
index 80292eecf65f..78811f1c9216 100644
--- a/tests/qtest/virtio-net-failover.c
+++ b/tests/qtest/virtio-net-failover.c
@@ -1141,6 +1141,11 @@ static void test_migrate_guest_off_abort(gconstpointer opaque)
         ret = migrate_status(qts);
 
         status = qdict_get_str(ret, "status");
+        if (strcmp(status, "completed") == 0) {
+            g_test_skip("Failed to cancel the migration");
+            qobject_unref(ret);
+            goto out;
+        }
         if (strcmp(status, "active") == 0) {
             qobject_unref(ret);
             break;
@@ -1155,8 +1160,12 @@ static void test_migrate_guest_off_abort(gconstpointer opaque)
 
     while (true) {
         ret = migrate_status(qts);
-
         status = qdict_get_str(ret, "status");
+        if (strcmp(status, "completed") == 0) {
+            g_test_skip("Failed to cancel the migration");
+            qobject_unref(ret);
+            goto out;
+        }
         if (strcmp(status, "cancelled") == 0) {
             qobject_unref(ret);
             break;
@@ -1169,6 +1178,7 @@ static void test_migrate_guest_off_abort(gconstpointer opaque)
     check_one_card(qts, true, "standby0", MAC_STANDBY0);
     check_one_card(qts, false, "primary0", MAC_PRIMARY0);
 
+out:
     qos_object_destroy((QOSGraphObject *)vdev);
     machine_stop(qts);
 }
@@ -1251,8 +1261,7 @@ static void test_migrate_abort_wait_unplug(gconstpointer opaque)
             qobject_unref(ret);
             break;
         }
-        g_assert_cmpstr(status, !=, "failed");
-        g_assert_cmpstr(status, !=, "active");
+        g_assert_cmpstr(status, ==, "cancelling");
         qobject_unref(ret);
     }
 
@@ -1324,11 +1333,11 @@ static void test_migrate_abort_active(gconstpointer opaque)
         ret = migrate_status(qts);
 
         status = qdict_get_str(ret, "status");
+        g_assert_cmpstr(status, !=, "failed");
         if (strcmp(status, "wait-unplug") != 0) {
             qobject_unref(ret);
             break;
         }
-        g_assert_cmpstr(status, !=, "failed");
         qobject_unref(ret);
     }
 
@@ -1340,6 +1349,11 @@ static void test_migrate_abort_active(gconstpointer opaque)
         ret = migrate_status(qts);
 
         status = qdict_get_str(ret, "status");
+        if (strcmp(status, "completed") == 0) {
+            g_test_skip("Failed to cancel the migration");
+            qobject_unref(ret);
+            goto out;
+        }
         if (strcmp(status, "cancelled") == 0) {
             qobject_unref(ret);
             break;
@@ -1352,6 +1366,7 @@ static void test_migrate_abort_active(gconstpointer opaque)
     check_one_card(qts, true, "standby0", MAC_STANDBY0);
     check_one_card(qts, true, "primary0", MAC_PRIMARY0);
 
+out:
     qos_object_destroy((QOSGraphObject *)vdev);
     machine_stop(qts);
 }
@@ -1425,6 +1440,11 @@ static void test_migrate_off_abort(gconstpointer opaque)
         ret = migrate_status(qts);
 
         status = qdict_get_str(ret, "status");
+        if (strcmp(status, "completed") == 0) {
+            g_test_skip("Failed to cancel the migration");
+            qobject_unref(ret);
+            goto out;
+        }
         if (strcmp(status, "cancelled") == 0) {
             qobject_unref(ret);
             break;
@@ -1437,6 +1457,7 @@ static void test_migrate_off_abort(gconstpointer opaque)
     check_one_card(qts, true, "standby0", MAC_STANDBY0);
     check_one_card(qts, true, "primary0", MAC_PRIMARY0);
 
+out:
     qos_object_destroy((QOSGraphObject *)vdev);
     machine_stop(qts);
 }
-- 
2.35.1
Re: [PATCH] tests/qtest: failover: fix infinite loop
Posted by Peter Maydell 3 years, 10 months ago 
On Tue, 29 Mar 2022 at 13:43, Laurent Vivier <lvivier@redhat.com> wrote:
>
> If the migration is over before we cancel it, we are
> waiting in a loop a state that never comes because the state
> is already "completed".
>
> To avoid an infinite loop, skip the test if the migration
> is "completed" before we were able to cancel it.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  tests/qtest/virtio-net-failover.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)

Applied to master, thanks.

-- PMM
Re: [PATCH] tests/qtest: failover: fix infinite loop
Posted by Thomas Huth 3 years, 10 months ago 
On 29/03/2022 14.42, Laurent Vivier wrote:
> If the migration is over before we cancel it, we are
> waiting in a loop a state that never comes because the state
> is already "completed".
> 
> To avoid an infinite loop, skip the test if the migration
> is "completed" before we were able to cancel it.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   tests/qtest/virtio-net-failover.c | 29 +++++++++++++++++++++++++----
>   1 file changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c
> index 80292eecf65f..78811f1c9216 100644
> --- a/tests/qtest/virtio-net-failover.c
> +++ b/tests/qtest/virtio-net-failover.c
> @@ -1141,6 +1141,11 @@ static void test_migrate_guest_off_abort(gconstpointer opaque)
>           ret = migrate_status(qts);
>   
>           status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "completed") == 0) {
> +            g_test_skip("Failed to cancel the migration");
> +            qobject_unref(ret);
> +            goto out;
> +        }
>           if (strcmp(status, "active") == 0) {
>               qobject_unref(ret);
>               break;
> @@ -1155,8 +1160,12 @@ static void test_migrate_guest_off_abort(gconstpointer opaque)
>   
>       while (true) {
>           ret = migrate_status(qts);
> -
>           status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "completed") == 0) {
> +            g_test_skip("Failed to cancel the migration");
> +            qobject_unref(ret);
> +            goto out;
> +        }
>           if (strcmp(status, "cancelled") == 0) {
>               qobject_unref(ret);
>               break;
> @@ -1169,6 +1178,7 @@ static void test_migrate_guest_off_abort(gconstpointer opaque)
>       check_one_card(qts, true, "standby0", MAC_STANDBY0);
>       check_one_card(qts, false, "primary0", MAC_PRIMARY0);
>   
> +out:
>       qos_object_destroy((QOSGraphObject *)vdev);
>       machine_stop(qts);
>   }
> @@ -1251,8 +1261,7 @@ static void test_migrate_abort_wait_unplug(gconstpointer opaque)
>               qobject_unref(ret);
>               break;
>           }
> -        g_assert_cmpstr(status, !=, "failed");
> -        g_assert_cmpstr(status, !=, "active");
> +        g_assert_cmpstr(status, ==, "cancelling");
>           qobject_unref(ret);
>       }
>   
> @@ -1324,11 +1333,11 @@ static void test_migrate_abort_active(gconstpointer opaque)
>           ret = migrate_status(qts);
>   
>           status = qdict_get_str(ret, "status");
> +        g_assert_cmpstr(status, !=, "failed");
>           if (strcmp(status, "wait-unplug") != 0) {
>               qobject_unref(ret);
>               break;
>           }
> -        g_assert_cmpstr(status, !=, "failed");
>           qobject_unref(ret);
>       }
>   
> @@ -1340,6 +1349,11 @@ static void test_migrate_abort_active(gconstpointer opaque)
>           ret = migrate_status(qts);
>   
>           status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "completed") == 0) {
> +            g_test_skip("Failed to cancel the migration");
> +            qobject_unref(ret);
> +            goto out;
> +        }
>           if (strcmp(status, "cancelled") == 0) {
>               qobject_unref(ret);
>               break;
> @@ -1352,6 +1366,7 @@ static void test_migrate_abort_active(gconstpointer opaque)
>       check_one_card(qts, true, "standby0", MAC_STANDBY0);
>       check_one_card(qts, true, "primary0", MAC_PRIMARY0);
>   
> +out:
>       qos_object_destroy((QOSGraphObject *)vdev);
>       machine_stop(qts);
>   }
> @@ -1425,6 +1440,11 @@ static void test_migrate_off_abort(gconstpointer opaque)
>           ret = migrate_status(qts);
>   
>           status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "completed") == 0) {
> +            g_test_skip("Failed to cancel the migration");
> +            qobject_unref(ret);
> +            goto out;
> +        }
>           if (strcmp(status, "cancelled") == 0) {
>               qobject_unref(ret);
>               break;
> @@ -1437,6 +1457,7 @@ static void test_migrate_off_abort(gconstpointer opaque)
>       check_one_card(qts, true, "standby0", MAC_STANDBY0);
>       check_one_card(qts, true, "primary0", MAC_PRIMARY0);
>   
> +out:
>       qos_object_destroy((QOSGraphObject *)vdev);
>       machine_stop(qts);
>   }

Acked-by: Thomas Huth <thuth@redhat.com>

Is this still urgent for 7.0, or can it wait for the 7.1 cycle?

  Thomas
Re: [PATCH] tests/qtest: failover: fix infinite loop
Posted by Peter Maydell 3 years, 10 months ago 
On Tue, 29 Mar 2022 at 13:47, Thomas Huth <thuth@redhat.com> wrote:
>
> On 29/03/2022 14.42, Laurent Vivier wrote:
> > If the migration is over before we cancel it, we are
> > waiting in a loop a state that never comes because the state
> > is already "completed".
> >
> > To avoid an infinite loop, skip the test if the migration
> > is "completed" before we were able to cancel it.

> Is this still urgent for 7.0, or can it wait for the 7.1 cycle?

It's a test case change that fixes at least one hang I've seen
in "make check". I prefer those to go in, at least before rc3,
because the CI loop being unreliable makes the whole release
process slower and more annoying.

thanks
-- PMM
Re: [PATCH] tests/qtest: failover: fix infinite loop
Posted by Thomas Huth 3 years, 10 months ago 
On 29/03/2022 15.23, Peter Maydell wrote:
> On Tue, 29 Mar 2022 at 13:47, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 29/03/2022 14.42, Laurent Vivier wrote:
>>> If the migration is over before we cancel it, we are
>>> waiting in a loop a state that never comes because the state
>>> is already "completed".
>>>
>>> To avoid an infinite loop, skip the test if the migration
>>> is "completed" before we were able to cancel it.
> 
>> Is this still urgent for 7.0, or can it wait for the 7.1 cycle?
> 
> It's a test case change that fixes at least one hang I've seen
> in "make check". I prefer those to go in, at least before rc3,
> because the CI loop being unreliable makes the whole release
> process slower and more annoying.

Ok. Do you want to pick it directly, or shall I create a pull request for 
this? (I don't have much else queued right now, that's why I ask)

  Thomas
Re: [PATCH] tests/qtest: failover: fix infinite loop
Posted by Peter Maydell 3 years, 10 months ago 
On Tue, 29 Mar 2022 at 14:25, Thomas Huth <thuth@redhat.com> wrote:
>
> On 29/03/2022 15.23, Peter Maydell wrote:
> > On Tue, 29 Mar 2022 at 13:47, Thomas Huth <thuth@redhat.com> wrote:
> >>
> >> On 29/03/2022 14.42, Laurent Vivier wrote:
> >>> If the migration is over before we cancel it, we are
> >>> waiting in a loop a state that never comes because the state
> >>> is already "completed".
> >>>
> >>> To avoid an infinite loop, skip the test if the migration
> >>> is "completed" before we were able to cancel it.
> >
> >> Is this still urgent for 7.0, or can it wait for the 7.1 cycle?
> >
> > It's a test case change that fixes at least one hang I've seen
> > in "make check". I prefer those to go in, at least before rc3,
> > because the CI loop being unreliable makes the whole release
> > process slower and more annoying.
>
> Ok. Do you want to pick it directly, or shall I create a pull request for
> this? (I don't have much else queued right now, that's why I ask)

I can apply it directly if you don't have anything else you're
sending anyway.

thanks
-- PMM
Re: [PATCH] tests/qtest: failover: fix infinite loop
Posted by Thomas Huth 3 years, 10 months ago 
On 29/03/2022 15.27, Peter Maydell wrote:
> On Tue, 29 Mar 2022 at 14:25, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 29/03/2022 15.23, Peter Maydell wrote:
>>> On Tue, 29 Mar 2022 at 13:47, Thomas Huth <thuth@redhat.com> wrote:
>>>>
>>>> On 29/03/2022 14.42, Laurent Vivier wrote:
>>>>> If the migration is over before we cancel it, we are
>>>>> waiting in a loop a state that never comes because the state
>>>>> is already "completed".
>>>>>
>>>>> To avoid an infinite loop, skip the test if the migration
>>>>> is "completed" before we were able to cancel it.
>>>
>>>> Is this still urgent for 7.0, or can it wait for the 7.1 cycle?
>>>
>>> It's a test case change that fixes at least one hang I've seen
>>> in "make check". I prefer those to go in, at least before rc3,
>>> because the CI loop being unreliable makes the whole release
>>> process slower and more annoying.
>>
>> Ok. Do you want to pick it directly, or shall I create a pull request for
>> this? (I don't have much else queued right now, that's why I ask)
> 
> I can apply it directly if you don't have anything else you're
> sending anyway.

Ok, then please apply directly! Thanks!

  Thomas

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.