| Message ID | 01a063686e62ce97e7d0bc9fa935389f074ecb4b.1681983401.git.lukasstraub2@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration/ram.c: Refactor compress code | expand |
Lukas Straub <lukasstraub2@web.de> wrote: > Add postcopy tests with compress enabled to ensure nothing breaks > with the refactoring in the next commits. > > preempt+compress is blocked, so no test needed for that case. > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Juan Quintela <quintela@redhat.com> And I wanted to removed the old compression code and it gets new users. Sniff. > --- > tests/qtest/migration-test.c | 83 +++++++++++++++++++++++------------- > 1 file changed, 53 insertions(+), 30 deletions(-) > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > index 1f2a019ce0..930cb4f29d 100644 > --- a/tests/qtest/migration-test.c > +++ b/tests/qtest/migration-test.c > @@ -1127,6 +1127,36 @@ test_migrate_tls_x509_finish(QTestState *from, > #endif /* CONFIG_TASN1 */ > #endif /* CONFIG_GNUTLS */ > > +static void * > +test_migrate_compress_start(QTestState *from, > + QTestState *to) > +{ > + migrate_set_parameter_int(from, "compress-level", 1); > + migrate_set_parameter_int(from, "compress-threads", 4); > + migrate_set_parameter_bool(from, "compress-wait-thread", true); > + migrate_set_parameter_int(to, "decompress-threads", 4); > + > + migrate_set_capability(from, "compress", true); > + migrate_set_capability(to, "compress", true); > + > + return NULL; > +} Independently of this patch, we need to change this test to use 4 compression tests and 3 decompression or anything that is not the same number in both sides. I was complaining about this and when I arrived to the end of the path found that this was code movement. Later, Juan.
On Thu, 20 Apr 2023 12:20:25 +0200 Juan Quintela <quintela@redhat.com> wrote: > Lukas Straub <lukasstraub2@web.de> wrote: > > Add postcopy tests with compress enabled to ensure nothing breaks > > with the refactoring in the next commits. > > > > preempt+compress is blocked, so no test needed for that case. > > > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> > > Reviewed-by: Juan Quintela <quintela@redhat.com> > > And I wanted to removed the old compression code and it gets new users. Sniff. Who know how many compress threads users are out there... By the way, I'm not against deprecating compress threads in the long run. I'm already working on (cleanly :)) adding colo support with multifd. > > --- > > tests/qtest/migration-test.c | 83 +++++++++++++++++++++++------------- > > 1 file changed, 53 insertions(+), 30 deletions(-) > > > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > > index 1f2a019ce0..930cb4f29d 100644 > > --- a/tests/qtest/migration-test.c > > +++ b/tests/qtest/migration-test.c > > @@ -1127,6 +1127,36 @@ test_migrate_tls_x509_finish(QTestState *from, > > #endif /* CONFIG_TASN1 */ > > #endif /* CONFIG_GNUTLS */ > > > > +static void * > > +test_migrate_compress_start(QTestState *from, > > + QTestState *to) > > +{ > > + migrate_set_parameter_int(from, "compress-level", 1); > > + migrate_set_parameter_int(from, "compress-threads", 4); > > + migrate_set_parameter_bool(from, "compress-wait-thread", true); > > + migrate_set_parameter_int(to, "decompress-threads", 4); > > + > > + migrate_set_capability(from, "compress", true); > > + migrate_set_capability(to, "compress", true); > > + > > + return NULL; > > +} > > Independently of this patch, we need to change this test to use 4 > compression tests and 3 decompression or anything that is not the same > number in both sides. > > I was complaining about this and when I arrived to the end of the path > found that this was code movement. > > Later, Juan. > Oops, forgot to mention, the test is based on this patch https://lore.kernel.org/qemu-devel/2f4abb67cf5f3e1591b2666676462a93bdd20bbc.1680618040.git.lukasstraub2@web.de/ Will probably carry the patch with this series then. So you mean 4 compress _threads_ and 3 decompress _threads_? --
Lukas Straub <lukasstraub2@web.de> wrote: > On Thu, 20 Apr 2023 12:20:25 +0200 > Juan Quintela <quintela@redhat.com> wrote: > >> Lukas Straub <lukasstraub2@web.de> wrote: >> > Add postcopy tests with compress enabled to ensure nothing breaks >> > with the refactoring in the next commits. >> > >> > preempt+compress is blocked, so no test needed for that case. >> > >> > Signed-off-by: Lukas Straub <lukasstraub2@web.de> >> >> Reviewed-by: Juan Quintela <quintela@redhat.com> >> >> And I wanted to removed the old compression code and it gets new users. Sniff. > > Who know how many compress threads users are out there... Not too much. We broke it during development and nobody found it. And the reason that I wrote the multifd-zlib compression code was because I was not able to get a migration-test working with compression, so .... > By the way, I'm not against deprecating compress threads in the long > run. I'm already working on (cleanly :)) adding colo support with > multifd. Ok, then I will still put the deprecate comment there. >> > --- >> > tests/qtest/migration-test.c | 83 +++++++++++++++++++++++------------- >> > 1 file changed, 53 insertions(+), 30 deletions(-) >> > >> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c >> > index 1f2a019ce0..930cb4f29d 100644 >> > --- a/tests/qtest/migration-test.c >> > +++ b/tests/qtest/migration-test.c >> > @@ -1127,6 +1127,36 @@ test_migrate_tls_x509_finish(QTestState *from, >> > #endif /* CONFIG_TASN1 */ >> > #endif /* CONFIG_GNUTLS */ >> > >> > +static void * >> > +test_migrate_compress_start(QTestState *from, >> > + QTestState *to) >> > +{ >> > + migrate_set_parameter_int(from, "compress-level", 1); >> > + migrate_set_parameter_int(from, "compress-threads", 4); >> > + migrate_set_parameter_bool(from, "compress-wait-thread", true); >> > + migrate_set_parameter_int(to, "decompress-threads", 4); >> > + >> > + migrate_set_capability(from, "compress", true); >> > + migrate_set_capability(to, "compress", true); >> > + >> > + return NULL; >> > +} >> >> Independently of this patch, we need to change this test to use 4 >> compression tests and 3 decompression or anything that is not the same >> number in both sides. >> >> I was complaining about this and when I arrived to the end of the path >> found that this was code movement. >> >> Later, Juan. >> > > Oops, forgot to mention, the test is based on this patch > https://lore.kernel.org/qemu-devel/2f4abb67cf5f3e1591b2666676462a93bdd20bbc.1680618040.git.lukasstraub2@web.de/ > > Will probably carry the patch with this series then. So you mean 4 > compress _threads_ and 3 decompress _threads_? Yeap. Later, Juan.
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index 1f2a019ce0..930cb4f29d 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -1127,6 +1127,36 @@ test_migrate_tls_x509_finish(QTestState *from, #endif /* CONFIG_TASN1 */ #endif /* CONFIG_GNUTLS */ +static void * +test_migrate_compress_start(QTestState *from, + QTestState *to) +{ + migrate_set_parameter_int(from, "compress-level", 1); + migrate_set_parameter_int(from, "compress-threads", 4); + migrate_set_parameter_bool(from, "compress-wait-thread", true); + migrate_set_parameter_int(to, "decompress-threads", 4); + + migrate_set_capability(from, "compress", true); + migrate_set_capability(to, "compress", true); + + return NULL; +} + +static void * +test_migrate_compress_nowait_start(QTestState *from, + QTestState *to) +{ + migrate_set_parameter_int(from, "compress-level", 9); + migrate_set_parameter_int(from, "compress-threads", 1); + migrate_set_parameter_bool(from, "compress-wait-thread", false); + migrate_set_parameter_int(to, "decompress-threads", 1); + + migrate_set_capability(from, "compress", true); + migrate_set_capability(to, "compress", true); + + return NULL; +} + static int migrate_postcopy_prepare(QTestState **from_ptr, QTestState **to_ptr, MigrateCommon *args) @@ -1204,6 +1234,15 @@ static void test_postcopy(void) test_postcopy_common(&args); } +static void test_postcopy_compress(void) +{ + MigrateCommon args = { + .start_hook = test_migrate_compress_start + }; + + test_postcopy_common(&args); +} + static void test_postcopy_preempt(void) { MigrateCommon args = { @@ -1305,6 +1344,15 @@ static void test_postcopy_recovery(void) test_postcopy_recovery_common(&args); } +static void test_postcopy_recovery_compress(void) +{ + MigrateCommon args = { + .start_hook = test_migrate_compress_start + }; + + test_postcopy_recovery_common(&args); +} + #ifdef CONFIG_GNUTLS static void test_postcopy_recovery_tls_psk(void) { @@ -1338,6 +1386,7 @@ static void test_postcopy_preempt_all(void) test_postcopy_recovery_common(&args); } + #endif static void test_baddest(void) @@ -1559,21 +1608,6 @@ static void test_precopy_unix_xbzrle(void) test_precopy_common(&args); } -static void * -test_migrate_compress_start(QTestState *from, - QTestState *to) -{ - migrate_set_parameter_int(from, "compress-level", 1); - migrate_set_parameter_int(from, "compress-threads", 4); - migrate_set_parameter_bool(from, "compress-wait-thread", true); - migrate_set_parameter_int(to, "decompress-threads", 4); - - migrate_set_capability(from, "compress", true); - migrate_set_capability(to, "compress", true); - - return NULL; -} - static void test_precopy_unix_compress(void) { g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); @@ -1591,21 +1625,6 @@ static void test_precopy_unix_compress(void) test_precopy_common(&args); } -static void * -test_migrate_compress_nowait_start(QTestState *from, - QTestState *to) -{ - migrate_set_parameter_int(from, "compress-level", 9); - migrate_set_parameter_int(from, "compress-threads", 1); - migrate_set_parameter_bool(from, "compress-wait-thread", false); - migrate_set_parameter_int(to, "decompress-threads", 1); - - migrate_set_capability(from, "compress", true); - migrate_set_capability(to, "compress", true); - - return NULL; -} - static void test_precopy_unix_compress_nowait(void) { g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); @@ -2604,8 +2623,12 @@ int main(int argc, char **argv) if (has_uffd) { qtest_add_func("/migration/postcopy/plain", test_postcopy); + qtest_add_func("/migration/postcopy/compress/plain", + test_postcopy_compress); qtest_add_func("/migration/postcopy/recovery/plain", test_postcopy_recovery); + qtest_add_func("/migration/postcopy/recovery/compress/plain", + test_postcopy_recovery_compress); qtest_add_func("/migration/postcopy/preempt/plain", test_postcopy_preempt); qtest_add_func("/migration/postcopy/preempt/recovery/plain", test_postcopy_preempt_recovery);
Add postcopy tests with compress enabled to ensure nothing breaks with the refactoring in the next commits. preempt+compress is blocked, so no test needed for that case. Signed-off-by: Lukas Straub <lukasstraub2@web.de> --- tests/qtest/migration-test.c | 83 +++++++++++++++++++++++------------- 1 file changed, 53 insertions(+), 30 deletions(-) -- 2.40.0