diff mbox series

[v3,7/9] tests/qtest: capture RESUME events during migration

Message ID 20230531132400.1129576-8-berrange@redhat.com (mailing list archive)
State New, archived
Headers show
Series tests/qtest: make migration-test massively faster | expand

Commit Message

Daniel P. Berrangé May 31, 2023, 1:23 p.m. UTC
When running migration tests we monitor for a STOP event so we can skip
redundant waits. This will be needed for the RESUME event too shortly.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qtest/migration-helpers.c | 12 ++++++++++++
 tests/qtest/migration-helpers.h |  2 ++
 tests/qtest/migration-test.c    |  5 +++++
 3 files changed, 19 insertions(+)

Comments

Thomas Huth June 1, 2023, 9:38 a.m. UTC | #1
On 31/05/2023 15.23, Daniel P. Berrangé wrote:
> When running migration tests we monitor for a STOP event so we can skip
> redundant waits. This will be needed for the RESUME event too shortly.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/qtest/migration-helpers.c | 12 ++++++++++++
>   tests/qtest/migration-helpers.h |  2 ++
>   tests/qtest/migration-test.c    |  5 +++++
>   3 files changed, 19 insertions(+)


Acked-by: Thomas Huth <thuth@redhat.com>
Juan Quintela June 1, 2023, 12:31 p.m. UTC | #2
Daniel P. Berrangé <berrange@redhat.com> wrote:
> When running migration tests we monitor for a STOP event so we can skip
> redundant waits. This will be needed for the RESUME event too shortly.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qtest/migration-helpers.c | 12 ++++++++++++
>  tests/qtest/migration-helpers.h |  2 ++
>  tests/qtest/migration-test.c    |  5 +++++
>  3 files changed, 19 insertions(+)
>
> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> index 884d8a2e07..d50b565967 100644
> --- a/tests/qtest/migration-helpers.c
> +++ b/tests/qtest/migration-helpers.c
> @@ -35,6 +35,18 @@ bool migrate_watch_for_stop(QTestState *who, const char *name,
>      return false;
>  }
>  
> +bool migrate_watch_for_resume(QTestState *who, const char *name,
> +                              QDict *event, void *opaque)
> +{
> +    bool *seen = opaque;
> +
> +    if (g_str_equal(name, "RESUME")) {
> +        *seen = true;
> +    }
> +
> +    return false;
> +}
> +

I think I am not understanding this.

Can we wait for both RESUME and STOP events?

Or do you want an implementation that can only look for one event?

Later, Juan.
Daniel P. Berrangé June 1, 2023, 12:34 p.m. UTC | #3
On Thu, Jun 01, 2023 at 02:31:13PM +0200, Juan Quintela wrote:
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> > When running migration tests we monitor for a STOP event so we can skip
> > redundant waits. This will be needed for the RESUME event too shortly.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  tests/qtest/migration-helpers.c | 12 ++++++++++++
> >  tests/qtest/migration-helpers.h |  2 ++
> >  tests/qtest/migration-test.c    |  5 +++++
> >  3 files changed, 19 insertions(+)
> >
> > diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> > index 884d8a2e07..d50b565967 100644
> > --- a/tests/qtest/migration-helpers.c
> > +++ b/tests/qtest/migration-helpers.c
> > @@ -35,6 +35,18 @@ bool migrate_watch_for_stop(QTestState *who, const char *name,
> >      return false;
> >  }
> >  
> > +bool migrate_watch_for_resume(QTestState *who, const char *name,
> > +                              QDict *event, void *opaque)
> > +{
> > +    bool *seen = opaque;
> > +
> > +    if (g_str_equal(name, "RESUME")) {
> > +        *seen = true;
> > +    }
> > +
> > +    return false;
> > +}
> > +
> 
> I think I am not understanding this.
> 
> Can we wait for both RESUME and STOP events?
> 
> Or do you want an implementation that can only look for one event?

For the purpose of the migration test we only need the STOP event
on the src and the RESUME event on the dst. While I could have made
it track both STOP and RESUME events on both src & dst, I figured
it was overkill.

With regards,
Daniel
Juan Quintela June 1, 2023, 12:37 p.m. UTC | #4
Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Thu, Jun 01, 2023 at 02:31:13PM +0200, Juan Quintela wrote:
>> Daniel P. Berrangé <berrange@redhat.com> wrote:
>> > When running migration tests we monitor for a STOP event so we can skip
>> > redundant waits. This will be needed for the RESUME event too shortly.
>> >
>> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> > ---
>> >  tests/qtest/migration-helpers.c | 12 ++++++++++++
>> >  tests/qtest/migration-helpers.h |  2 ++
>> >  tests/qtest/migration-test.c    |  5 +++++
>> >  3 files changed, 19 insertions(+)
>> >
>> > diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
>> > index 884d8a2e07..d50b565967 100644
>> > --- a/tests/qtest/migration-helpers.c
>> > +++ b/tests/qtest/migration-helpers.c
>> > @@ -35,6 +35,18 @@ bool migrate_watch_for_stop(QTestState *who, const char *name,
>> >      return false;
>> >  }
>> >  
>> > +bool migrate_watch_for_resume(QTestState *who, const char *name,
>> > +                              QDict *event, void *opaque)
>> > +{
>> > +    bool *seen = opaque;
>> > +
>> > +    if (g_str_equal(name, "RESUME")) {
>> > +        *seen = true;
>> > +    }
>> > +
>> > +    return false;
>> > +}
>> > +
>> 
>> I think I am not understanding this.
>> 
>> Can we wait for both RESUME and STOP events?
>> 
>> Or do you want an implementation that can only look for one event?
>
> For the purpose of the migration test we only need the STOP event
> on the src and the RESUME event on the dst. While I could have made
> it track both STOP and RESUME events on both src & dst, I figured
> it was overkill.

