| Message ID | 1479285571-28145-1-git-send-email-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 16/11/2016 09:39, Thomas Huth wrote: > The ppc64 postcopy test does not work with KVM-PR, and it is also > causing annoying warning messages when run on a x86 host. So let's > use KVM here only if we know that we're running with KVM-HV (which > automatically also means that we're running on a ppc64 host), and > fall back to TCG otherwise. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > tests/postcopy-test.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c > index d6613c5..dafe8be 100644 > --- a/tests/postcopy-test.c > +++ b/tests/postcopy-test.c > @@ -380,17 +380,21 @@ static void test_migrate(void) > " -incoming %s", > tmpfs, bootpath, uri); > } else if (strcmp(arch, "ppc64") == 0) { > + const char *accel; > + > + /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */ > + accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg"; why not "kvm" instead of "kvm:tcg"? If it doesn't work it should fail. Laurent
On 16.11.2016 10:19, Laurent Vivier wrote: > > > On 16/11/2016 09:39, Thomas Huth wrote: >> The ppc64 postcopy test does not work with KVM-PR, and it is also >> causing annoying warning messages when run on a x86 host. So let's >> use KVM here only if we know that we're running with KVM-HV (which >> automatically also means that we're running on a ppc64 host), and >> fall back to TCG otherwise. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> tests/postcopy-test.c | 12 ++++++++---- >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c >> index d6613c5..dafe8be 100644 >> --- a/tests/postcopy-test.c >> +++ b/tests/postcopy-test.c >> @@ -380,17 +380,21 @@ static void test_migrate(void) >> " -incoming %s", >> tmpfs, bootpath, uri); >> } else if (strcmp(arch, "ppc64") == 0) { >> + const char *accel; >> + >> + /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */ >> + accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg"; > > why not "kvm" instead of "kvm:tcg"? > If it doesn't work it should fail. Yes, sounds right. I'll send a v2... Thomas
On 16/11/2016 09:39, Thomas Huth wrote: > The ppc64 postcopy test does not work with KVM-PR, and it is also > causing annoying warning messages when run on a x86 host. So let's > use KVM here only if we know that we're running with KVM-HV (which > automatically also means that we're running on a ppc64 host), and > fall back to TCG otherwise. > > Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Laurent Vivier <lvivier@redhat.com> > --- > tests/postcopy-test.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c > index d6613c5..dafe8be 100644 > --- a/tests/postcopy-test.c > +++ b/tests/postcopy-test.c > @@ -380,17 +380,21 @@ static void test_migrate(void) > " -incoming %s", > tmpfs, bootpath, uri); > } else if (strcmp(arch, "ppc64") == 0) { > + const char *accel; > + > + /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */ > + accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg"; > init_bootfile_ppc(bootpath); > - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M" > + cmd_src = g_strdup_printf("-machine accel=%s -m 256M" > " -name pcsource,debug-threads=on" > " -serial file:%s/src_serial" > " -drive file=%s,if=pflash,format=raw", > - tmpfs, bootpath); > - cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M" > + accel, tmpfs, bootpath); > + cmd_dst = g_strdup_printf("-machine accel=%s -m 256M" > " -name pcdest,debug-threads=on" > " -serial file:%s/dest_serial" > " -incoming %s", > - tmpfs, uri); > + accel, tmpfs, uri); > } else { > g_assert_not_reached(); > } >
On Wed, 16 Nov 2016 09:39:31 +0100 Thomas Huth <thuth@redhat.com> wrote: > The ppc64 postcopy test does not work with KVM-PR, and it is also > causing annoying warning messages when run on a x86 host. So let's > use KVM here only if we know that we're running with KVM-HV (which > automatically also means that we're running on a ppc64 host), and > fall back to TCG otherwise. > This patch addresses two issues actually: - the annoying warning when running on a ppc64 guest on a non-ppc64 host - the fact that KVM-PR seems to be currently broken I agree that the former makes sense, but what about the case of running a x86 guest on a non-x86 host ? I'm still feeling uncomfortable with the KVM-PR case... is this a workaround we want to keep until we find out what's going on or are we starting to partially deprecate KVM PR ? In any case, I guess we should document this and probably print some meaningful error message. > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > tests/postcopy-test.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c > index d6613c5..dafe8be 100644 > --- a/tests/postcopy-test.c > +++ b/tests/postcopy-test.c > @@ -380,17 +380,21 @@ static void test_migrate(void) > " -incoming %s", > tmpfs, bootpath, uri); > } else if (strcmp(arch, "ppc64") == 0) { > + const char *accel; > + > + /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */ > + accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg"; > init_bootfile_ppc(bootpath); > - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M" > + cmd_src = g_strdup_printf("-machine accel=%s -m 256M" > " -name pcsource,debug-threads=on" > " -serial file:%s/src_serial" > " -drive file=%s,if=pflash,format=raw", > - tmpfs, bootpath); > - cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M" > + accel, tmpfs, bootpath); > + cmd_dst = g_strdup_printf("-machine accel=%s -m 256M" > " -name pcdest,debug-threads=on" > " -serial file:%s/dest_serial" > " -incoming %s", > - tmpfs, uri); > + accel, tmpfs, uri); > } else { > g_assert_not_reached(); > }
* Greg Kurz (groug@kaod.org) wrote: > On Wed, 16 Nov 2016 09:39:31 +0100 > Thomas Huth <thuth@redhat.com> wrote: > > > The ppc64 postcopy test does not work with KVM-PR, and it is also > > causing annoying warning messages when run on a x86 host. So let's > > use KVM here only if we know that we're running with KVM-HV (which > > automatically also means that we're running on a ppc64 host), and > > fall back to TCG otherwise. > > > > This patch addresses two issues actually: > - the annoying warning when running on a ppc64 guest on a non-ppc64 host > - the fact that KVM-PR seems to be currently broken > > I agree that the former makes sense, but what about the case of running > a x86 guest on a non-x86 host ? > > I'm still feeling uncomfortable with the KVM-PR case... is this a workaround > we want to keep until we find out what's going on or are we starting to > partially deprecate KVM PR ? In any case, I guess we should document this > and probably print some meaningful error message. This is certainly a work around for now, it doesn't suggest anything about deprecation. Dave > > Signed-off-by: Thomas Huth <thuth@redhat.com> > > --- > > tests/postcopy-test.c | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c > > index d6613c5..dafe8be 100644 > > --- a/tests/postcopy-test.c > > +++ b/tests/postcopy-test.c > > @@ -380,17 +380,21 @@ static void test_migrate(void) > > " -incoming %s", > > tmpfs, bootpath, uri); > > } else if (strcmp(arch, "ppc64") == 0) { > > + const char *accel; > > + > > + /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */ > > + accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg"; > > init_bootfile_ppc(bootpath); > > - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M" > > + cmd_src = g_strdup_printf("-machine accel=%s -m 256M" > > " -name pcsource,debug-threads=on" > > " -serial file:%s/src_serial" > > " -drive file=%s,if=pflash,format=raw", > > - tmpfs, bootpath); > > - cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M" > > + accel, tmpfs, bootpath); > > + cmd_dst = g_strdup_printf("-machine accel=%s -m 256M" > > " -name pcdest,debug-threads=on" > > " -serial file:%s/dest_serial" > > " -incoming %s", > > - tmpfs, uri); > > + accel, tmpfs, uri); > > } else { > > g_assert_not_reached(); > > } > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Wed, 16 Nov 2016 12:24:50 +0000 "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Greg Kurz (groug@kaod.org) wrote: > > On Wed, 16 Nov 2016 09:39:31 +0100 > > Thomas Huth <thuth@redhat.com> wrote: > > > > > The ppc64 postcopy test does not work with KVM-PR, and it is also > > > causing annoying warning messages when run on a x86 host. So let's > > > use KVM here only if we know that we're running with KVM-HV (which > > > automatically also means that we're running on a ppc64 host), and > > > fall back to TCG otherwise. > > > > > > > This patch addresses two issues actually: > > - the annoying warning when running on a ppc64 guest on a non-ppc64 host > > - the fact that KVM-PR seems to be currently broken > > > > I agree that the former makes sense, but what about the case of running > > a x86 guest on a non-x86 host ? > > > > I'm still feeling uncomfortable with the KVM-PR case... is this a workaround > > we want to keep until we find out what's going on or are we starting to > > partially deprecate KVM PR ? In any case, I guess we should document this > > and probably print some meaningful error message. > > This is certainly a work around for now, it doesn't suggest anything about > deprecation. > Well it doesn't suggest anything actually, it just silently skips KVM PR... I would at least expect a comment in the code mentioning this is a workaround and maybe an explicit warning for the user. If the user really wants to run this test with KVM on ppc64, then she should ensure it is KVM HV. Cheers. -- Greg > Dave > > > > Signed-off-by: Thomas Huth <thuth@redhat.com> > > > --- > > > tests/postcopy-test.c | 12 ++++++++---- > > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > > > diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c > > > index d6613c5..dafe8be 100644 > > > --- a/tests/postcopy-test.c > > > +++ b/tests/postcopy-test.c > > > @@ -380,17 +380,21 @@ static void test_migrate(void) > > > " -incoming %s", > > > tmpfs, bootpath, uri); > > > } else if (strcmp(arch, "ppc64") == 0) { > > > + const char *accel; > > > + > > > + /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */ > > > + accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg"; > > > init_bootfile_ppc(bootpath); > > > - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M" > > > + cmd_src = g_strdup_printf("-machine accel=%s -m 256M" > > > " -name pcsource,debug-threads=on" > > > " -serial file:%s/src_serial" > > > " -drive file=%s,if=pflash,format=raw", > > > - tmpfs, bootpath); > > > - cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M" > > > + accel, tmpfs, bootpath); > > > + cmd_dst = g_strdup_printf("-machine accel=%s -m 256M" > > > " -name pcdest,debug-threads=on" > > > " -serial file:%s/dest_serial" > > > " -incoming %s", > > > - tmpfs, uri); > > > + accel, tmpfs, uri); > > > } else { > > > g_assert_not_reached(); > > > } > > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 16.11.2016 13:37, Greg Kurz wrote: > On Wed, 16 Nov 2016 12:24:50 +0000 > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > >> * Greg Kurz (groug@kaod.org) wrote: >>> On Wed, 16 Nov 2016 09:39:31 +0100 >>> Thomas Huth <thuth@redhat.com> wrote: >>> >>>> The ppc64 postcopy test does not work with KVM-PR, and it is also >>>> causing annoying warning messages when run on a x86 host. So let's >>>> use KVM here only if we know that we're running with KVM-HV (which >>>> automatically also means that we're running on a ppc64 host), and >>>> fall back to TCG otherwise. >>>> >>> >>> This patch addresses two issues actually: >>> - the annoying warning when running on a ppc64 guest on a non-ppc64 host >>> - the fact that KVM-PR seems to be currently broken >>> >>> I agree that the former makes sense, but what about the case of running >>> a x86 guest on a non-x86 host ? Of course you also get these '"kvm" accelerator not found' messages there. But so far, I think nobody complained about that yet (only for ppc64 running on x86). And at least the test succeeds there - unlike with KVM-PR, where the test fails completely. >>> I'm still feeling uncomfortable with the KVM-PR case... is this a workaround >>> we want to keep until we find out what's going on or are we starting to >>> partially deprecate KVM PR ? In any case, I guess we should document this >>> and probably print some meaningful error message. >> >> This is certainly a work around for now, it doesn't suggest anything about >> deprecation. > > Well it doesn't suggest anything actually, it just silently skips KVM PR... > I would at least expect a comment in the code mentioning this is a > workaround and maybe an explicit warning for the user. If the user really > wants to run this test with KVM on ppc64, then she should ensure it is > KVM HV. Honestly, also considering the number of patches that Laurent already wrote here and never have been accepted, all this has become quite an ugly bike-shed painting discussion. My opinion: - If we want to properly test KVM (be it KVM-HV or KVM-PR), write a proper kvm-unit-test instead. I.e. I personally don't care if this test in QEMU is only run with TCG or with KVM. - The current status of "make check" is broken, since it does not work on KVM-PR. We've got to fix that before the release. That means I currently really don't care if we've spill out a warning message for KVM-PR here or not - sure, somebody just got to look at KVM-PR later, but that's IMHO off-topic for the test here in the QEMU context. So if you think that the patch for fixing this issue here with the QEMU test should look differently, please propose a different patch instead. I'm fine with every other approach as long as we get this fixed in time for QEMU 2.8. Thomas
On Wed, 16 Nov 2016 14:17:47 +0100 Thomas Huth <thuth@redhat.com> wrote: > On 16.11.2016 13:37, Greg Kurz wrote: > > On Wed, 16 Nov 2016 12:24:50 +0000 > > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > > > >> * Greg Kurz (groug@kaod.org) wrote: > >>> On Wed, 16 Nov 2016 09:39:31 +0100 > >>> Thomas Huth <thuth@redhat.com> wrote: > >>> > >>>> The ppc64 postcopy test does not work with KVM-PR, and it is also > >>>> causing annoying warning messages when run on a x86 host. So let's > >>>> use KVM here only if we know that we're running with KVM-HV (which > >>>> automatically also means that we're running on a ppc64 host), and > >>>> fall back to TCG otherwise. > >>>> > >>> > >>> This patch addresses two issues actually: > >>> - the annoying warning when running on a ppc64 guest on a non-ppc64 host > >>> - the fact that KVM-PR seems to be currently broken > >>> > >>> I agree that the former makes sense, but what about the case of running > >>> a x86 guest on a non-x86 host ? > > Of course you also get these '"kvm" accelerator not found' messages > there. But so far, I think nobody complained about that yet (only for > ppc64 running on x86). And at least the test succeeds there - unlike > with KVM-PR, where the test fails completely. > > >>> I'm still feeling uncomfortable with the KVM-PR case... is this a workaround > >>> we want to keep until we find out what's going on or are we starting to > >>> partially deprecate KVM PR ? In any case, I guess we should document this > >>> and probably print some meaningful error message. > >> > >> This is certainly a work around for now, it doesn't suggest anything about > >> deprecation. > > > > Well it doesn't suggest anything actually, it just silently skips KVM PR... > > I would at least expect a comment in the code mentioning this is a > > workaround and maybe an explicit warning for the user. If the user really > > wants to run this test with KVM on ppc64, then she should ensure it is > > KVM HV. > > Honestly, also considering the number of patches that Laurent already > wrote here and never have been accepted, all this has become quite an > ugly bike-shed painting discussion. > Understood. I'm done with the trivial details ;) > My opinion: > > - If we want to properly test KVM (be it KVM-HV or KVM-PR), write > a proper kvm-unit-test instead. I.e. I personally don't care if this > test in QEMU is only run with TCG or with KVM. > Agreed. > - The current status of "make check" is broken, since it does not > work on KVM-PR. We've got to fix that before the release. > > That means I currently really don't care if we've spill out a warning > message for KVM-PR here or not - sure, somebody just got to look at > KVM-PR later, but that's IMHO off-topic for the test here in the QEMU > context. > > So if you think that the patch for fixing this issue here with the QEMU > test should look differently, please propose a different patch instead. > I'm fine with every other approach as long as we get this fixed in time > for QEMU 2.8. > The changes to the code look ok and I prefer to spend time chasing the KVM PR issue rather than arguing on a comment... Cheers. -- Greg > Thomas >
On Wed, 16 Nov 2016 09:39:31 +0100 Thomas Huth <thuth@redhat.com> wrote: > The ppc64 postcopy test does not work with KVM-PR, and it is also > causing annoying warning messages when run on a x86 host. So let's > use KVM here only if we know that we're running with KVM-HV (which > automatically also means that we're running on a ppc64 host), and > fall back to TCG otherwise. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- FWIW Reviewed-by: Greg Kurz <groug@kaod.org> > tests/postcopy-test.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c > index d6613c5..dafe8be 100644 > --- a/tests/postcopy-test.c > +++ b/tests/postcopy-test.c > @@ -380,17 +380,21 @@ static void test_migrate(void) > " -incoming %s", > tmpfs, bootpath, uri); > } else if (strcmp(arch, "ppc64") == 0) { > + const char *accel; > + > + /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */ > + accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg"; > init_bootfile_ppc(bootpath); > - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M" > + cmd_src = g_strdup_printf("-machine accel=%s -m 256M" > " -name pcsource,debug-threads=on" > " -serial file:%s/src_serial" > " -drive file=%s,if=pflash,format=raw", > - tmpfs, bootpath); > - cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M" > + accel, tmpfs, bootpath); > + cmd_dst = g_strdup_printf("-machine accel=%s -m 256M" > " -name pcdest,debug-threads=on" > " -serial file:%s/dest_serial" > " -incoming %s", > - tmpfs, uri); > + accel, tmpfs, uri); > } else { > g_assert_not_reached(); > }
On Wed, Nov 16, 2016 at 02:17:47PM +0100, Thomas Huth wrote: > On 16.11.2016 13:37, Greg Kurz wrote: > > On Wed, 16 Nov 2016 12:24:50 +0000 > > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > > > >> * Greg Kurz (groug@kaod.org) wrote: > >>> On Wed, 16 Nov 2016 09:39:31 +0100 > >>> Thomas Huth <thuth@redhat.com> wrote: > >>> > >>>> The ppc64 postcopy test does not work with KVM-PR, and it is also > >>>> causing annoying warning messages when run on a x86 host. So let's > >>>> use KVM here only if we know that we're running with KVM-HV (which > >>>> automatically also means that we're running on a ppc64 host), and > >>>> fall back to TCG otherwise. > >>>> > >>> > >>> This patch addresses two issues actually: > >>> - the annoying warning when running on a ppc64 guest on a non-ppc64 host > >>> - the fact that KVM-PR seems to be currently broken > >>> > >>> I agree that the former makes sense, but what about the case of running > >>> a x86 guest on a non-x86 host ? > > Of course you also get these '"kvm" accelerator not found' messages > there. But so far, I think nobody complained about that yet (only for > ppc64 running on x86). And at least the test succeeds there - unlike > with KVM-PR, where the test fails completely. Well, I guess I should complain about them then. It is slightly irritating when doing my pre-pull tests on a ppc64 host, although I'm more or less used to it now. > >>> I'm still feeling uncomfortable with the KVM-PR case... is this a workaround > >>> we want to keep until we find out what's going on or are we starting to > >>> partially deprecate KVM PR ? In any case, I guess we should document this > >>> and probably print some meaningful error message. > >> > >> This is certainly a work around for now, it doesn't suggest anything about > >> deprecation. > > > > Well it doesn't suggest anything actually, it just silently skips KVM PR... > > I would at least expect a comment in the code mentioning this is a > > workaround and maybe an explicit warning for the user. If the user really > > wants to run this test with KVM on ppc64, then she should ensure it is > > KVM HV. > > Honestly, also considering the number of patches that Laurent already > wrote here and never have been accepted, all this has become quite an > ugly bike-shed painting discussion. > > My opinion: > > - If we want to properly test KVM (be it KVM-HV or KVM-PR), write > a proper kvm-unit-test instead. I.e. I personally don't care if this > test in QEMU is only run with TCG or with KVM. > > - The current status of "make check" is broken, since it does not > work on KVM-PR. We've got to fix that before the release. > > That means I currently really don't care if we've spill out a warning > message for KVM-PR here or not - sure, somebody just got to look at > KVM-PR later, but that's IMHO off-topic for the test here in the QEMU > context. > > So if you think that the patch for fixing this issue here with the QEMU > test should look differently, please propose a different patch instead. > I'm fine with every other approach as long as we get this fixed in time > for QEMU 2.8. Hm, yeah, I concur.x > > Thomas >
On Wed, Nov 16, 2016 at 09:39:31AM +0100, Thomas Huth wrote: > The ppc64 postcopy test does not work with KVM-PR, and it is also > causing annoying warning messages when run on a x86 host. So let's > use KVM here only if we know that we're running with KVM-HV (which > automatically also means that we're running on a ppc64 host), and > fall back to TCG otherwise. > > Signed-off-by: Thomas Huth <thuth@redhat.com> Applied to ppc-for-2.8. Longer term, I think we should default to tcg for all these tests - on x86 as well - then run KVM *as well* when available. But in the short term we should fix make check for the 2.8 release. > --- > tests/postcopy-test.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c > index d6613c5..dafe8be 100644 > --- a/tests/postcopy-test.c > +++ b/tests/postcopy-test.c > @@ -380,17 +380,21 @@ static void test_migrate(void) > " -incoming %s", > tmpfs, bootpath, uri); > } else if (strcmp(arch, "ppc64") == 0) { > + const char *accel; > + > + /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */ > + accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg"; > init_bootfile_ppc(bootpath); > - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M" > + cmd_src = g_strdup_printf("-machine accel=%s -m 256M" > " -name pcsource,debug-threads=on" > " -serial file:%s/src_serial" > " -drive file=%s,if=pflash,format=raw", > - tmpfs, bootpath); > - cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M" > + accel, tmpfs, bootpath); > + cmd_dst = g_strdup_printf("-machine accel=%s -m 256M" > " -name pcdest,debug-threads=on" > " -serial file:%s/dest_serial" > " -incoming %s", > - tmpfs, uri); > + accel, tmpfs, uri); > } else { > g_assert_not_reached(); > }
On 17/11/2016 04:18, David Gibson wrote: > On Wed, Nov 16, 2016 at 09:39:31AM +0100, Thomas Huth wrote: >> The ppc64 postcopy test does not work with KVM-PR, and it is also >> causing annoying warning messages when run on a x86 host. So let's >> use KVM here only if we know that we're running with KVM-HV (which >> automatically also means that we're running on a ppc64 host), and >> fall back to TCG otherwise. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> > > Applied to ppc-for-2.8. > > Longer term, I think we should default to tcg for all these tests - on > x86 as well - then run KVM *as well* when available. But in the short > term we should fix make check for the 2.8 release. I agree with that. Laurent
On 16/11/2016 15:17, Greg Kurz wrote: > On Wed, 16 Nov 2016 14:17:47 +0100 > Thomas Huth <thuth@redhat.com> wrote: > >> On 16.11.2016 13:37, Greg Kurz wrote: >>> On Wed, 16 Nov 2016 12:24:50 +0000 >>> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: >>> >>>> * Greg Kurz (groug@kaod.org) wrote: >>>>> On Wed, 16 Nov 2016 09:39:31 +0100 >>>>> Thomas Huth <thuth@redhat.com> wrote: >>>>> >>>>>> The ppc64 postcopy test does not work with KVM-PR, and it is also >>>>>> causing annoying warning messages when run on a x86 host. So let's >>>>>> use KVM here only if we know that we're running with KVM-HV (which >>>>>> automatically also means that we're running on a ppc64 host), and >>>>>> fall back to TCG otherwise. >>>>>> [..] > The changes to the code look ok and I prefer to spend time chasing the > KVM PR issue rather than arguing on a comment... For the problem itself, it seems to appear only after a BOOK3S_INTERRUPT_SYSCALL interrupt for an KVM_EXIT_PAPR_HCALL (H_PUT_TERM_CHAR). In this case, KVM has to exit to QEMU to manage the output. The following interrupt is always an BOOK3S_INTERRUPT_PROGRAM with an emulation failure. Laurent
Hi Laurent, On Thu, 17 Nov 2016 21:22:33 +0100 Laurent Vivier <lvivier@redhat.com> wrote: > On 16/11/2016 15:17, Greg Kurz wrote: > > On Wed, 16 Nov 2016 14:17:47 +0100 > > Thomas Huth <thuth@redhat.com> wrote: > > > >> On 16.11.2016 13:37, Greg Kurz wrote: > >>> On Wed, 16 Nov 2016 12:24:50 +0000 > >>> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > >>> > >>>> * Greg Kurz (groug@kaod.org) wrote: > >>>>> On Wed, 16 Nov 2016 09:39:31 +0100 > >>>>> Thomas Huth <thuth@redhat.com> wrote: > >>>>> > >>>>>> The ppc64 postcopy test does not work with KVM-PR, and it is also > >>>>>> causing annoying warning messages when run on a x86 host. So let's > >>>>>> use KVM here only if we know that we're running with KVM-HV (which > >>>>>> automatically also means that we're running on a ppc64 host), and > >>>>>> fall back to TCG otherwise. > >>>>>> > [..] > > The changes to the code look ok and I prefer to spend time chasing the > > KVM PR issue rather than arguing on a comment... > > For the problem itself, it seems to appear only after a > BOOK3S_INTERRUPT_SYSCALL interrupt for an KVM_EXIT_PAPR_HCALL > (H_PUT_TERM_CHAR). In this case, KVM has to exit to QEMU to manage the > output. The following interrupt is always an BOOK3S_INTERRUPT_PROGRAM > with an emulation failure. > Which specific problem are you referring to ? On my side, when running postcopy-test in a nested guest, I hit either one of the three following issues (in decreasing order of probability of occurence): 1) "Memory content inconsistency at ..." like Stefan 2) "Unexpected 32 on dest_serial serial" accompanied by the following in dmesg [131613.428616] Couldn't emulate instruction 0x00000000 (op 0 xop 0) [131613.503515] kvmppc_handle_exit_pr: emulation at d8 failed (00000000) 3) hang because the destination QEMU is looping on: ioctl(19, KVM_RUN, 0) = 2 (RESUME_HOST) Host runs OpenPower HostOS (kernel 4.9, QEMU 2.7) and guest runs fedora25. Cheers. -- Greg > Laurent
On 18/11/2016 13:53, Greg Kurz wrote: > Hi Laurent, Hi Greg, > On Thu, 17 Nov 2016 21:22:33 +0100 > Laurent Vivier <lvivier@redhat.com> wrote: > >> On 16/11/2016 15:17, Greg Kurz wrote: >>> On Wed, 16 Nov 2016 14:17:47 +0100 >>> Thomas Huth <thuth@redhat.com> wrote: >>> >>>> On 16.11.2016 13:37, Greg Kurz wrote: >>>>> On Wed, 16 Nov 2016 12:24:50 +0000 >>>>> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: >>>>> >>>>>> * Greg Kurz (groug@kaod.org) wrote: >>>>>>> On Wed, 16 Nov 2016 09:39:31 +0100 >>>>>>> Thomas Huth <thuth@redhat.com> wrote: >>>>>>> >>>>>>>> The ppc64 postcopy test does not work with KVM-PR, and it is also >>>>>>>> causing annoying warning messages when run on a x86 host. So let's >>>>>>>> use KVM here only if we know that we're running with KVM-HV (which >>>>>>>> automatically also means that we're running on a ppc64 host), and >>>>>>>> fall back to TCG otherwise. >>>>>>>> >> [..] >>> The changes to the code look ok and I prefer to spend time chasing the >>> KVM PR issue rather than arguing on a comment... >> >> For the problem itself, it seems to appear only after a >> BOOK3S_INTERRUPT_SYSCALL interrupt for an KVM_EXIT_PAPR_HCALL >> (H_PUT_TERM_CHAR). In this case, KVM has to exit to QEMU to manage the >> output. The following interrupt is always an BOOK3S_INTERRUPT_PROGRAM >> with an emulation failure. >> > > Which specific problem are you referring to ? .. > 2) "Unexpected 32 on dest_serial serial" accompanied by the following in dmesg > > [131613.428616] Couldn't emulate instruction 0x00000000 (op 0 xop 0) > [131613.503515] kvmppc_handle_exit_pr: emulation at d8 failed (00000000) This one, test running on a bare metal PowerMac G5 (F24), 4.9.0-rc5 kernel. And I have only and everytime this error. Laurent
diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c index d6613c5..dafe8be 100644 --- a/tests/postcopy-test.c +++ b/tests/postcopy-test.c @@ -380,17 +380,21 @@ static void test_migrate(void) " -incoming %s", tmpfs, bootpath, uri); } else if (strcmp(arch, "ppc64") == 0) { + const char *accel; + + /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */ + accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg"; init_bootfile_ppc(bootpath); - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M" + cmd_src = g_strdup_printf("-machine accel=%s -m 256M" " -name pcsource,debug-threads=on" " -serial file:%s/src_serial" " -drive file=%s,if=pflash,format=raw", - tmpfs, bootpath); - cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M" + accel, tmpfs, bootpath); + cmd_dst = g_strdup_printf("-machine accel=%s -m 256M" " -name pcdest,debug-threads=on" " -serial file:%s/dest_serial" " -incoming %s", - tmpfs, uri); + accel, tmpfs, uri); } else { g_assert_not_reached(); }
The ppc64 postcopy test does not work with KVM-PR, and it is also causing annoying warning messages when run on a x86 host. So let's use KVM here only if we know that we're running with KVM-HV (which automatically also means that we're running on a ppc64 host), and fall back to TCG otherwise. Signed-off-by: Thomas Huth <thuth@redhat.com> --- tests/postcopy-test.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)