[PATCH v6 11/13] tests: Add postcopy tls migration test

Peter Xu posted 13 patches 3 years, 1 month ago
There is a newer version of this series
[PATCH v6 11/13] tests: Add postcopy tls migration test
Posted by Peter Xu 3 years, 1 month ago
We just added TLS tests for precopy but not postcopy.  Add the
corresponding test for vanilla postcopy.

Rename the vanilla postcopy to "postcopy/plain" because all postcopy tests
will only use unix sockets as channel.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tests/qtest/migration-test.c | 50 +++++++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index d33e8060f9..e8304aa454 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -481,6 +481,10 @@ typedef struct {
     bool only_target;
     /* Use dirty ring if true; dirty logging otherwise */
     bool use_dirty_ring;
+    /* Whether use TLS channels for postcopy test? */
+    bool postcopy_tls;
+    /* Used only if postcopy_tls==true, to cache the data object */
+    void *postcopy_tls_data;
     const char *opts_source;
     const char *opts_target;
 } MigrateStart;
@@ -980,6 +984,10 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
         return -1;
     }
 
+    if (args->postcopy_tls) {
+        args->postcopy_tls_data = test_migrate_tls_psk_start_match(from, to);
+    }
+
     migrate_set_capability(from, "postcopy-ram", true);
     migrate_set_capability(to, "postcopy-ram", true);
     migrate_set_capability(to, "postcopy-blocktime", true);
@@ -1004,7 +1012,8 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
     return 0;
 }
 
-static void migrate_postcopy_complete(QTestState *from, QTestState *to)
+static void migrate_postcopy_complete(QTestState *from, QTestState *to,
+                                      MigrateStart *args)
 {
     wait_for_migration_complete(from);
 
@@ -1015,19 +1024,38 @@ static void migrate_postcopy_complete(QTestState *from, QTestState *to)
         read_blocktime(to);
     }
 
+    if (args->postcopy_tls) {
+        assert(args->postcopy_tls_data);
+        test_migrate_tls_psk_finish(from, to, args->postcopy_tls_data);
+        args->postcopy_tls_data = NULL;
+    }
+
     test_migrate_end(from, to, true);
 }
 
-static void test_postcopy(void)
+static void test_postcopy_common(MigrateStart *args)
 {
-    MigrateStart args = {};
     QTestState *from, *to;
 
-    if (migrate_postcopy_prepare(&from, &to, &args)) {
+    if (migrate_postcopy_prepare(&from, &to, args)) {
         return;
     }
     migrate_postcopy_start(from, to);
-    migrate_postcopy_complete(from, to);
+    migrate_postcopy_complete(from, to, args);
+}
+
+static void test_postcopy(void)
+{
+    MigrateStart args = { };
+
+    test_postcopy_common(&args);
+}
+
+static void test_postcopy_tls_psk(void)
+{
+    MigrateStart args = { .postcopy_tls = true };
+
+    test_postcopy_common(&args);
 }
 
 static void test_postcopy_recovery(void)
@@ -1089,7 +1117,7 @@ static void test_postcopy_recovery(void)
     /* Restore the postcopy bandwidth to unlimited */
     migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0);
 
-    migrate_postcopy_complete(from, to);
+    migrate_postcopy_complete(from, to, &args);
 }
 
 static void test_baddest(void)
@@ -2133,7 +2161,15 @@ int main(int argc, char **argv)
 
     module_call_init(MODULE_INIT_QOM);
 
-    qtest_add_func("/migration/postcopy/unix", test_postcopy);
+    qtest_add_func("/migration/postcopy/plain", test_postcopy);
+#ifdef CONFIG_GNUTLS
+    /*
+     * NOTE: psk test is enough for postcopy, as other types of TLS
+     * channels are tested under precopy.  Here what we want to test is the
+     * general postcopy path that has TLS channel enabled.
+     */
+    qtest_add_func("/migration/postcopy/tls/psk", test_postcopy_tls_psk);
+#endif /* CONFIG_GNUTLS */
     qtest_add_func("/migration/postcopy/recovery", test_postcopy_recovery);
     qtest_add_func("/migration/bad_dest", test_baddest);
     qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
