| Message ID | 20220824094029.1634519-42-bmeng.cn@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests/qtest: Enable running qtest on Windows | expand |
* Bin Meng (bmeng.cn@gmail.com) wrote: > From: Xuzhou Cheng <xuzhou.cheng@windriver.com> > > Make sure QEMU process "to" is killed 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> > --- > > 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 125d48d855..18ec079abf 100644 > --- a/tests/qtest/migration-test.c > +++ b/tests/qtest/migration-test.c > @@ -2132,6 +2132,10 @@ static void test_multifd_tcp_cancel(void) > wait_for_migration_pass(from); > > migrate_cancel(from); > + /* Make sure QEMU process "to" is killed */ > + if (qtest_probe_child(to)) { > + qtest_kill_qemu(to); > + } I'm not sure that's safe - what happens if the qemu exits between the probe and kill? Dave > args = (MigrateStart){ > .only_target = true, > -- > 2.34.1 >
Hi On Wed, Aug 24, 2022 at 10:56 PM Dr. David Alan Gilbert <dgilbert@redhat.com> wrote: > * Bin Meng (bmeng.cn@gmail.com) wrote: > > From: Xuzhou Cheng <xuzhou.cheng@windriver.com> > > > > Make sure QEMU process "to" is killed 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> > > --- > > > > 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 125d48d855..18ec079abf 100644 > > --- a/tests/qtest/migration-test.c > > +++ b/tests/qtest/migration-test.c > > @@ -2132,6 +2132,10 @@ static void test_multifd_tcp_cancel(void) > > wait_for_migration_pass(from); > > > > migrate_cancel(from); > > + /* Make sure QEMU process "to" is killed */ > > + if (qtest_probe_child(to)) { > > + qtest_kill_qemu(to); > > + } > > I'm not sure that's safe - what happens if the qemu exits between the > probe and kill? > It looks safe to me, qtest_probe_child() resets the qemu_pid if it already exited. Otherwise, there is a process/handle waiting for waitpid/CloseHandle done in qtest_kill_qemu(). We are missing a CloseHandle() in qtest_probe_child() though, I'll send a patch. so lgtm, Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
On Thu, Sep 1, 2022 at 7:35 PM Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > > Hi > > On Wed, Aug 24, 2022 at 10:56 PM Dr. David Alan Gilbert <dgilbert@redhat.com> wrote: >> >> * Bin Meng (bmeng.cn@gmail.com) wrote: >> > From: Xuzhou Cheng <xuzhou.cheng@windriver.com> >> > >> > Make sure QEMU process "to" is killed 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> >> > --- >> > >> > 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 125d48d855..18ec079abf 100644 >> > --- a/tests/qtest/migration-test.c >> > +++ b/tests/qtest/migration-test.c >> > @@ -2132,6 +2132,10 @@ static void test_multifd_tcp_cancel(void) >> > wait_for_migration_pass(from); >> > >> > migrate_cancel(from); >> > + /* Make sure QEMU process "to" is killed */ >> > + if (qtest_probe_child(to)) { >> > + qtest_kill_qemu(to); >> > + } >> >> I'm not sure that's safe - what happens if the qemu exits between the >> probe and kill? > Umm, indeed there will be an issue if qemu exists between the probe and kill. I will change to a busy wait in v2. > > It looks safe to me, qtest_probe_child() resets the qemu_pid if it already exited. Otherwise, there is a process/handle waiting for waitpid/CloseHandle done in qtest_kill_qemu(). > > We are missing a CloseHandle() in qtest_probe_child() though, I'll send a patch. > > so lgtm, > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Regards, Bin
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index 125d48d855..18ec079abf 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -2132,6 +2132,10 @@ static void test_multifd_tcp_cancel(void) wait_for_migration_pass(from); migrate_cancel(from); + /* Make sure QEMU process "to" is killed */ + if (qtest_probe_child(to)) { + qtest_kill_qemu(to); + } args = (MigrateStart){ .only_target = true,