Re: [PATCH v2 26/39] tests/qtest: migration-test: Make sure QEMU process "to" exited after migration is canceled

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

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

Regards,
Bin

Re: [PATCH v2 26/39] tests/qtest: migration-test: Make sure QEMU process "to" exited after migration is canceled

2022-09-21 Thread Marc-André Lureau 
Hi

On Tue, Sep 20, 2022 at 3:18 PM Bin Meng  wrote:

> From: Xuzhou Cheng 
>
> Make sure QEMU process "to" exited before launching another target
> for migration in the test_multifd_tcp_cancel case.
>
> Signed-off-by: Xuzhou Cheng 
> Signed-off-by: Bin Meng 
> Reviewed-by: Marc-André Lureau 
>

fwiw, I didn't r-b the version with a busy wait
(
https://patchew.org/QEMU/20220824094029.1634519-1-bmeng...@gmail.com/20220824094029.1634519-42-bmeng...@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

Re: [PATCH v2 26/39] tests/qtest: migration-test: Make sure QEMU process "to" exited after migration is canceled

2022-09-21 Thread Daniel P . Berrangé 
On Wed, Sep 21, 2022 at 05:29:55PM +0100, Dr. David Alan Gilbert wrote:
> * Bin Meng (bmeng...@gmail.com) wrote:
> > From: Xuzhou Cheng 
> > 
> > Make sure QEMU process "to" exited before launching another target
> > for migration in the test_multifd_tcp_cancel case.
> > 
> > Signed-off-by: Xuzhou Cheng 
> > Signed-off-by: Bin Meng 
> > Reviewed-by: Marc-André Lureau 
> 
> 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 :|

Re: [PATCH v2 26/39] tests/qtest: migration-test: Make sure QEMU process "to" exited after migration is canceled

2022-09-21 Thread Dr. David Alan Gilbert 
* Bin Meng (bmeng...@gmail.com) wrote:
> From: Xuzhou Cheng 
> 
> Make sure QEMU process "to" exited before launching another target
> for migration in the test_multifd_tcp_cancel case.
> 
> Signed-off-by: Xuzhou Cheng 
> Signed-off-by: Bin Meng 
> Reviewed-by: Marc-André Lureau 

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 / dgilb...@redhat.com / Manchester, UK

[PATCH v2 26/39] tests/qtest: migration-test: Make sure QEMU process "to" exited after migration is canceled

2022-09-20 Thread Bin Meng 
From: Xuzhou Cheng 

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

Signed-off-by: Xuzhou Cheng 
Signed-off-by: Bin Meng 
Reviewed-by: Marc-André Lureau 
---

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