Message ID | 20200708172542.25012-1-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
On Wed, 8 Jul 2020 at 22:32, Paolo Bonzini <pbonzini@redhat.com> wrote: > > The following changes since commit eb2c66b10efd2b914b56b20ae90655914310c925: > > Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-07-06' into staging (2020-07-07 19:47:26 +0100) > > are available in the Git repository at: > > git://github.com/bonzini/qemu.git tags/for-upstream > > for you to fetch changes up to 392f34e59755f99d69586a63e0f5d80a7ef67f94: > > apic: Report current_count via 'info lapic' (2020-07-08 10:01:08 -0400) Hi; this still has the OSX failure, I'm afraid: /Users/pm215/src/qemu-for-merges/ui/cocoa.m:1478:9: error: implicit declaration of function 'cpu_throttle_set' is invalid in C99 [- Werror,-Wimplicit-function-declaration] cpu_throttle_set(throttle_pct); ^ /Users/pm215/src/qemu-for-merges/ui/cocoa.m:1478:9: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes] (other builds haven't reported back yet) thanks -- PMM
On Fri, 10 Jul 2020 at 13:14, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Wed, 8 Jul 2020 at 22:32, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > The following changes since commit eb2c66b10efd2b914b56b20ae90655914310c925: > > > > Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-07-06' into staging (2020-07-07 19:47:26 +0100) > > > > are available in the Git repository at: > > > > git://github.com/bonzini/qemu.git tags/for-upstream > > > > for you to fetch changes up to 392f34e59755f99d69586a63e0f5d80a7ef67f94: > > > > apic: Report current_count via 'info lapic' (2020-07-08 10:01:08 -0400) > > Hi; this still has the OSX failure, I'm afraid: > > /Users/pm215/src/qemu-for-merges/ui/cocoa.m:1478:9: error: implicit > declaration of function 'cpu_throttle_set' is invalid in C99 [- > Werror,-Wimplicit-function-declaration] > cpu_throttle_set(throttle_pct); > ^ > /Users/pm215/src/qemu-for-merges/ui/cocoa.m:1478:9: error: this > function declaration is not a prototype [-Werror,-Wstrict-prototypes] Squashing this into "cpu-throttle: new module, extracted from cpus.c" should fix this (ui/cocoa.m was just forgotten when adding #include lines after moving the function into its own header): diff --git a/ui/cocoa.m b/ui/cocoa.m index cb556e4e66..0910b4a716 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -32,6 +32,7 @@ #include "ui/input.h" #include "sysemu/sysemu.h" #include "sysemu/runstate.h" +#include "sysemu/cpu-throttle.h" #include "qapi/error.h" #include "qapi/qapi-commands-block.h" #include "qapi/qapi-commands-misc.h" (am just doing a compile-and-test run with that change). thanks -- PMM
On 7/10/20 2:14 PM, Peter Maydell wrote: > On Wed, 8 Jul 2020 at 22:32, Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> The following changes since commit eb2c66b10efd2b914b56b20ae90655914310c925: >> >> Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-07-06' into staging (2020-07-07 19:47:26 +0100) >> >> are available in the Git repository at: >> >> git://github.com/bonzini/qemu.git tags/for-upstream >> >> for you to fetch changes up to 392f34e59755f99d69586a63e0f5d80a7ef67f94: >> >> apic: Report current_count via 'info lapic' (2020-07-08 10:01:08 -0400) > > Hi; this still has the OSX failure, I'm afraid: > > /Users/pm215/src/qemu-for-merges/ui/cocoa.m:1478:9: error: implicit > declaration of function 'cpu_throttle_set' is invalid in C99 [- > Werror,-Wimplicit-function-declaration] > cpu_throttle_set(throttle_pct); > ^ > /Users/pm215/src/qemu-for-merges/ui/cocoa.m:1478:9: error: this > function declaration is not a prototype [-Werror,-Wstrict-prototypes] > > (other builds haven't reported back yet) > > thanks > -- PMM > Hi Peter, I got regular green test reports from cirrus-ci for Mac, seems different compilation options. The prototypes for cpu_throttle_ functions are in sysemu/cpu-throttle.h so the fix should be to just #include "sysemu-cpu_throttle.h" The fact that we get so wildly different results from CI is concerning to me. Should I resend you the cpu throttle patch with this change? Ciao C
On Fri, 10 Jul 2020 at 13:38, Claudio Fontana <cfontana@suse.de> wrote:
> I got regular green test reports from cirrus-ci for Mac, seems different compilation options.
That's odd -- what is cirrus-ci doing differently? Building
the cocoa UI frontend is the default and is definitely
something we want to be testing in the CI...
thanks
-- PMM
On 7/10/20 2:43 PM, Peter Maydell wrote: > On Fri, 10 Jul 2020 at 13:38, Claudio Fontana <cfontana@suse.de> wrote: >> I got regular green test reports from cirrus-ci for Mac, seems different compilation options. > > That's odd -- what is cirrus-ci doing differently? Building > the cocoa UI frontend is the default and is definitely > something we want to be testing in the CI... > > thanks > -- PMM > If you can access it, https://cirrus-ci.com/task/5537514263937024?command=main#L2039 the thing is treated here as a warning, which is in the middle of a large amount of other warnings. private/var/folders/3y/l0z1x3693dl_8n0qybp4dqwh0000gn/T/cirrus-ci-build/ui/vnc-auth-sasl.c:648:29: warning: 'sasl_errdetail' is deprecated: first deprecated in macOS 10.11 [-Wdeprecated-declarations] sasl_errdetail(vs->sasl.conn)); /* ... lots of similar warnings */ private/var/folders/3y/l0z1x3693dl_8n0qybp4dqwh0000gn/T/cirrus-ci-build/ui/cocoa.m:1478:9: warning: implicit declaration of function 'cpu_throttle_set' is invalid in C99 [-Wimplicit-function-declaration] cpu_throttle_set(throttle_pct); ^ Apparently the cirrus-ci I am using is not treating this as an error, while what you are using is.. Thanks, Claudio
On Fri, 10 Jul 2020 at 13:52, Claudio Fontana <cfontana@suse.de> wrote: > If you can access it, > > https://cirrus-ci.com/task/5537514263937024?command=main#L2039 > > the thing is treated here as a warning, which is in the middle of a large amount of other warnings. > > private/var/folders/3y/l0z1x3693dl_8n0qybp4dqwh0000gn/T/cirrus-ci-build/ui/vnc-auth-sasl.c:648:29: warning: 'sasl_errdetail' is deprecated: first deprecated in macOS 10.11 [-Wdeprecated-declarations] > sasl_errdetail(vs->sasl.conn)); > > /* ... lots of similar warnings */ > > private/var/folders/3y/l0z1x3693dl_8n0qybp4dqwh0000gn/T/cirrus-ci-build/ui/cocoa.m:1478:9: warning: implicit declaration of function 'cpu_throttle_set' is invalid in C99 [-Wimplicit-function-declaration] > cpu_throttle_set(throttle_pct); > ^ > > > Apparently the cirrus-ci I am using is not treating this as an error, while what you are using is.. Ah, I see. Yeah, configure by default doesn't enable -Werror for OSX. My build tree has '--extra-cflags=-fdiagnostics-color=never -Werror -Wno-error=deprecated-declarations' (of which the last two are relevant here). We should probably make configure use -Werror on OSX now with the same logic as Linux, I've been using it that way for ages without issues. thanks -- PMM
On 7/10/20 2:28 PM, Peter Maydell wrote: > On Fri, 10 Jul 2020 at 13:14, Peter Maydell <peter.maydell@linaro.org> wrote: >> >> On Wed, 8 Jul 2020 at 22:32, Paolo Bonzini <pbonzini@redhat.com> wrote: >>> >>> The following changes since commit eb2c66b10efd2b914b56b20ae90655914310c925: >>> >>> Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-07-06' into staging (2020-07-07 19:47:26 +0100) >>> >>> are available in the Git repository at: >>> >>> git://github.com/bonzini/qemu.git tags/for-upstream >>> >>> for you to fetch changes up to 392f34e59755f99d69586a63e0f5d80a7ef67f94: >>> >>> apic: Report current_count via 'info lapic' (2020-07-08 10:01:08 -0400) >> >> Hi; this still has the OSX failure, I'm afraid: >> >> /Users/pm215/src/qemu-for-merges/ui/cocoa.m:1478:9: error: implicit >> declaration of function 'cpu_throttle_set' is invalid in C99 [- >> Werror,-Wimplicit-function-declaration] >> cpu_throttle_set(throttle_pct); >> ^ >> /Users/pm215/src/qemu-for-merges/ui/cocoa.m:1478:9: error: this >> function declaration is not a prototype [-Werror,-Wstrict-prototypes] > > Squashing this into "cpu-throttle: new module, extracted from cpus.c" > should fix this (ui/cocoa.m was just forgotten when adding #include lines > after moving the function into its own header): > > diff --git a/ui/cocoa.m b/ui/cocoa.m > index cb556e4e66..0910b4a716 100644 > --- a/ui/cocoa.m > +++ b/ui/cocoa.m > @@ -32,6 +32,7 @@ > #include "ui/input.h" > #include "sysemu/sysemu.h" > #include "sysemu/runstate.h" > +#include "sysemu/cpu-throttle.h" > #include "qapi/error.h" > #include "qapi/qapi-commands-block.h" > #include "qapi/qapi-commands-misc.h" > > (am just doing a compile-and-test run with that change). > > thanks > -- PMM > Ah, just noticed this, yes, indeed I clearly forgot that, at the same time clearly I need to improve both my local environment and the CI setups I am using, because for me everything works! Or, we could swap configurations so that QEMU builds are always successful... ;-) Ciao, Claudio
On 7/10/20 2:55 PM, Peter Maydell wrote: > On Fri, 10 Jul 2020 at 13:52, Claudio Fontana <cfontana@suse.de> wrote: >> If you can access it, >> >> https://cirrus-ci.com/task/5537514263937024?command=main#L2039 >> >> the thing is treated here as a warning, which is in the middle of a large amount of other warnings. >> >> private/var/folders/3y/l0z1x3693dl_8n0qybp4dqwh0000gn/T/cirrus-ci-build/ui/vnc-auth-sasl.c:648:29: warning: 'sasl_errdetail' is deprecated: first deprecated in macOS 10.11 [-Wdeprecated-declarations] >> sasl_errdetail(vs->sasl.conn)); >> >> /* ... lots of similar warnings */ >> >> private/var/folders/3y/l0z1x3693dl_8n0qybp4dqwh0000gn/T/cirrus-ci-build/ui/cocoa.m:1478:9: warning: implicit declaration of function 'cpu_throttle_set' is invalid in C99 [-Wimplicit-function-declaration] >> cpu_throttle_set(throttle_pct); >> ^ >> >> >> Apparently the cirrus-ci I am using is not treating this as an error, while what you are using is.. > > Ah, I see. Yeah, configure by default doesn't enable -Werror for OSX. > My build tree has > '--extra-cflags=-fdiagnostics-color=never -Werror > -Wno-error=deprecated-declarations' TIL this is different on OSX... > > (of which the last two are relevant here). We should probably > make configure use -Werror on OSX now with the same logic as Linux, > I've been using it that way for ages without issues. Yes please!
On 7/10/20 3:11 PM, Philippe Mathieu-Daudé wrote: > On 7/10/20 2:55 PM, Peter Maydell wrote: >> On Fri, 10 Jul 2020 at 13:52, Claudio Fontana <cfontana@suse.de> wrote: >>> If you can access it, >>> >>> https://cirrus-ci.com/task/5537514263937024?command=main#L2039 >>> >>> the thing is treated here as a warning, which is in the middle of a large amount of other warnings. >>> >>> private/var/folders/3y/l0z1x3693dl_8n0qybp4dqwh0000gn/T/cirrus-ci-build/ui/vnc-auth-sasl.c:648:29: warning: 'sasl_errdetail' is deprecated: first deprecated in macOS 10.11 [-Wdeprecated-declarations] >>> sasl_errdetail(vs->sasl.conn)); >>> >>> /* ... lots of similar warnings */ >>> >>> private/var/folders/3y/l0z1x3693dl_8n0qybp4dqwh0000gn/T/cirrus-ci-build/ui/cocoa.m:1478:9: warning: implicit declaration of function 'cpu_throttle_set' is invalid in C99 [-Wimplicit-function-declaration] >>> cpu_throttle_set(throttle_pct); >>> ^ >>> >>> >>> Apparently the cirrus-ci I am using is not treating this as an error, while what you are using is.. >> >> Ah, I see. Yeah, configure by default doesn't enable -Werror for OSX. >> My build tree has >> '--extra-cflags=-fdiagnostics-color=never -Werror >> -Wno-error=deprecated-declarations' > > TIL this is different on OSX... > >> >> (of which the last two are relevant here). We should probably >> make configure use -Werror on OSX now with the same logic as Linux, >> I've been using it that way for ages without issues. > > Yes please! > Speaking of MacOS and CI, commit 57ee95ed4ee7b4c039ec5f0705c45734c56706bc Author: Max Reitz <mreitz@redhat.com> Date: Thu Jun 25 14:55:30 2020 +0200 iotests: Make _filter_img_create more active broke cirrus-ci completely for me due to missing "readarray" builtin. Maybe it is bash vs zsh? Ciao, Claudio
On 10.07.20 16:44, Claudio Fontana wrote: > On 7/10/20 3:11 PM, Philippe Mathieu-Daudé wrote: >> On 7/10/20 2:55 PM, Peter Maydell wrote: >>> On Fri, 10 Jul 2020 at 13:52, Claudio Fontana <cfontana@suse.de> wrote: >>>> If you can access it, >>>> >>>> https://cirrus-ci.com/task/5537514263937024?command=main#L2039 >>>> >>>> the thing is treated here as a warning, which is in the middle of a large amount of other warnings. >>>> >>>> private/var/folders/3y/l0z1x3693dl_8n0qybp4dqwh0000gn/T/cirrus-ci-build/ui/vnc-auth-sasl.c:648:29: warning: 'sasl_errdetail' is deprecated: first deprecated in macOS 10.11 [-Wdeprecated-declarations] >>>> sasl_errdetail(vs->sasl.conn)); >>>> >>>> /* ... lots of similar warnings */ >>>> >>>> private/var/folders/3y/l0z1x3693dl_8n0qybp4dqwh0000gn/T/cirrus-ci-build/ui/cocoa.m:1478:9: warning: implicit declaration of function 'cpu_throttle_set' is invalid in C99 [-Wimplicit-function-declaration] >>>> cpu_throttle_set(throttle_pct); >>>> ^ >>>> >>>> >>>> Apparently the cirrus-ci I am using is not treating this as an error, while what you are using is.. >>> >>> Ah, I see. Yeah, configure by default doesn't enable -Werror for OSX. >>> My build tree has >>> '--extra-cflags=-fdiagnostics-color=never -Werror >>> -Wno-error=deprecated-declarations' >> >> TIL this is different on OSX... >> >>> >>> (of which the last two are relevant here). We should probably >>> make configure use -Werror on OSX now with the same logic as Linux, >>> I've been using it that way for ages without issues. >> >> Yes please! >> > > Speaking of MacOS and CI, commit 57ee95ed4ee7b4c039ec5f0705c45734c56706bc > > Author: Max Reitz <mreitz@redhat.com> > Date: Thu Jun 25 14:55:30 2020 +0200 > > iotests: Make _filter_img_create more active > > broke cirrus-ci completely for me due to missing "readarray" builtin. > > Maybe it is bash vs zsh? Possible, but the iotests depend on bash. All the iotests shell source files explicitly reference bash in their shebang line, and so far we’ve always assumed that they are thus going to be run by bash. I suppose it’s well possible we’ve been lucky so far and all bash-isms we used were supported by zsh as well. But why does cirrus-ci run them via zsh if the shebang line explicitly launches “bash”? Max
On Fri, 10 Jul 2020 at 16:13, Max Reitz <mreitz@redhat.com> wrote: > > On 10.07.20 16:44, Claudio Fontana wrote: > > Speaking of MacOS and CI, commit 57ee95ed4ee7b4c039ec5f0705c45734c56706bc > > > > Author: Max Reitz <mreitz@redhat.com> > > Date: Thu Jun 25 14:55:30 2020 +0200 > > > > iotests: Make _filter_img_create more active > > > > broke cirrus-ci completely for me due to missing "readarray" builtin. > > > > Maybe it is bash vs zsh? > > Possible, but the iotests depend on bash. All the iotests shell source > files explicitly reference bash in their shebang line, and so far we’ve > always assumed that they are thus going to be run by bash. readarray only arrived sometime in bash 4, and the OSX system bash is 3.2.57, so it won't have that builtin. thanks -- PMM
On 10.07.20 17:18, Peter Maydell wrote: > On Fri, 10 Jul 2020 at 16:13, Max Reitz <mreitz@redhat.com> wrote: >> >> On 10.07.20 16:44, Claudio Fontana wrote: >>> Speaking of MacOS and CI, commit 57ee95ed4ee7b4c039ec5f0705c45734c56706bc >>> >>> Author: Max Reitz <mreitz@redhat.com> >>> Date: Thu Jun 25 14:55:30 2020 +0200 >>> >>> iotests: Make _filter_img_create more active >>> >>> broke cirrus-ci completely for me due to missing "readarray" builtin. >>> >>> Maybe it is bash vs zsh? >> >> Possible, but the iotests depend on bash. All the iotests shell source >> files explicitly reference bash in their shebang line, and so far we’ve >> always assumed that they are thus going to be run by bash. > > readarray only arrived sometime in bash 4, and the OSX system > bash is 3.2.57, so it won't have that builtin. It arrived with 4.0, actually, which was released 11 years ago. I had assumed that would be sufficiently mature. So, um, 11 years isn’t sufficiently mature then and I’ll have to work around not having readarray for macOS? Max
On Fri, 10 Jul 2020 at 16:31, Max Reitz <mreitz@redhat.com> wrote: > > On 10.07.20 17:18, Peter Maydell wrote: > > readarray only arrived sometime in bash 4, and the OSX system > > bash is 3.2.57, so it won't have that builtin. > > It arrived with 4.0, actually, which was released 11 years ago. > I had assumed that would be sufficiently mature. > > So, um, 11 years isn’t sufficiently mature then and I’ll have to work > around not having readarray for macOS? It's the usual Apple-vs-GPL3 issue. I note that the iotests do seem to regularly run into non-portable constructs: Kevin's latest pullreq has just failed due to a use of 'truncate' that doesn't work on the BSDs. thanks -- PMM
On 10.07.20 17:42, Peter Maydell wrote: > On Fri, 10 Jul 2020 at 16:31, Max Reitz <mreitz@redhat.com> wrote: >> >> On 10.07.20 17:18, Peter Maydell wrote: >>> readarray only arrived sometime in bash 4, and the OSX system >>> bash is 3.2.57, so it won't have that builtin. >> >> It arrived with 4.0, actually, which was released 11 years ago. >> I had assumed that would be sufficiently mature. >> >> So, um, 11 years isn’t sufficiently mature then and I’ll have to work >> around not having readarray for macOS? > > It's the usual Apple-vs-GPL3 issue. > > I note that the iotests do seem to regularly run into > non-portable constructs: Kevin's latest pullreq has > just failed due to a use of 'truncate' that doesn't > work on the BSDs. :/ I’ll send a patch to drop readarray. Max
On Fri, Jul 10, 2020 at 04:42:11PM +0100, Peter Maydell wrote: > On Fri, 10 Jul 2020 at 16:31, Max Reitz <mreitz@redhat.com> wrote: > > > > On 10.07.20 17:18, Peter Maydell wrote: > > > readarray only arrived sometime in bash 4, and the OSX system > > > bash is 3.2.57, so it won't have that builtin. > > > > It arrived with 4.0, actually, which was released 11 years ago. > > I had assumed that would be sufficiently mature. > > > > So, um, 11 years isn’t sufficiently mature then and I’ll have to work > > around not having readarray for macOS? > > It's the usual Apple-vs-GPL3 issue. > > I note that the iotests do seem to regularly run into > non-portable constructs: Kevin's latest pullreq has > just failed due to a use of 'truncate' that doesn't > work on the BSDs. Since we already depend on homebrew for the build environment, we can pull in the newer bash from homebrew, and ignore the ancient version from macOS stock install. Regards, Daniel
On Fri, 10 Jul 2020 at 16:46, Max Reitz <mreitz@redhat.com> wrote: > > On 10.07.20 17:42, Peter Maydell wrote: > > On Fri, 10 Jul 2020 at 16:31, Max Reitz <mreitz@redhat.com> wrote: > >> > >> On 10.07.20 17:18, Peter Maydell wrote: > >>> readarray only arrived sometime in bash 4, and the OSX system > >>> bash is 3.2.57, so it won't have that builtin. > >> > >> It arrived with 4.0, actually, which was released 11 years ago. > >> I had assumed that would be sufficiently mature. > >> > >> So, um, 11 years isn’t sufficiently mature then and I’ll have to work > >> around not having readarray for macOS? > > > > It's the usual Apple-vs-GPL3 issue. > > > > I note that the iotests do seem to regularly run into > > non-portable constructs: Kevin's latest pullreq has > > just failed due to a use of 'truncate' that doesn't > > work on the BSDs. > > :/ > > I’ll send a patch to drop readarray. Thanks. I realised as a result of discussion on IRC that this slipped through my testing because my OSX system wasn't running iotests at all because it didn't have a GNU sed available. As Dan says, the other option is to do the same thing we do for GNU sed, and skip all the iotests if we don't have a new enough bash. I can always install sed and bash out of homebrew to bring my setup into line with the cirrus CI one. If we take that path we should update the cirrus CI config to make sure it also installs homebrew bash (and that the iotests use bash-from-the-path, not /bin/bash.) -- PMM
On 10/07/2020 17.47, Daniel P. Berrangé wrote: > On Fri, Jul 10, 2020 at 04:42:11PM +0100, Peter Maydell wrote: >> On Fri, 10 Jul 2020 at 16:31, Max Reitz <mreitz@redhat.com> wrote: >>> >>> On 10.07.20 17:18, Peter Maydell wrote: >>>> readarray only arrived sometime in bash 4, and the OSX system >>>> bash is 3.2.57, so it won't have that builtin. >>> >>> It arrived with 4.0, actually, which was released 11 years ago. >>> I had assumed that would be sufficiently mature. >>> >>> So, um, 11 years isn’t sufficiently mature then and I’ll have to work >>> around not having readarray for macOS? >> >> It's the usual Apple-vs-GPL3 issue. >> >> I note that the iotests do seem to regularly run into >> non-portable constructs: Kevin's latest pullreq has >> just failed due to a use of 'truncate' that doesn't >> work on the BSDs. > > Since we already depend on homebrew for the build environment, we can > pull in the newer bash from homebrew, and ignore the ancient version > from macOS stock install. I just had the same idea. And then add a simple check for bash 3.x in tests/check-block.sh so that it skips the iotests if only the old version is available. Thomas