diff mbox series

[v5,7/7] tests/migration-test: add qpl compression test

Message ID 20240319164527.1873891-8-yuan1.liu@intel.com (mailing list archive)
State New, archived
Headers show
Series Live Migration With IAA | expand

Commit Message

Yuan Liu March 19, 2024, 4:45 p.m. UTC 
add qpl to compression method test for multifd migration

the migration with qpl compression needs to access IAA hardware
resource, please run "check-qtest" with sudo or root permission,
otherwise migration test will fail

Signed-off-by: Yuan Liu <yuan1.liu@intel.com>
Reviewed-by: Nanhai Zou <nanhai.zou@intel.com>
---
 tests/qtest/migration-test.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Comments

Daniel P. Berrangé March 20, 2024, 10:45 a.m. UTC | #1 
On Wed, Mar 20, 2024 at 12:45:27AM +0800, Yuan Liu wrote:
> add qpl to compression method test for multifd migration
> 
> the migration with qpl compression needs to access IAA hardware
> resource, please run "check-qtest" with sudo or root permission,
> otherwise migration test will fail

That's not an acceptable requirement.

If someone builds QEMU with QPL, the migration test *must*
pass 100% reliably when either running on a host without
the QPL required hardware, or when lacking permissions.

The test case needs to detect these scenarios and automatically
skip the test if it is incapable of running successfully.

This raises another question though. If QPL migration requires
running as root, then it is effectively unusable for QEMU, as
no sane deployment ever runs QEMU as root.

Is there a way to make QPL work for non-root users ?

