| Message ID | 20240525131241.378473-3-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix s390x flic migration and add some more qtests | expand |
Hi, On Sat, 25 May 2024 at 18:44, Nicholas Piggin <npiggin@gmail.com> wrote: > s390x is more stable now. Enable it. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > tests/qtest/migration-test.c | 12 ------------ > 1 file changed, 12 deletions(-) > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > index 94d5057857..7987faaded 100644 > --- a/tests/qtest/migration-test.c > +++ b/tests/qtest/migration-test.c > @@ -3428,16 +3428,6 @@ int main(int argc, char **argv) > migration_test_add("/migration/analyze-script", test_analyze_script); > #endif > > - /* > - * On s390x, the test seems to be touchy with TCG, perhaps due to race > - * conditions on dirty bits, so disable it there until the problems are > - * resolved. > - */ -> https://lists.nongnu.org/archive/html/qemu-devel/2024-05/msg04774.html * Above patch (not reviewed yet) adds comment about sporadic problems on s390x, and this patch says s390x is stable now? It'll help to mention in the commit log - what changed to make it stable in 1 day. * IIUC, this and the ppc64 patch above enable 'migration-test' for s390x and ppc64 platforms, when KVM is not available for them? ie. When running s390x & ppc64 migration-tests with TCG on non s390x or non-ppc64 machines, right? Maybe the commit message could say something to the effect of - enable s390x and ppc64 'migration-test' to run with TCG across platforms where KVM for s390x OR KVM for ppc64 is not available. > - if (g_str_equal(arch, "s390x") && !has_kvm) { > - g_test_message("Skipping tests: s390x host with KVM is required"); > - goto test_add_done; > - } > - > if (is_x86) { > migration_test_add("/migration/precopy/unix/suspend/live", > test_precopy_unix_suspend_live); > @@ -3619,8 +3609,6 @@ int main(int argc, char **argv) > test_vcpu_dirty_limit); > } > > -test_add_done: > - > ret = g_test_run(); > > g_assert_cmpint(ret, ==, 0); > -- * Otherwise, the change looks okay to enable 'migration-test' for s390x with TCG IIUC. Thank you. --- - Prasad
On Mon May 27, 2024 at 3:46 PM AEST, Prasad Pandit wrote: > Hi, > > On Sat, 25 May 2024 at 18:44, Nicholas Piggin <npiggin@gmail.com> wrote: > > s390x is more stable now. Enable it. > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > tests/qtest/migration-test.c | 12 ------------ > > 1 file changed, 12 deletions(-) > > > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > > index 94d5057857..7987faaded 100644 > > --- a/tests/qtest/migration-test.c > > +++ b/tests/qtest/migration-test.c > > @@ -3428,16 +3428,6 @@ int main(int argc, char **argv) > > migration_test_add("/migration/analyze-script", test_analyze_script); > > #endif > > > > - /* > > - * On s390x, the test seems to be touchy with TCG, perhaps due to race > > - * conditions on dirty bits, so disable it there until the problems are > > - * resolved. > > - */ > > -> https://lists.nongnu.org/archive/html/qemu-devel/2024-05/msg04774.html > > * Above patch (not reviewed yet) adds comment about sporadic problems > on s390x, and this patch says s390x is stable now? It'll help to > mention in the commit log - what changed to make it stable in 1 day. Patch 1 of this series. > * IIUC, this and the ppc64 patch above enable 'migration-test' for > s390x and ppc64 platforms, when KVM is not available for them? ie. > When running s390x & ppc64 migration-tests with TCG on non s390x or > non-ppc64 machines, right? Maybe the commit message could say > something to the effect of - enable s390x and ppc64 'migration-test' > to run with TCG across platforms where KVM for s390x OR KVM for > ppc64 is not available. Yes they should be called "enable for TCG" indeed. > > - if (g_str_equal(arch, "s390x") && !has_kvm) { > > - g_test_message("Skipping tests: s390x host with KVM is required"); > > - goto test_add_done; > > - } > > - > > if (is_x86) { > > migration_test_add("/migration/precopy/unix/suspend/live", > > test_precopy_unix_suspend_live); > > @@ -3619,8 +3609,6 @@ int main(int argc, char **argv) > > test_vcpu_dirty_limit); > > } > > > > -test_add_done: > > - > > ret = g_test_run(); > > > > g_assert_cmpint(ret, ==, 0); > > -- > > * Otherwise, the change looks okay to enable 'migration-test' for > s390x with TCG IIUC. Thanks, Nick
On Mon, 27 May 2024 at 12:34, Nicholas Piggin <npiggin@gmail.com> wrote: > > * Above patch (not reviewed yet) adds comment about sporadic problems > > on s390x, and this patch says s390x is stable now? It'll help to > > mention in the commit log - what changed to make it stable in 1 day. > > Patch 1 of this series. * Ah, that one got filtered to another folder. It'll help to add something about the 'flic pending' case being specific to migration of TCG device and so it's reasonable to remove 'has_kvm' check and enable migration-test to run with TCG. > Yes they should be called "enable for TCG" indeed. > Ack with commit message changes: Reviewed-by: Prasad Pandit <pjp@fedoraproject.org> Thank you. --- - Prasad
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index 94d5057857..7987faaded 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -3428,16 +3428,6 @@ int main(int argc, char **argv) migration_test_add("/migration/analyze-script", test_analyze_script); #endif - /* - * On s390x, the test seems to be touchy with TCG, perhaps due to race - * conditions on dirty bits, so disable it there until the problems are - * resolved. - */ - if (g_str_equal(arch, "s390x") && !has_kvm) { - g_test_message("Skipping tests: s390x host with KVM is required"); - goto test_add_done; - } - if (is_x86) { migration_test_add("/migration/precopy/unix/suspend/live", test_precopy_unix_suspend_live); @@ -3619,8 +3609,6 @@ int main(int argc, char **argv) test_vcpu_dirty_limit); } -test_add_done: - ret = g_test_run(); g_assert_cmpint(ret, ==, 0);
s390x is more stable now. Enable it. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- tests/qtest/migration-test.c | 12 ------------ 1 file changed, 12 deletions(-)