mbox

[PULL,v2,00/52] Misc patches for QEMU 5.1 soft freeze

Message ID 20200708172542.25012-1-pbonzini@redhat.com
State New
Headers show

Pull-request

git://github.com/bonzini/qemu.git tags/for-upstream

Message

Paolo Bonzini July 8, 2020, 5:25 p.m. UTC
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)

----------------------------------------------------------------
* Make checkpatch say 'qemu' instead of 'kernel' (Aleksandar)
* Fix PSE guests with emulated NPT (Alexander B. #1)
* Fix leak (Alexander B. #2)
* HVF fixes (Roman, Cameron)
* New Sapphire Rapids CPUID bits (Cathy)
* cpus.c and softmmu/ cleanups (Claudio)
* TAP driver tweaks (Daniel, Havard)
* object-add bugfix and testcases (Eric A.)
* Fix Coverity MIN_CONST and MAX_CONST (Eric B.)
* "info lapic" improvement (Jan)
* SSE fixes (Joseph)
* "-msg guest-name" option (Mario)
* support for AMD nested live migration (myself)
* Small i386 TCG fixes (myself)
* improved error reporting for Xen (myself)
* fix "-cpu host -overcommit cpu-pm=on" (myself)
* Add accel/Kconfig (Philippe)
* KVM API cleanup (Philippe)
* iscsi sense handling fixes (Yongji)
* Misc bugfixes

----------------------------------------------------------------
v1->v2: add TCG stub for "target/i386: fix IEEE SSE floating-point exception raising"
	remove "cpu-timers, icount: new modules" due to s390 failures
	remove "chardev/tcp: fix error message double free error" which was merged independently
	add "apic: Report current_count via 'info lapic'"
	fix semantic conflict

Aleksandar Markovic (1):
      checkpatch: Change occurences of 'kernel' to 'qemu' in user messages

Alexander Boettcher (1):
      tcg/svm: use host cr4 during NPT page table walk

Alexander Bulekov (1):
      pc: fix leak in pc_system_flash_cleanup_unused

Cameron Esfahani (1):
      i386: hvf: Make long mode enter and exit clearer

Cathy Zhang (2):
      target/i386: Add SERIALIZE cpu feature
      target/i386: Enable TSX Suspend Load Address Tracking feature

Claudio Fontana (2):
      softmmu: move softmmu only files from root
      cpu-throttle: new module, extracted from cpus.c

Daniel P. Berrangé (1):
      scripts: improve message when TAP based tests fail

Eric Auger (3):
      qom: Introduce object_property_try_add_child()
      tests/qmp-cmd-test: Add qmp/object-add-duplicate-id
      tests/qmp-cmd-test: Add qmp/object-add-failure-modes

Eric Blake (1):
      coverity: provide Coverity-friendly MIN_CONST and MAX_CONST

Havard Skinnemoen (1):
      tests: Inject test name also when the test fails

Jan Kiszka (1):
      apic: Report current_count via 'info lapic'

Joseph Myers (2):
      target/i386: set SSE FTZ in correct floating-point state
      target/i386: fix IEEE SSE floating-point exception raising

Luwei Kang (1):
      target/i386: Correct the warning message of Intel PT

Mario Smarduch (1):
      util/qemu-error: prepend guest name to error message to identify affected VM owner

Paolo Bonzini (7):
      KVM: add support for AMD nested live migration
      Makefile: simplify MINIKCONF rules
      target/i386: remove gen_io_end
      target/i386: implement undocumented "smsw r32" behavior
      KVM: x86: believe what KVM says about WAITPKG
      target/i386: sev: provide proper error reporting for query-sev-capabilities
      target/i386: sev: fail query-sev-capabilities if QEMU cannot use SEV

Philippe Mathieu-Daudé (16):
      hw/core/null-machine: Do not initialize unused chardev backends
      MAINTAINERS: Fix KVM path expansion glob
      MAINTAINERS: Add an 'overall' entry for accelerators
      MAINTAINERS: Cover the HAX accelerator stub
      Makefile: Remove dangerous EOL trailing backslash
      Makefile: Write MINIKCONF variables as one entry per line
      accel/Kconfig: Extract accel selectors into their own config
      accel/Kconfig: Add the TCG selector
      accel/tcg: Add stub for probe_access()
      cpus: Move CPU code from exec.c to cpus-common.c
      accel/kvm: Let kvm_check_extension use global KVM state
      accel/kvm: Simplify kvm_check_extension()
      accel/kvm: Simplify kvm_check_extension_list()
      target/i386/kvm: Simplify get_para_features()
      target/i386/kvm: Simplify kvm_get_mce_cap_supported()
      target/i386/kvm: Simplify kvm_get_supported_[feature]_msrs()

Roman Bolshakov (7):
      i386: hvf: Set env->eip in macvm_set_rip()
      i386: hvf: Move synchronize functions to sysemu
      i386: hvf: Add hvf_cpu_synchronize_pre_loadvm()
      i386: hvf: Move Guest LMA reset to macvm_set_cr0()
      i386: hvf: Don't duplicate register reset
      i386: hvf: Clean up synchronize functions
      MAINTAINERS: Add Cameron as HVF co-maintainer

Thomas Huth (1):
      softmmu/vl: Remove the check for colons in -accel parameters

Xie Yongji (2):
      iscsi: handle check condition status in retry loop
      iscsi: return -EIO when sense fields are meaningless

 Kconfig                                      |   4 +
 Kconfig.host                                 |   7 -
 MAINTAINERS                                  |  29 +-
 Makefile                                     |  12 +-
 Makefile.target                              |   7 +-
 accel/Kconfig                                |   9 +
 accel/kvm/kvm-all.c                          |  72 +--
 accel/stubs/tcg-stub.c                       |   7 +
 block/iscsi.c                                |  22 +-
 cpus-common.c                                |  18 +
 exec.c                                       |  22 -
 hw/core/null-machine.c                       |   5 +
 hw/hyperv/hyperv.c                           |   2 +-
 hw/i386/kvm/clock.c                          |   2 +-
 hw/i386/kvm/i8254.c                          |   4 +-
 hw/i386/kvm/ioapic.c                         |   2 +-
 hw/i386/pc_sysfw.c                           |   5 +
 hw/intc/apic.c                               |  18 -
 hw/intc/apic_common.c                        |  19 +
 hw/intc/arm_gic_kvm.c                        |   2 +-
 hw/intc/openpic_kvm.c                        |   2 +-
 hw/intc/xics_kvm.c                           |   2 +-
 hw/s390x/s390-stattrib-kvm.c                 |   2 +-
 include/hw/core/cpu.h                        |  37 --
 include/hw/i386/apic_internal.h              |   1 +
 include/qemu/error-report.h                  |   2 +
 include/qemu/main-loop.h                     |   5 +
 include/qemu/osdep.h                         |  21 +-
 include/qom/object.h                         |  26 +-
 include/sysemu/cpu-throttle.h                |  68 +++
 include/sysemu/hvf.h                         |   2 +-
 include/sysemu/hw_accel.h                    |  13 +
 include/sysemu/kvm.h                         |   2 +-
 migration/migration.c                        |   1 +
 migration/ram.c                              |   1 +
 qemu-options.hx                              |  12 +-
 qom/object.c                                 |  21 +-
 qom/object_interfaces.c                      |   7 +-
 scripts/checkpatch.pl                        |   6 +-
 scripts/tap-driver.pl                        |   2 +-
 softmmu/Makefile.objs                        |  11 +
 arch_init.c => softmmu/arch_init.c           |   0
 balloon.c => softmmu/balloon.c               |   0
 softmmu/cpu-throttle.c                       | 122 ++++
 cpus.c => softmmu/cpus.c                     | 107 +---
 ioport.c => softmmu/ioport.c                 |   0
 memory.c => softmmu/memory.c                 |   0
 memory_mapping.c => softmmu/memory_mapping.c |   0
 qtest.c => softmmu/qtest.c                   |   0
 softmmu/vl.c                                 |  14 +-
 target/arm/kvm.c                             |  18 +-
 target/arm/kvm32.c                           |   2 +-
 target/arm/kvm64.c                           |  15 +-
 target/i386/Makefile.objs                    |   1 +
 target/i386/cpu.c                            |  13 +-
 target/i386/cpu.h                            |  10 +
 target/i386/excp_helper.c                    |   4 +-
 target/i386/fpu_helper.c                     |  37 +-
 target/i386/gdbstub.c                        |   1 +
 target/i386/helper.c                         |   6 +-
 target/i386/helper.h                         |   1 +
 target/i386/hvf/hvf.c                        | 137 +----
 target/i386/hvf/vmx.h                        |  17 +-
 target/i386/kvm.c                            | 143 +++--
 target/i386/kvm_i386.h                       |   1 +
 target/i386/machine.c                        |  31 +-
 target/i386/monitor.c                        |  10 +-
 target/i386/ops_sse.h                        |  28 +-
 target/i386/sev-stub.c                       |   3 +-
 target/i386/sev.c                            |  27 +-
 target/i386/sev_i386.h                       |   2 +-
 target/i386/svm.h                            |   1 +
 target/i386/svm_helper.c                     |   7 +-
 target/i386/tcg-stub.c                       |  25 +
 target/i386/translate.c                      |  36 +-
 target/mips/kvm.c                            |   4 +-
 target/ppc/kvm.c                             |  34 +-
 target/s390x/cpu_models.c                    |   3 +-
 target/s390x/kvm.c                           |  30 +-
 tests/Makefile.include                       |   2 +-
 tests/qtest/qmp-cmd-test.c                   | 109 +++-
 tests/tcg/i386/Makefile.target               |   4 +
 tests/tcg/i386/test-i386-sse-exceptions.c    | 813 +++++++++++++++++++++++++++
 util/qemu-error.c                            |   7 +
 84 files changed, 1750 insertions(+), 587 deletions(-)
 create mode 100644 Kconfig
 create mode 100644 accel/Kconfig
 create mode 100644 include/sysemu/cpu-throttle.h
 rename arch_init.c => softmmu/arch_init.c (100%)
 rename balloon.c => softmmu/balloon.c (100%)
 create mode 100644 softmmu/cpu-throttle.c
 rename cpus.c => softmmu/cpus.c (95%)
 rename ioport.c => softmmu/ioport.c (100%)
 rename memory.c => softmmu/memory.c (100%)
 rename memory_mapping.c => softmmu/memory_mapping.c (100%)
 rename qtest.c => softmmu/qtest.c (100%)
 create mode 100644 target/i386/tcg-stub.c
 create mode 100644 tests/tcg/i386/test-i386-sse-exceptions.c

Comments

Peter Maydell July 10, 2020, 12:14 p.m. UTC | #1
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
Peter Maydell July 10, 2020, 12:28 p.m. UTC | #2
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
Claudio Fontana July 10, 2020, 12:38 p.m. UTC | #3
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
Peter Maydell July 10, 2020, 12:43 p.m. UTC | #4
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
Claudio Fontana July 10, 2020, 12:52 p.m. UTC | #5
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
Peter Maydell July 10, 2020, 12:55 p.m. UTC | #6
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
Claudio Fontana July 10, 2020, 12:55 p.m. UTC | #7
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
Philippe Mathieu-Daudé July 10, 2020, 1:11 p.m. UTC | #8
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!
Claudio Fontana July 10, 2020, 2:44 p.m. UTC | #9
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
Max Reitz July 10, 2020, 3:13 p.m. UTC | #10
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
Peter Maydell July 10, 2020, 3:18 p.m. UTC | #11
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
Max Reitz July 10, 2020, 3:31 p.m. UTC | #12
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
Peter Maydell July 10, 2020, 3:42 p.m. UTC | #13
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
Max Reitz July 10, 2020, 3:46 p.m. UTC | #14
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
Daniel P. Berrangé July 10, 2020, 3:47 p.m. UTC | #15
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
Peter Maydell July 10, 2020, 3:50 p.m. UTC | #16
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
Thomas Huth July 10, 2020, 3:53 p.m. UTC | #17
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