-- 
2.32.0
Re: [PATCH v6 11/13] tests: Add postcopy tls migration test
Posted by Daniel P. Berrangé 3 years, 1 month ago
On Tue, May 17, 2022 at 03:57:28PM -0400, Peter Xu wrote:
> We just added TLS tests for precopy but not postcopy.  Add the
> corresponding test for vanilla postcopy.
> 
> Rename the vanilla postcopy to "postcopy/plain" because all postcopy tests
> will only use unix sockets as channel.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  tests/qtest/migration-test.c | 50 +++++++++++++++++++++++++++++++-----
>  1 file changed, 43 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index d33e8060f9..e8304aa454 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -481,6 +481,10 @@ typedef struct {
>      bool only_target;
>      /* Use dirty ring if true; dirty logging otherwise */
>      bool use_dirty_ring;
> +    /* Whether use TLS channels for postcopy test? */
> +    bool postcopy_tls;
> +    /* Used only if postcopy_tls==true, to cache the data object */
> +    void *postcopy_tls_data;

Rather than adding these fields, I think it would be preferrable to
pass the hooks in the same way I did for the precopy tests.

>      const char *opts_source;
>      const char *opts_target;
>  } MigrateStart;
> @@ -980,6 +984,10 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
>          return -1;

Add 'TestMigrateStartHook start_hook' to this method

>      }
>  
> +    if (args->postcopy_tls) {
> +        args->postcopy_tls_data = test_migrate_tls_psk_start_match(from, to);
> +    }

call the 'start_hook'

> +
>      migrate_set_capability(from, "postcopy-ram", true);
>      migrate_set_capability(to, "postcopy-ram", true);
>      migrate_set_capability(to, "postcopy-blocktime", true);
> @@ -1004,7 +1012,8 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
>      return 0;


>  }
>  
> -static void migrate_postcopy_complete(QTestState *from, QTestState *to)
> +static void migrate_postcopy_complete(QTestState *from, QTestState *to,
> +                                      MigrateStart *args)

Here pass in the TestMigrateFinishHook , and the void* data
from the migrate_postcoy_prepare method.

>  {
>      wait_for_migration_complete(from);
>  
> @@ -1015,19 +1024,38 @@ static void migrate_postcopy_complete(QTestState *from, QTestState *to)
>          read_blocktime(to);
>      }
>  
> +    if (args->postcopy_tls) {
> +        assert(args->postcopy_tls_data);
> +        test_migrate_tls_psk_finish(from, to, args->postcopy_tls_data);
> +        args->postcopy_tls_data = NULL;
> +    }
> +
>      test_migrate_end(from, to, true);
>  }
>  
> -static void test_postcopy(void)
> +static void test_postcopy_common(MigrateStart *args)

Use 'MigrateCommon' here, which embeds a copy of 'MigrateStart',
while also having fields to the TestMigrateStartHook and
TestMigrateFinishHook.