> 
> Signed-off-by: Yuan Liu <yuan1.liu@intel.com>
> Reviewed-by: Nanhai Zou <nanhai.zou@intel.com>
> ---
>  tests/qtest/migration-test.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 71895abb7f..052d0d60fd 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -2815,6 +2815,15 @@ test_migrate_precopy_tcp_multifd_zstd_start(QTestState *from,
>  }
>  #endif /* CONFIG_ZSTD */
>  
> +#ifdef CONFIG_QPL
> +static void *
> +test_migrate_precopy_tcp_multifd_qpl_start(QTestState *from,
> +                                            QTestState *to)
> +{
> +    return test_migrate_precopy_tcp_multifd_start_common(from, to, "qpl");
> +}
> +#endif /* CONFIG_QPL */
> +
>  static void test_multifd_tcp_none(void)
>  {
>      MigrateCommon args = {
> @@ -2880,6 +2889,17 @@ static void test_multifd_tcp_zstd(void)
>  }
>  #endif
>  
> +#ifdef CONFIG_QPL
> +static void test_multifd_tcp_qpl(void)
> +{
> +    MigrateCommon args = {
> +        .listen_uri = "defer",
> +        .start_hook = test_migrate_precopy_tcp_multifd_qpl_start,
> +    };
> +    test_precopy_common(&args);
> +}
> +#endif
> +
>  #ifdef CONFIG_GNUTLS
>  static void *
>  test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
> @@ -3789,6 +3809,10 @@ int main(int argc, char **argv)
>      migration_test_add("/migration/multifd/tcp/plain/zstd",
>                         test_multifd_tcp_zstd);
>  #endif
> +#ifdef CONFIG_QPL
> +    migration_test_add("/migration/multifd/tcp/plain/qpl",
> +                       test_multifd_tcp_qpl);
> +#endif
>  #ifdef CONFIG_GNUTLS
>      migration_test_add("/migration/multifd/tcp/tls/psk/match",
>                         test_multifd_tcp_tls_psk_match);
> -- 
> 2.39.3
> 
> 

With regards,
Daniel
Yuan Liu March 20, 2024, 3:30 p.m. UTC | #2 
> -----Original Message-----
> From: Daniel P. Berrangé <berrange@redhat.com>
> Sent: Wednesday, March 20, 2024 6:46 PM
> To: Liu, Yuan1 <yuan1.liu@intel.com>
> Cc: peterx@redhat.com; farosas@suse.de; qemu-devel@nongnu.org;
> hao.xiang@bytedance.com; bryan.zhang@bytedance.com; Zou, Nanhai
> <nanhai.zou@intel.com>
> Subject: Re: [PATCH v5 7/7] tests/migration-test: add qpl compression test
> 
> On Wed, Mar 20, 2024 at 12:45:27AM +0800, Yuan Liu wrote:
> > add qpl to compression method test for multifd migration
> >
> > the migration with qpl compression needs to access IAA hardware
> > resource, please run "check-qtest" with sudo or root permission,
> > otherwise migration test will fail
> 
> That's not an acceptable requirement.
> 
> If someone builds QEMU with QPL, the migration test *must*
> pass 100% reliably when either running on a host without
> the QPL required hardware, or when lacking permissions.
> 
> The test case needs to detect these scenarios and automatically
> skip the test if it is incapable of running successfully.
> This raises another question though. If QPL migration requires
> running as root, then it is effectively unusable for QEMU, as
> no sane deployment ever runs QEMU as root.
> 
> Is there a way to make QPL work for non-root users ?

There are two issues here
1. I need to add an IAA resource detection before the QPL test begins
   In this way, when QPL resources are unavailable, the live migration 
   test will not be affected.

2. I need to add some additional information about IAA configuration in 
   the devel/qpl-compression.rst documentation. In addition to configuring 
   IAA resources, the system administrator also needs to assign IAA resources
   to user groups.
   For example, the system administrator runs "chown -R user /dev/iax", then
   all IAA resources can be accessed by "user", this method does not require 
   sudo or root permissions

> > Signed-off-by: Yuan Liu <yuan1.liu@intel.com>
> > Reviewed-by: Nanhai Zou <nanhai.zou@intel.com>
> > ---
> >  tests/qtest/migration-test.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index 71895abb7f..052d0d60fd 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -2815,6 +2815,15 @@
> test_migrate_precopy_tcp_multifd_zstd_start(QTestState *from,
> >  }
> >  #endif /* CONFIG_ZSTD */
> >
> > +#ifdef CONFIG_QPL
> > +static void *
> > +test_migrate_precopy_tcp_multifd_qpl_start(QTestState *from,
> > +                                            QTestState *to)
> > +{
> > +    return test_migrate_precopy_tcp_multifd_start_common(from, to,
> "qpl");
> > +}
> > +#endif /* CONFIG_QPL */
> > +
> >  static void test_multifd_tcp_none(void)
> >  {
> >      MigrateCommon args = {
> > @@ -2880,6 +2889,17 @@ static void test_multifd_tcp_zstd(void)
> >  }
> >  #endif
> >
> > +#ifdef CONFIG_QPL
> > +static void test_multifd_tcp_qpl(void)
> > +{
> > +    MigrateCommon args = {
> > +        .listen_uri = "defer",
> > +        .start_hook = test_migrate_precopy_tcp_multifd_qpl_start,
> > +    };
> > +    test_precopy_common(&args);
> > +}
> > +#endif
> > +
> >  #ifdef CONFIG_GNUTLS
> >  static void *
> >  test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
> > @@ -3789,6 +3809,10 @@ int main(int argc, char **argv)
> >      migration_test_add("/migration/multifd/tcp/plain/zstd",
> >                         test_multifd_tcp_zstd);
> >  #endif
> > +#ifdef CONFIG_QPL
> > +    migration_test_add("/migration/multifd/tcp/plain/qpl",
> > +                       test_multifd_tcp_qpl);
> > +#endif
> >  #ifdef CONFIG_GNUTLS
> >      migration_test_add("/migration/multifd/tcp/tls/psk/match",
> >                         test_multifd_tcp_tls_psk_match);
> > --
> > 2.39.3
> >
> >
> 
> 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 :|
Daniel P. Berrangé March 20, 2024, 3:39 p.m. UTC | #3 
On Wed, Mar 20, 2024 at 03:30:40PM +0000, Liu, Yuan1 wrote:
> > -----Original Message-----
> > From: Daniel P. Berrangé <berrange@redhat.com>
> > Sent: Wednesday, March 20, 2024 6:46 PM
> > To: Liu, Yuan1 <yuan1.liu@intel.com>
> > Cc: peterx@redhat.com; farosas@suse.de; qemu-devel@nongnu.org;
> > hao.xiang@bytedance.com; bryan.zhang@bytedance.com; Zou, Nanhai
> > <nanhai.zou@intel.com>
> > Subject: Re: [PATCH v5 7/7] tests/migration-test: add qpl compression test
> > 
> > On Wed, Mar 20, 2024 at 12:45:27AM +0800, Yuan Liu wrote:
> > > add qpl to compression method test for multifd migration
> > >
> > > the migration with qpl compression needs to access IAA hardware
> > > resource, please run "check-qtest" with sudo or root permission,
> > > otherwise migration test will fail
> > 
> > That's not an acceptable requirement.
> > 
> > If someone builds QEMU with QPL, the migration test *must*
> > pass 100% reliably when either running on a host without
> > the QPL required hardware, or when lacking permissions.
> > 
> > The test case needs to detect these scenarios and automatically
> > skip the test if it is incapable of running successfully.
> > This raises another question though. If QPL migration requires
> > running as root, then it is effectively unusable for QEMU, as
> > no sane deployment ever runs QEMU as root.
> > 
> > Is there a way to make QPL work for non-root users ?
> 
> There are two issues here
> 1. I need to add an IAA resource detection before the QPL test begins
>    In this way, when QPL resources are unavailable, the live migration 
>    test will not be affected.
> 
> 2. I need to add some additional information about IAA configuration in 
>    the devel/qpl-compression.rst documentation. In addition to configuring 
>    IAA resources, the system administrator also needs to assign IAA resources
>    to user groups.
>    For example, the system administrator runs "chown -R user /dev/iax", then
>    all IAA resources can be accessed by "user", this method does not require 
>    sudo or root permissions

Ok, so in the test suite you likely should do something
approximately like

#ifdef CONFIG_QPL
  if (access("/dev/iax", R_OK|W_OK) == 0) {
    migration_test_add("/migration/multifd/tcp/plain/qpl",
                       test_multifd_tcp_qpl);
  }
#endif

possibly more if you need to actually query supported features
of /dev/iax before trying to use it

> 
> > > Signed-off-by: Yuan Liu <yuan1.liu@intel.com>
> > > Reviewed-by: Nanhai Zou <nanhai.zou@intel.com>
> > > ---
> > >  tests/qtest/migration-test.c | 24 ++++++++++++++++++++++++
> > >  1 file changed, 24 insertions(+)
> > >
> > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > > index 71895abb7f..052d0d60fd 100644
> > > --- a/tests/qtest/migration-test.c
> > > +++ b/tests/qtest/migration-test.c
> > > @@ -2815,6 +2815,15 @@
> > test_migrate_precopy_tcp_multifd_zstd_start(QTestState *from,
> > >  }
> > >  #endif /* CONFIG_ZSTD */
> > >
> > > +#ifdef CONFIG_QPL
> > > +static void *
> > > +test_migrate_precopy_tcp_multifd_qpl_start(QTestState *from,
> > > +                                            QTestState *to)
> > > +{
> > > +    return test_migrate_precopy_tcp_multifd_start_common(from, to,
> > "qpl");
> > > +}
> > > +#endif /* CONFIG_QPL */
> > > +
> > >  static void test_multifd_tcp_none(void)
> > >  {
> > >      MigrateCommon args = {
> > > @@ -2880,6 +2889,17 @@ static void test_multifd_tcp_zstd(void)
> > >  }
> > >  #endif
> > >
> > > +#ifdef CONFIG_QPL
> > > +static void test_multifd_tcp_qpl(void)
> > > +{
> > > +    MigrateCommon args = {
> > > +        .listen_uri = "defer",
> > > +        .start_hook = test_migrate_precopy_tcp_multifd_qpl_start,
> > > +    };
> > > +    test_precopy_common(&args);
> > > +}
> > > +#endif
> > > +
> > >  #ifdef CONFIG_GNUTLS
> > >  static void *
> > >  test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
> > > @@ -3789,6 +3809,10 @@ int main(int argc, char **argv)
> > >      migration_test_add("/migration/multifd/tcp/plain/zstd",
> > >                         test_multifd_tcp_zstd);
> > >  #endif
> > > +#ifdef CONFIG_QPL
> > > +    migration_test_add("/migration/multifd/tcp/plain/qpl",
> > > +                       test_multifd_tcp_qpl);
> > > +#endif
> > >  #ifdef CONFIG_GNUTLS
> > >      migration_test_add("/migration/multifd/tcp/tls/psk/match",
> > >                         test_multifd_tcp_tls_psk_match);
> > > --
> > > 2.39.3
> > >
> > >
> > 
> > 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 :|
> 

With regards,
Daniel
Yuan Liu March 20, 2024, 4:26 p.m. UTC | #4 
> -----Original Message-----
> From: Daniel P. Berrangé <berrange@redhat.com>
> Sent: Wednesday, March 20, 2024 11:40 PM
> To: Liu, Yuan1 <yuan1.liu@intel.com>
> Cc: peterx@redhat.com; farosas@suse.de; qemu-devel@nongnu.org;
> hao.xiang@bytedance.com; bryan.zhang@bytedance.com; Zou, Nanhai
> <nanhai.zou@intel.com>
> Subject: Re: [PATCH v5 7/7] tests/migration-test: add qpl compression test
> 
> On Wed, Mar 20, 2024 at 03:30:40PM +0000, Liu, Yuan1 wrote:
> > > -----Original Message-----
> > > From: Daniel P. Berrangé <berrange@redhat.com>
> > > Sent: Wednesday, March 20, 2024 6:46 PM
> > > To: Liu, Yuan1 <yuan1.liu@intel.com>
> > > Cc: peterx@redhat.com; farosas@suse.de; qemu-devel@nongnu.org;
> > > hao.xiang@bytedance.com; bryan.zhang@bytedance.com; Zou, Nanhai
> > > <nanhai.zou@intel.com>
> > > Subject: Re: [PATCH v5 7/7] tests/migration-test: add qpl compression
> test
> > >
> > > On Wed, Mar 20, 2024 at 12:45:27AM +0800, Yuan Liu wrote:
> > > > add qpl to compression method test for multifd migration
> > > >
> > > > the migration with qpl compression needs to access IAA hardware
> > > > resource, please run "check-qtest" with sudo or root permission,
> > > > otherwise migration test will fail
> > >
> > > That's not an acceptable requirement.
> > >
> > > If someone builds QEMU with QPL, the migration test *must*
> > > pass 100% reliably when either running on a host without
> > > the QPL required hardware, or when lacking permissions.
> > >
> > > The test case needs to detect these scenarios and automatically
> > > skip the test if it is incapable of running successfully.
> > > This raises another question though. If QPL migration requires
> > > running as root, then it is effectively unusable for QEMU, as
> > > no sane deployment ever runs QEMU as root.
> > >
> > > Is there a way to make QPL work for non-root users ?
> >
> > There are two issues here
> > 1. I need to add an IAA resource detection before the QPL test begins
> >    In this way, when QPL resources are unavailable, the live migration
> >    test will not be affected.
> >
> > 2. I need to add some additional information about IAA configuration in
> >    the devel/qpl-compression.rst documentation. In addition to
> configuring
> >    IAA resources, the system administrator also needs to assign IAA
> resources
> >    to user groups.
> >    For example, the system administrator runs "chown -R user /dev/iax",
> then
> >    all IAA resources can be accessed by "user", this method does not
> require
> >    sudo or root permissions
> 
> Ok, so in the test suite you likely should do something
> approximately like
> 
> #ifdef CONFIG_QPL
>   if (access("/dev/iax", R_OK|W_OK) == 0) {
>     migration_test_add("/migration/multifd/tcp/plain/qpl",
>                        test_multifd_tcp_qpl);
>   }
> #endif
> 
> possibly more if you need to actually query supported features
> of /dev/iax before trying to use it

Yes, very thanks for your suggestion, I will fix this in the next version.

> > > > Signed-off-by: Yuan Liu <yuan1.liu@intel.com>
> > > > Reviewed-by: Nanhai Zou <nanhai.zou@intel.com>
> > > > ---
> > > >  tests/qtest/migration-test.c | 24 ++++++++++++++++++++++++
> > > >  1 file changed, 24 insertions(+)
> > > >
> > > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-
> test.c
> > > > index 71895abb7f..052d0d60fd 100644
> > > > --- a/tests/qtest/migration-test.c
> > > > +++ b/tests/qtest/migration-test.c
> > > > @@ -2815,6 +2815,15 @@
> > > test_migrate_precopy_tcp_multifd_zstd_start(QTestState *from,
> > > >  }
> > > >  #endif /* CONFIG_ZSTD */
> > > >
> > > > +#ifdef CONFIG_QPL
> > > > +static void *
> > > > +test_migrate_precopy_tcp_multifd_qpl_start(QTestState *from,
> > > > +                                            QTestState *to)
> > > > +{
> > > > +    return test_migrate_precopy_tcp_multifd_start_common(from, to,
> > > "qpl");
> > > > +}
> > > > +#endif /* CONFIG_QPL */
> > > > +
> > > >  static void test_multifd_tcp_none(void)
> > > >  {
> > > >      MigrateCommon args = {
> > > > @@ -2880,6 +2889,17 @@ static void test_multifd_tcp_zstd(void)
> > > >  }
> > > >  #endif
> > > >
> > > > +#ifdef CONFIG_QPL
> > > > +static void test_multifd_tcp_qpl(void)
> > > > +{
> > > > +    MigrateCommon args = {
> > > > +        .listen_uri = "defer",
> > > > +        .start_hook = test_migrate_precopy_tcp_multifd_qpl_start,
> > > > +    };
> > > > +    test_precopy_common(&args);
> > > > +}
> > > > +#endif
> > > > +
> > > >  #ifdef CONFIG_GNUTLS
> > > >  static void *
> > > >  test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
> > > > @@ -3789,6 +3809,10 @@ int main(int argc, char **argv)
> > > >      migration_test_add("/migration/multifd/tcp/plain/zstd",
> > > >                         test_multifd_tcp_zstd);
> > > >  #endif
> > > > +#ifdef CONFIG_QPL
> > > > +    migration_test_add("/migration/multifd/tcp/plain/qpl",
> > > > +                       test_multifd_tcp_qpl);
> > > > +#endif
> > > >  #ifdef CONFIG_GNUTLS
> > > >      migration_test_add("/migration/multifd/tcp/tls/psk/match",
> > > >                         test_multifd_tcp_tls_psk_match);
> > > > --
> > > > 2.39.3
> > > >
> > > >
> > >
> > > 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 :|
> >
> 
> 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 :|
diff mbox series

Patch

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 71895abb7f..052d0d60fd 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2815,6 +2815,15 @@  test_migrate_precopy_tcp_multifd_zstd_start(QTestState *from,
 }
 #endif /* CONFIG_ZSTD */
 
+#ifdef CONFIG_QPL
+static void *
+test_migrate_precopy_tcp_multifd_qpl_start(QTestState *from,
+                                            QTestState *to)
+{
+    return test_migrate_precopy_tcp_multifd_start_common(from, to, "qpl");
+}
+#endif /* CONFIG_QPL */
+
 static void test_multifd_tcp_none(void)
 {
     MigrateCommon args = {
@@ -2880,6 +2889,17 @@  static void test_multifd_tcp_zstd(void)
 }
 #endif
 
+#ifdef CONFIG_QPL
+static void test_multifd_tcp_qpl(void)
+{
+    MigrateCommon args = {
+        .listen_uri = "defer",
+        .start_hook = test_migrate_precopy_tcp_multifd_qpl_start,
+    };
+    test_precopy_common(&args);
+}
+#endif
+
 #ifdef CONFIG_GNUTLS
 static void *
 test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
@@ -3789,6 +3809,10 @@  int main(int argc, char **argv)
     migration_test_add("/migration/multifd/tcp/plain/zstd",
                        test_multifd_tcp_zstd);
 #endif
+#ifdef CONFIG_QPL
+    migration_test_add("/migration/multifd/tcp/plain/qpl",
+                       test_multifd_tcp_qpl);
+#endif
 #ifdef CONFIG_GNUTLS
     migration_test_add("/migration/multifd/tcp/tls/psk/match",
                        test_multifd_tcp_tls_psk_match);