Re: [PATCH v2 08/10] tests/migration-tests: Always enable migration events

[Peter Xu <peterx@redhat.com>]

On Mon, Jun 17, 2024 at 05:23:24PM -0400, Peter Xu wrote:
> On Mon, Jun 17, 2024 at 04:51:32PM -0300, Fabiano Rosas wrote:
> > Peter Xu <peterx@redhat.com> writes:
> > 
> > > Libvirt should always enable it, so it'll be nice qtest also cover that for
> > > all tests.  Though this patch only enables it, no extra tests are done on
> > > these events yet.
> > >
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > ---
> > >  tests/qtest/migration-test.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > > index 13b59d4c10..9ae8892e26 100644
> > > --- a/tests/qtest/migration-test.c
> > > +++ b/tests/qtest/migration-test.c
> > > @@ -841,6 +841,13 @@ static int test_migrate_start(QTestState **from, QTestState **to,
> > >          unlink(shmem_path);
> > >      }
> > >  
> > > +    /*
> > > +     * Always enable migration events.  Libvirt always uses it, let's try
> > > +     * to mimic as closer as that.
> > > +     */
> > > +    migrate_set_capability(*from, "events", true);
> > > +    migrate_set_capability(*to, "events", true);
> > > +
> > 
> > What do we do with the one at migrate_incoming_qmp()?
> 
> Hmm missed that..  I'll drop that one in this same patch and rewrite the
> commit message.  New version attached:
> 
> ===8<===
> From 443fef4188d544362fc026b46784c15b82624642 Mon Sep 17 00:00:00 2001
> From: Peter Xu <peterx@redhat.com>
> Date: Mon, 17 Jun 2024 10:49:52 -0400
> Subject: [PATCH] tests/migration-tests: Always enable migration events
> 
> Libvirt should always enable it, so it'll be nice qtest also cover that for
> all tests on both sides.  migrate_incoming_qmp() used to enable it only on
> dst, now we enable them on both, as we'll start to sanity check events even
> on the src QEMU.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  tests/qtest/migration-helpers.c | 2 --
>  tests/qtest/migration-test.c    | 7 +++++++
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> index 0ac49ceb54..797b1e8c1c 100644
> --- a/tests/qtest/migration-helpers.c
> +++ b/tests/qtest/migration-helpers.c
> @@ -258,8 +258,6 @@ void migrate_incoming_qmp(QTestState *to, const char *uri, const char *fmt, ...)
>      g_assert(!qdict_haskey(args, "uri"));
>      qdict_put_str(args, "uri", uri);
>  
> -    migrate_set_capability(to, "events", true);
> -

Unfortunately this will break virtio-net-failover test... as it uses
migrate_incoming_qmp() without using test_migrate_start().

I'll leave it there for now, perhaps adding a comment.

>      rsp = qtest_qmp(to, "{ 'execute': 'migrate-incoming', 'arguments': %p}",
>                      args);
>  
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 640713bfd5..c015e801ac 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -851,6 +851,13 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>          unlink(shmem_path);
>      }
>  
> +    /*
> +     * Always enable migration events.  Libvirt always uses it, let's try
> +     * to mimic as closer as that.
> +     */
> +    migrate_set_capability(*from, "events", true);
> +    migrate_set_capability(*to, "events", true);
> +
>      return 0;
>  }
>  
> -- 
> 2.45.0
> 
> 
> -- 
> Peter Xu

-- 
Peter Xu