tests/qtest/migration-test.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
The TAP protocol version line must be the first thing printed on
stdout. The migration test failed that requirement in certain
scenarios:
# Skipping test: Userfault not available (builtdtime)
TAP version 13
# random seed: R02Sc120c807f11053eb90bfea845ba1e368
1..32
# Start of x86_64 tests
# Start of migration tests
....
The TAP version is printed by g_test_init(), so we need to make
sure that any methods which print are run after that.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/migration-test.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 75d4f1d4a9..3b615b0da9 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2462,14 +2462,18 @@ static bool kvm_dirty_ring_supported(void)
int main(int argc, char **argv)
{
- const bool has_kvm = qtest_has_accel("kvm");
- const bool has_uffd = ufd_version_check();
- const char *arch = qtest_get_arch();
+ bool has_kvm;
+ bool has_uffd;
+ const char *arch;
g_autoptr(GError) err = NULL;
int ret;
g_test_init(&argc, &argv, NULL);
+ has_kvm = qtest_has_accel("kvm");
+ has_uffd = ufd_version_check();
+ arch = qtest_get_arch();
+
/*
* On ppc64, the test only works with kvm-hv, but not with kvm-pr and TCG
* is touchy due to race conditions on dirty bits (especially on PPC for
--
2.39.2
Daniel P. Berrangé <berrange@redhat.com> wrote: > The TAP protocol version line must be the first thing printed on > stdout. The migration test failed that requirement in certain > scenarios: > > # Skipping test: Userfault not available (builtdtime) > TAP version 13 > # random seed: R02Sc120c807f11053eb90bfea845ba1e368 > 1..32 > # Start of x86_64 tests > # Start of migration tests > .... > > The TAP version is printed by g_test_init(), so we need to make > sure that any methods which print are run after that. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- Reviewed-by: Juan Quintela <quintela@redhat.com> > - const bool has_kvm = qtest_has_accel("kvm"); > - const bool has_uffd = ufd_version_check(); > - const char *arch = qtest_get_arch(); > + bool has_kvm; > + bool has_uffd; > + const char *arch; Why don't you move also the declarations of the variables? I think that one of the biggest troubles of C is variables that are not initialized. All compilers that we support are C99 or later, so we can do that (and we already do in lot of places.) And yeap, I know that CodingStyle says otherwise, but I think that what is wrong is CodingStyle. https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg03836.html Later, Juan.
On 23/03/2023 19.31, Juan Quintela wrote: > Daniel P. Berrangé <berrange@redhat.com> wrote: >> The TAP protocol version line must be the first thing printed on >> stdout. The migration test failed that requirement in certain >> scenarios: >> >> # Skipping test: Userfault not available (builtdtime) >> TAP version 13 >> # random seed: R02Sc120c807f11053eb90bfea845ba1e368 >> 1..32 >> # Start of x86_64 tests >> # Start of migration tests >> .... >> >> The TAP version is printed by g_test_init(), so we need to make >> sure that any methods which print are run after that. >> >> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> >> --- > > Reviewed-by: Juan Quintela <quintela@redhat.com> > >> - const bool has_kvm = qtest_has_accel("kvm"); >> - const bool has_uffd = ufd_version_check(); >> - const char *arch = qtest_get_arch(); >> + bool has_kvm; >> + bool has_uffd; >> + const char *arch; > > Why don't you move also the declarations of the variables? > I think that one of the biggest troubles of C is variables that are not > initialized. > > All compilers that we support are C99 or later, so we can do that (and > we already do in lot of places.) I think the coding style has been created before we switched to -std=gnu99 for compiling QEMU, so a lot of GCCs were still using C89 by default? > And yeap, I know that CodingStyle says otherwise, but I think that what > is wrong is CodingStyle. > > https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg03836.html Please use proper prefixes in the subject when sending patches ("docs/devel:" here), otherwise your patches might not get the right attention (at least on my side, it was filtered away as a patch that was relevant to me) - and also put some recent contributors on CC: Thanks, Thomas
Thomas Huth <thuth@redhat.com> wrote: > On 23/03/2023 19.31, Juan Quintela wrote: >> Daniel P. Berrangé <berrange@redhat.com> wrote: >>> The TAP protocol version line must be the first thing printed on >>> stdout. The migration test failed that requirement in certain >>> scenarios: >>> >>> # Skipping test: Userfault not available (builtdtime) >>> TAP version 13 >>> # random seed: R02Sc120c807f11053eb90bfea845ba1e368 >>> 1..32 >>> # Start of x86_64 tests >>> # Start of migration tests >>> .... >>> >>> The TAP version is printed by g_test_init(), so we need to make >>> sure that any methods which print are run after that. >>> >>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> >>> --- >> Reviewed-by: Juan Quintela <quintela@redhat.com> >> >>> - const bool has_kvm = qtest_has_accel("kvm"); >>> - const bool has_uffd = ufd_version_check(); >>> - const char *arch = qtest_get_arch(); >>> + bool has_kvm; >>> + bool has_uffd; >>> + const char *arch; >> Why don't you move also the declarations of the variables? >> I think that one of the biggest troubles of C is variables that are not >> initialized. >> All compilers that we support are C99 or later, so we can do that >> (and >> we already do in lot of places.) > > I think the coding style has been created before we switched to > -std=gnu99 for compiling QEMU, so a lot of GCCs were still using C89 > by default? Yes, that is the actitude. I got sick when I see new code that still does: char *foo = (char *)malloc(...); It is is C89, it has been enough to know that it is not needed. And yes, that particular one is not used in qemu anymore, but: void *opaque; .... Foo *foo = (Foo *)opaque; Is still introduced in new code, and it is not needed since C89. >> And yeap, I know that CodingStyle says otherwise, but I think that what >> is wrong is CodingStyle. >> https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg03836.html > > Please use proper prefixes in the subject when sending patches > ("docs/devel:" here), otherwise your patches might not get the right > attention (at least on my side, it was filtered away as a patch that > was relevant to me) - and also put some recent contributors on CC: I didn't knew the docs/devel preffix. About the CC'd, I expected that git-publish be good enough at doing that, but it appears not. Anyways, thanks. Later, Juan.
Juan Quintela <quintela@redhat.com> writes: > Thomas Huth <thuth@redhat.com> wrote: >> On 23/03/2023 19.31, Juan Quintela wrote: >>> Daniel P. Berrangé <berrange@redhat.com> wrote: >>>> The TAP protocol version line must be the first thing printed on >>>> stdout. The migration test failed that requirement in certain >>>> scenarios: >>>> >>>> # Skipping test: Userfault not available (builtdtime) >>>> TAP version 13 >>>> # random seed: R02Sc120c807f11053eb90bfea845ba1e368 >>>> 1..32 >>>> # Start of x86_64 tests >>>> # Start of migration tests >>>> .... >>>> >>>> The TAP version is printed by g_test_init(), so we need to make >>>> sure that any methods which print are run after that. >>>> >>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> >>>> --- >>> Reviewed-by: Juan Quintela <quintela@redhat.com> >>> >>>> - const bool has_kvm = qtest_has_accel("kvm"); >>>> - const bool has_uffd = ufd_version_check(); >>>> - const char *arch = qtest_get_arch(); >>>> + bool has_kvm; >>>> + bool has_uffd; >>>> + const char *arch; >>> Why don't you move also the declarations of the variables? >>> I think that one of the biggest troubles of C is variables that are not >>> initialized. >>> All compilers that we support are C99 or later, so we can do that >>> (and >>> we already do in lot of places.) >> >> I think the coding style has been created before we switched to >> -std=gnu99 for compiling QEMU, so a lot of GCCs were still using C89 >> by default? > > Yes, that is the actitude. > > I got sick when I see new code that still does: > > char *foo = (char *)malloc(...); > > It is is C89, it has been enough to know that it is not needed. > > And yes, that particular one is not used in qemu anymore, but: > > void *opaque; > > .... > > Foo *foo = (Foo *)opaque; > > Is still introduced in new code, and it is not needed since C89. > >>> And yeap, I know that CodingStyle says otherwise, but I think that what >>> is wrong is CodingStyle. >>> https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg03836.html >> >> Please use proper prefixes in the subject when sending patches >> ("docs/devel:" here), otherwise your patches might not get the right >> attention (at least on my side, it was filtered away as a patch that >> was relevant to me) - and also put some recent contributors on CC: > > I didn't knew the docs/devel preffix. > > About the CC'd, I expected that git-publish be good enough at doing > that, but it appears not. I've just sent: Subject: [RFC PATCH] MAINTAINERS: add a section for policy documents Date: Fri, 24 Mar 2023 17:38:36 +0000 Message-Id: <20230324173836.1821275-1-alex.bennee@linaro.org> to collect willing victims^H^H^H^H^H^H volunteers who want to track project related discussions like this. > > Anyways, thanks. > > Later, Juan. -- Alex Bennée Virtualisation Tech Lead @ Linaro
On 17/03/2023 18.05, Daniel P. Berrangé wrote: > The TAP protocol version line must be the first thing printed on > stdout. The migration test failed that requirement in certain > scenarios: > > # Skipping test: Userfault not available (builtdtime) > TAP version 13 > # random seed: R02Sc120c807f11053eb90bfea845ba1e368 > 1..32 > # Start of x86_64 tests > # Start of migration tests > .... > > The TAP version is printed by g_test_init(), so we need to make > sure that any methods which print are run after that. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > tests/qtest/migration-test.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) Reviewed-by: Thomas Huth <thuth@redhat.com>
Daniel P. Berrangé <berrange@redhat.com> writes: > The TAP protocol version line must be the first thing printed on > stdout. The migration test failed that requirement in certain > scenarios: > > # Skipping test: Userfault not available (builtdtime) > TAP version 13 > # random seed: R02Sc120c807f11053eb90bfea845ba1e368 > 1..32 > # Start of x86_64 tests > # Start of migration tests > .... > > The TAP version is printed by g_test_init(), so we need to make > sure that any methods which print are run after that. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Queued to pr/170323-for-8.0-1, thanks. -- Alex Bennée Virtualisation Tech Lead @ Linaro
On Fri, 17 Mar 2023 at 17:06, Daniel P. Berrangé <berrange@redhat.com> wrote: > > The TAP protocol version line must be the first thing printed on > stdout. The migration test failed that requirement in certain > scenarios: > > # Skipping test: Userfault not available (builtdtime) > TAP version 13 > # random seed: R02Sc120c807f11053eb90bfea845ba1e368 > 1..32 > # Start of x86_64 tests > # Start of migration tests > .... > > The TAP version is printed by g_test_init(), so we need to make > sure that any methods which print are run after that. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Yep, that's exactly the same as the patch I wrote but hadn't quite got as far as sending :-) Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
© 2016 - 2023 Red Hat, Inc.