Re: [PATCH 3/3] tests/qtest: Re-enable multifd cancel test

[Peter Xu]

On Mon, Jan 08, 2024 at 11:26:04AM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Wed, Jun 07, 2023 at 10:27:15AM +0200, Juan Quintela wrote:
> >> Fabiano Rosas <farosas@suse.de> wrote:
> >> > We've found the source of flakiness in this test, so re-enable it.
> >> >
> >> > Signed-off-by: Fabiano Rosas <farosas@suse.de>
> >> > ---
> >> >  tests/qtest/migration-test.c | 10 ++--------
> >> >  1 file changed, 2 insertions(+), 8 deletions(-)
> >> >
> >> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> >> > index b0c355bbd9..800ad23b75 100644
> >> > --- a/tests/qtest/migration-test.c
> >> > +++ b/tests/qtest/migration-test.c
> >> > @@ -2778,14 +2778,8 @@ int main(int argc, char **argv)
> >> >      }
> >> >      qtest_add_func("/migration/multifd/tcp/plain/none",
> >> >                     test_multifd_tcp_none);
> >> > -    /*
> >> > -     * This test is flaky and sometimes fails in CI and otherwise:
> >> > -     * don't run unless user opts in via environment variable.
> >> > -     */
> >> > -    if (getenv("QEMU_TEST_FLAKY_TESTS")) {
> >> > -        qtest_add_func("/migration/multifd/tcp/plain/cancel",
> >> > -                       test_multifd_tcp_cancel);
> >> > -    }
> >> > +    qtest_add_func("/migration/multifd/tcp/plain/cancel",
> >> > +                   test_multifd_tcp_cancel);
> >> >      qtest_add_func("/migration/multifd/tcp/plain/zlib",
> >> >                     test_multifd_tcp_zlib);
> >> >  #ifdef CONFIG_ZSTD
> >> 
> >> Reviewed-by: Juan Quintela <quintela@redhat.com>
> >> 
> >> 
> >> There was another failure with migration test that I will post during
> >> the rest of the day.  It needs both to get it right.
> >
> > This one didn't yet land upstream.  I'm not sure, but maybe Juan was saying
> > about this change:
> >
> >         commit d2026ee117147893f8d80f060cede6d872ecbd7f
> >         Author: Juan Quintela <quintela@trasno.org>
> >         Date:   Wed Apr 26 12:20:36 2023 +0200
> >
> >         multifd: Fix the number of channels ready
> 
> That's not it. It was something in the test itself around the fact that
> we use two sets of: from/to. There was supposed to be a situation where
> we'd start 'to2' while 'to' was still running and that would cause
> issues (possibly with sockets).
> 
> I think what might have happened is that someone merged a fix through
> another tree and Juan didn't notice. I think this is the one:
> 
>   commit f2d063e61ee2026700ab44bef967f663e976bec8
>   Author: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>   Date:   Fri Oct 28 12:57:32 2022 +0800
>   
>       tests/qtest: migration-test: Make sure QEMU process "to" exited after 
> migration is canceled
>       
>       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>
>       Message-Id: <20221028045736.679903-8-bin.meng@windriver.com>
>       Signed-off-by: Thomas Huth <thuth@redhat.com>

Hmm, i see.

> 
> > Fabiano, did you try to reproduce multifd-cancel with current master?  I'm
> > wondering whether this test has already been completely fixed, then maybe
> > we can pick up this patch now.
> 
> Yes, let's merge it. I have kept it enabled during testing of all of the
> recent race conditions we've debugged and haven't seen it fail. Current
> master also looks fine.

It needs a trivial touchup, but then I queued it.

Thanks,

-- 
Peter Xu