From: Xuzhou Cheng <xuzhou.cheng@windriver.com>

Make sure QEMU process "to" exited before launching another target
for migration in the test_multifd_tcp_cancel case.

Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

Changes in v2:
- Change to a busy wait after migration is canceled

 tests/qtest/migration-test.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index c87afad9e8..aedd9ddb72 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2133,6 +2133,10 @@ static void test_multifd_tcp_cancel(void)
     wait_for_migration_pass(from);
 
     migrate_cancel(from);
+    /* Make sure QEMU process "to" exited */
+    while (qtest_probe_child(to)) {
+        ;
+    }
 
     args = (MigrateStart){
         .only_target = true,
-- 
2.34.1

Hi

On Tue, Sep 20, 2022 at 3:18 PM Bin Meng <bmeng.cn@gmail.com> wrote:

> From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>
> Make sure QEMU process "to" exited before launching another target
> for migration in the test_multifd_tcp_cancel case.
>
> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>

fwiw, I didn't r-b the version with a busy wait
(
https://patchew.org/QEMU/20220824094029.1634519-1-bmeng.cn@gmail.com/20220824094029.1634519-42-bmeng.cn@gmail.com/
)

---
>
> Changes in v2:
> - Change to a busy wait after migration is canceled
>
>  tests/qtest/migration-test.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index c87afad9e8..aedd9ddb72 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -2133,6 +2133,10 @@ static void test_multifd_tcp_cancel(void)
>      wait_for_migration_pass(from);
>
>      migrate_cancel(from);
> +    /* Make sure QEMU process "to" exited */
> +    while (qtest_probe_child(to)) {
> +        ;
> +    }
>
>      args = (MigrateStart){
>          .only_target = true,
> --
> 2.34.1
>
>
>

-- 
Marc-André Lureau

On Thu, Sep 22, 2022 at 5:54 AM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Tue, Sep 20, 2022 at 3:18 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>>
>> From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>>
>> Make sure QEMU process "to" exited before launching another target
>> for migration in the test_multifd_tcp_cancel case.
>>
>> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
>
> fwiw, I didn't r-b the version with a busy wait
> (https://patchew.org/QEMU/20220824094029.1634519-1-bmeng.cn@gmail.com/20220824094029.1634519-42-bmeng.cn@gmail.com/)
>

My mistake. The R-B tag was added before I changed the implementation
and I forgot to remove the tag.

Regards,
Bin

* Bin Meng (bmeng.cn@gmail.com) wrote:
> From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> 
> Make sure QEMU process "to" exited before launching another target
> for migration in the test_multifd_tcp_cancel case.
> 
> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Hmm you might want to put a small usleep in that loop; otherwise
it'll burn CPU.

There is a slim risk with this that another, entirely unrelated, process 
will start up with the same PID between the end of migrate_cancel
and then you'll be spinning on it rather than the 'to' qemu.

I wonder if there's a better way to check for it dieing; e.g. an error
on it's qmp interface or something?

Dave

> ---
> 
> Changes in v2:
> - Change to a busy wait after migration is canceled
> 
>  tests/qtest/migration-test.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index c87afad9e8..aedd9ddb72 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -2133,6 +2133,10 @@ static void test_multifd_tcp_cancel(void)
>      wait_for_migration_pass(from);
>  
>      migrate_cancel(from);
> +    /* Make sure QEMU process "to" exited */
> +    while (qtest_probe_child(to)) {
> +        ;
> +    }
>  
>      args = (MigrateStart){
>          .only_target = true,
> -- 
> 2.34.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

On Wed, Sep 21, 2022 at 05:29:55PM +0100, Dr. David Alan Gilbert wrote:
> * Bin Meng (bmeng.cn@gmail.com) wrote:
> > From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> > 
> > Make sure QEMU process "to" exited before launching another target
> > for migration in the test_multifd_tcp_cancel case.
> > 
> > Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Hmm you might want to put a small usleep in that loop; otherwise
> it'll burn CPU.
> 
> There is a slim risk with this that another, entirely unrelated, process 
> will start up with the same PID between the end of migrate_cancel
> and then you'll be spinning on it rather than the 'to' qemu.
> 
> I wonder if there's a better way to check for it dieing; e.g. an error
> on it's qmp interface or something?

Both the qtest and qmp sockets should give EOF. So if there's an API
that can call g_poll() on the FD with POLL_HUP event, it would be the
reliable way to detect it, without busy-looping.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|