Fair enough.
Daniel P. Berrangé June 1, 2023, 12:44 p.m. UTC | #5
On Thu, Jun 01, 2023 at 02:37:37PM +0200, Juan Quintela wrote:
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> > On Thu, Jun 01, 2023 at 02:31:13PM +0200, Juan Quintela wrote:
> >> Daniel P. Berrangé <berrange@redhat.com> wrote:
> >> > When running migration tests we monitor for a STOP event so we can skip
> >> > redundant waits. This will be needed for the RESUME event too shortly.
> >> >
> >> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> >> > ---
> >> >  tests/qtest/migration-helpers.c | 12 ++++++++++++
> >> >  tests/qtest/migration-helpers.h |  2 ++
> >> >  tests/qtest/migration-test.c    |  5 +++++
> >> >  3 files changed, 19 insertions(+)
> >> >
> >> > diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> >> > index 884d8a2e07..d50b565967 100644
> >> > --- a/tests/qtest/migration-helpers.c
> >> > +++ b/tests/qtest/migration-helpers.c
> >> > @@ -35,6 +35,18 @@ bool migrate_watch_for_stop(QTestState *who, const char *name,
> >> >      return false;
> >> >  }
> >> >  
> >> > +bool migrate_watch_for_resume(QTestState *who, const char *name,
> >> > +                              QDict *event, void *opaque)
> >> > +{
> >> > +    bool *seen = opaque;
> >> > +
> >> > +    if (g_str_equal(name, "RESUME")) {
> >> > +        *seen = true;
> >> > +    }
> >> > +
> >> > +    return false;
> >> > +}
> >> > +
> >> 
> >> I think I am not understanding this.
> >> 
> >> Can we wait for both RESUME and STOP events?
> >> 
> >> Or do you want an implementation that can only look for one event?
> >
> > For the purpose of the migration test we only need the STOP event
> > on the src and the RESUME event on the dst. While I could have made
> > it track both STOP and RESUME events on both src & dst, I figured
> > it was overkill.
> 
> Fair enough.

I think I'll rename 'bool got_stop' to  'bool got_src_stop' and have
'bool got_dst_resume'  to make it explicit which QEMU they're referring
to.


With regards,
Daniel
diff mbox series

Patch

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 884d8a2e07..d50b565967 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -35,6 +35,18 @@  bool migrate_watch_for_stop(QTestState *who, const char *name,
     return false;
 }
 
+bool migrate_watch_for_resume(QTestState *who, const char *name,
+                              QDict *event, void *opaque)
+{
+    bool *seen = opaque;
+
+    if (g_str_equal(name, "RESUME")) {
+        *seen = true;
+    }
+
+    return false;
+}
+
 /*
  * Send QMP command "migrate".
  * Arguments are built from @fmt... (formatted like
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index aab0745cfe..009e250e90 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -17,6 +17,8 @@ 
 
 bool migrate_watch_for_stop(QTestState *who, const char *name,
                             QDict *event, void *opaque);
+bool migrate_watch_for_resume(QTestState *who, const char *name,
+                              QDict *event, void *opaque);
 
 G_GNUC_PRINTF(3, 4)
 void migrate_qmp(QTestState *who, const char *uri, const char *fmt, ...);
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index d8b4282abc..aee25e1c4f 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -44,6 +44,7 @@  unsigned start_address;
 unsigned end_address;
 static bool uffd_feature_thread_id;
 static bool got_stop;
+static bool got_resume;
 
 /*
  * Dirtylimit stop working if dirty page rate error
@@ -607,6 +608,7 @@  static int test_migrate_start(QTestState **from, QTestState **to,
     }
 
     got_stop = false;
+    got_resume = false;
     bootpath = g_strdup_printf("%s/bootsect", tmpfs);
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         /* the assembled x86 boot sector should be exactly one sector large */
@@ -712,6 +714,9 @@  static int test_migrate_start(QTestState **from, QTestState **to,
                                  args->opts_target ? args->opts_target : "",
                                  ignore_stderr);
     *to = qtest_init(cmd_target);
+    qtest_qmp_set_event_callback(*to,
+                                 migrate_watch_for_resume,
+                                 &got_resume);
 
     /*
      * Remove shmem file immediately to avoid memory leak in test failed case.