>  {
> -    MigrateStart args = {};
>      QTestState *from, *to;
>  
> -    if (migrate_postcopy_prepare(&from, &to, &args)) {
> +    if (migrate_postcopy_prepare(&from, &to, args)) {
>          return;
>      }
>      migrate_postcopy_start(from, to);
> -    migrate_postcopy_complete(from, to);
> +    migrate_postcopy_complete(from, to, args);

eg this would become

    void *hook_data;

    if (migrate_postcopy_prepare(&from, &to, args.start_hook, &hook_data)) {
        return;
    }
    migrate_postcopy_start(from, to);
    migrate_postcopy_complete(from, to, args.finish_hook, hook_data);
   

> +}
> +
> +static void test_postcopy(void)
> +{
> +    MigrateStart args = { };

MigrateCommon args = {}


> +
> +    test_postcopy_common(&args);
> +}
> +
> +static void test_postcopy_tls_psk(void)
> +{
> +    MigrateStart args = { .postcopy_tls = true };

MigrateCommon args = {
   .start_hook = test_migrate_tls_psk_start_match,
   .finish_hook = test_migrate_tls_psk_finish,
}

> +
> +    test_postcopy_common(&args);
>  }
>  
>  static void test_postcopy_recovery(void)

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 v6 11/13] tests: Add postcopy tls migration test
Posted by Peter Xu 3 years, 1 month ago
On Thu, May 19, 2022 at 11:11:34AM +0100, Daniel P. Berrangé wrote:
> On Tue, May 17, 2022 at 03:57:28PM -0400, Peter Xu wrote:
> > We just added TLS tests for precopy but not postcopy.  Add the
> > corresponding test for vanilla postcopy.
> > 
> > Rename the vanilla postcopy to "postcopy/plain" because all postcopy tests
> > will only use unix sockets as channel.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  tests/qtest/migration-test.c | 50 +++++++++++++++++++++++++++++++-----
> >  1 file changed, 43 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index d33e8060f9..e8304aa454 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -481,6 +481,10 @@ typedef struct {
> >      bool only_target;
> >      /* Use dirty ring if true; dirty logging otherwise */
> >      bool use_dirty_ring;
> > +    /* Whether use TLS channels for postcopy test? */
> > +    bool postcopy_tls;
> > +    /* Used only if postcopy_tls==true, to cache the data object */
> > +    void *postcopy_tls_data;
> 
> Rather than adding these fields, I think it would be preferrable to
> pass the hooks in the same way I did for the precopy tests.

I can give it a shot.

Ideally I think we should rename MigrationCommon to MigrationPrecopy and
keep all the precopy stuff there, meanwhile we could have MigrationPostcopy
which will also include MigrationStart but keeps the postcopy bits around.
Then I'd need to move start_hook and so into MigrationStart.  But let me
start from simple..

-- 
Peter Xu


Re: [PATCH v6 11/13] tests: Add postcopy tls migration test
Posted by Dr. David Alan Gilbert 3 years, 1 month ago
* Peter Xu (peterx@redhat.com) wrote:
> We just added TLS tests for precopy but not postcopy.  Add the
> corresponding test for vanilla postcopy.
> 
> Rename the vanilla postcopy to "postcopy/plain" because all postcopy tests
> will only use unix sockets as channel.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  tests/qtest/migration-test.c | 50 +++++++++++++++++++++++++++++++-----
>  1 file changed, 43 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index d33e8060f9..e8304aa454 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -481,6 +481,10 @@ typedef struct {
>      bool only_target;
>      /* Use dirty ring if true; dirty logging otherwise */
>      bool use_dirty_ring;
> +    /* Whether use TLS channels for postcopy test? */
> +    bool postcopy_tls;
> +    /* Used only if postcopy_tls==true, to cache the data object */
> +    void *postcopy_tls_data;
>      const char *opts_source;
>      const char *opts_target;
>  } MigrateStart;
> @@ -980,6 +984,10 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
>          return -1;
>      }
>  
> +    if (args->postcopy_tls) {
> +        args->postcopy_tls_data = test_migrate_tls_psk_start_match(from, to);
> +    }
> +
>      migrate_set_capability(from, "postcopy-ram", true);
>      migrate_set_capability(to, "postcopy-ram", true);
>      migrate_set_capability(to, "postcopy-blocktime", true);
> @@ -1004,7 +1012,8 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
>      return 0;
>  }
>  
> -static void migrate_postcopy_complete(QTestState *from, QTestState *to)
> +static void migrate_postcopy_complete(QTestState *from, QTestState *to,
> +                                      MigrateStart *args)
>  {
>      wait_for_migration_complete(from);
>  
> @@ -1015,19 +1024,38 @@ static void migrate_postcopy_complete(QTestState *from, QTestState *to)
>          read_blocktime(to);
>      }
>  
> +    if (args->postcopy_tls) {
> +        assert(args->postcopy_tls_data);
> +        test_migrate_tls_psk_finish(from, to, args->postcopy_tls_data);
> +        args->postcopy_tls_data = NULL;
> +    }
> +
>      test_migrate_end(from, to, true);
>  }
>  
> -static void test_postcopy(void)
> +static void test_postcopy_common(MigrateStart *args)
>  {
> -    MigrateStart args = {};
>      QTestState *from, *to;
>  
> -    if (migrate_postcopy_prepare(&from, &to, &args)) {
> +    if (migrate_postcopy_prepare(&from, &to, args)) {
>          return;
>      }
>      migrate_postcopy_start(from, to);
> -    migrate_postcopy_complete(from, to);
> +    migrate_postcopy_complete(from, to, args);
> +}
> +
> +static void test_postcopy(void)
> +{
> +    MigrateStart args = { };
> +
> +    test_postcopy_common(&args);
> +}
> +
> +static void test_postcopy_tls_psk(void)
> +{
> +    MigrateStart args = { .postcopy_tls = true };
> +
> +    test_postcopy_common(&args);
>  }
>  
>  static void test_postcopy_recovery(void)
> @@ -1089,7 +1117,7 @@ static void test_postcopy_recovery(void)
>      /* Restore the postcopy bandwidth to unlimited */
>      migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0);
>  
> -    migrate_postcopy_complete(from, to);
> +    migrate_postcopy_complete(from, to, &args);
>  }
>  
>  static void test_baddest(void)
> @@ -2133,7 +2161,15 @@ int main(int argc, char **argv)
>  
>      module_call_init(MODULE_INIT_QOM);
>  
> -    qtest_add_func("/migration/postcopy/unix", test_postcopy);
> +    qtest_add_func("/migration/postcopy/plain", test_postcopy);
> +#ifdef CONFIG_GNUTLS
> +    /*
> +     * NOTE: psk test is enough for postcopy, as other types of TLS
> +     * channels are tested under precopy.  Here what we want to test is the
> +     * general postcopy path that has TLS channel enabled.
> +     */
> +    qtest_add_func("/migration/postcopy/tls/psk", test_postcopy_tls_psk);
> +#endif /* CONFIG_GNUTLS */
>      qtest_add_func("/migration/postcopy/recovery", test_postcopy_recovery);
>      qtest_add_func("/migration/bad_dest", test_baddest);
>      qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
> -- 
> 2.32.0
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK