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

[Liu, Yuan1]

> -----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 :|