| Message ID | 20240319164527.1873891-8-yuan1.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Live Migration With IAA | expand |
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
> -----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 :|
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
> -----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 --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);