The BootLinux tests are currently failing with an ugly python
stack trace on my RHEL8 system since they cannot get a free port
(likely due to the firewall settings on my system). Let's properly
check the return value of find_free_port() instead and cancel the
test gracefully if it cannot get a free port.
Message-Id: <20220228114325.818294-1-thuth@redhat.com>
Reviewed-by: Beraldo Leal <bleal@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
The FreeBSD CI definitions now reside in other files than .cirrs.yml.
Update the entry in MAINTAINERS accordingly.
Message-Id: <20220217141138.917292-1-thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Adds clobbers and merges remaining separate asm statements.
Signed-off-by: David Miller <dmiller423@gmail.com>
Message-Id: <20220301214305.2778-1-dmiller423@gmail.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
[thuth: dropped changes to mie3-compl.c, whitespace fixes]
Signed-off-by: Thomas Huth <thuth@redhat.com>
The exrl-trt* tests use two pre-initialized variables for the
results of the assembly code:
uint64_t r1 = 0xffffffffffffffffull;
uint64_t r2 = 0xffffffffffffffffull;
But then the assembly code copies over the full contents
of the register into the output variable, without taking
care of this pre-initialized values:
" lgr %[r1],%%r1\n"
" lgr %[r2],%%r2\n"
The code then finally compares the register contents to
a value that apparently depends on the pre-initialized values:
if (r2 != 0xffffffffffffffaaull) {
write(1, "bad r2\n", 7);
return 1;
}
This all works with GCC, since the 0xffffffffffffffff got into
the r2 register there by accident, but it fails completely with
Clang.
Let's fix this by declaring the r1 and r2 variables as proper
register variables instead, so the pre-initialized values get
correctly passed into the inline assembly code.
Message-Id: <20220301092431.1448419-1-thuth@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
These instructions use addressing with a "base address", meaning
that if register r0 is used, it is always treated as zero, no matter
what value is stored in the register. So we have to make sure not
to use register r0 for these instructions in our tests. There was
no problem with GCC so far since it seems to always pick other
registers by default, but Clang likes to chose register r0, too,
so we have to use the "a" constraint to make sure that it does
not pick r0 here.
Message-Id: <20220301093911.1450719-1-thuth@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
* Clang fixes
* Vector/VSX instruction batch fixes
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEEoPZlSPBIlev+awtgUaNDx8/77KEFAmIjHL4ACgkQUaNDx8/7
7KH+sg/+JZW/X+U7uZRT9vK6vS4jqFG7gn55ouZKSvAQtPUMtUFdkEL/jeA3IuHh
kRvmjGwWjtjgM9TIiJsVBo8yJeMqUO8f6AKUtaQU/3vT4CPO2Jbd8X0A58OOoe4I
M3fzWf/18bAYPPEZAxmlJ8f8LEPsBOLM0tQ2py7JkVpUQU3WcP7/YgXbYRH1l+9k
m1lMyXGZ/dQDxkFel9TGcfuHXVecBngyx4O1pJ1JkHduo+bLAi6h5HJgdj1wEVfr
5vyqn7OULm6bPXWRb1j+CnCj0QO0VAky4hC4HDGajMlcnG8l/X8Dzs2bD+ua/VbE
1aNkVpuNtuu0ljKfarqMED2no8lPJg4MpKOZvi8O128/bU+IkmJUYI2g73Gr6URL
V6/Awh0syy9mi04gsXMD5kCwFLuBNQ5jC3z1aAZNy7Be/l37vzAoN0i53eYAGK3D
rBTuMictAKRXKyXQ63VLiYqSdUB6zVoOIM6w3HJxQTQ8YkJ/Hx5cfOGv+lnTIdsK
cX4e7s1p8YPSnDOEfEKTk+gnVcTn57FAhNJhelRuC1s2/OI8Gz4n+3eUGmcl04Bq
+gcD1UEGMEsQpjMz+ikPuGMZvmF5gPr+9S3DPyMfUOoQJFUI8GOxq9SmUVKC9fki
6oHTHpOvZg8MSJz8gzETq18hwHRKoRb95KzVHWGj1EG0ekjHG2U=
=C+wh
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/legoater/tags/pull-ppc-20220305' into staging
ppc-7.0 queue :
* Clang fixes
* Vector/VSX instruction batch fixes
# gpg: Signature made Sat 05 Mar 2022 08:18:06 GMT
# gpg: using RSA key A0F66548F04895EBFE6B0B6051A343C7CFFBECA1
# gpg: Good signature from "Cédric Le Goater <clg@kaod.org>" [undefined]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg: There is no indication that the signature belongs to the owner.
# Primary key fingerprint: A0F6 6548 F048 95EB FE6B 0B60 51A3 43C7 CFFB ECA1
* remotes/legoater/tags/pull-ppc-20220305:
target/ppc: Add missing helper_reset_fpstatus to helper_XVCVSPBF16
target/ppc: Add missing helper_reset_fpstatus to VSX_MAX_MINC
target/ppc: split XXGENPCV macros for readability
target/ppc: use andc in vrlqmi
target/ppc: use extract/extract2 to create vrlqnm mask
target/ppc: use ext32u and deposit in do_vx_vmulhw_i64
target/ppc: Fix vmul[eo]* instructions marked 2.07
tests/tcg/ppc64le: Use Altivec register names in clobber list
tests/tcg/ppc64le: emit bcdsub with .long when needed
tests/tcg/ppc64le: drop __int128 usage in bcdsub
target/ppc: change xs[n]madd[am]sp to use float64r32_muladd
tests/tcg/ppc64le: use inline asm instead of __builtin_mtfsf
Use long endian options for ppc64
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
- qemu-storage-daemon: Add --daemonize
- Fix x-blockdev-amend and block node activation code which incorrectly
executed code in the iothread that must run in the main thread.
- Add macros for coroutine-safe TLS variables (required for correctness
with LTO)
- Fix crashes with concurrent I/O and bdrv_refresh_limits()
- Split block APIs in global state and I/O
- iotests: Don't refuse to run at all without GNU sed, just skip tests
that need it
-----BEGIN PGP SIGNATURE-----
iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmIiSecRHGt3b2xmQHJl
ZGhhdC5jb20ACgkQfwmycsiPL9aSMBAAhS1FLwiUPJ5zsRlYkFiJ76M5AEJPNgYT
F3QqBxJa4d/rR8Hibx0p6bFU21QKIat2OIkepcaVGh8oOM8/8DKx1dUlhQt3IOQq
yTJ5klBTxQtnBYapEsZC1bcRgRhLXbhjsXtJluzJrfvIYO0BPdVmpetTY4vJ7v79
U2lYImHkUYZ3xH84qXj3ymfURyBc8LpjmMwWrCaEkjxcwfgb1fOeZuGEy7B387aL
zpYE2oKjSSI20TTbJ+VsPgf2CglmTRl2kILnWP0tFjh5clpozkXAJ/0WW/TwgQgJ
20Blvxk4inSfkMxHPdW0ttoBfW+WqftFFh1t0xqeUn6AfQFJkpQ93RmWk4rpKc8k
rVcXIO54sYNEcJfkofs0m7N6rDk5HBq1WA7wt5veWBeNeoKWALcqjFSlr52FofJr
bcCFnf/DRrGJ9XSi0XDqAqJeuqcGARVViqJZL3jUm+7VuLYcdA7d1wVUzuPUdv+0
KdANzzoLaGR8xNbB+NqRBuzOcxoXYRZWbKH5i2XDk+FCwl5qcg/XalsAcM0bwXPL
moRkH7csqrnD4cBZDSToZoi/iNdlynSIZmI8pL5Tr9btPODBF8lQEiPtJziSHReo
v7S1nR0Q6NNOpuZUMzLJJoPcm+uy7n672SAoWhpbvh0NTdW9msxtqY2KGCKjJH8l
f5zp/zljV0Y=
=Jdal
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/kwolf-gitlab/tags/for-upstream' into staging
Block layer patches
- qemu-storage-daemon: Add --daemonize
- Fix x-blockdev-amend and block node activation code which incorrectly
executed code in the iothread that must run in the main thread.
- Add macros for coroutine-safe TLS variables (required for correctness
with LTO)
- Fix crashes with concurrent I/O and bdrv_refresh_limits()
- Split block APIs in global state and I/O
- iotests: Don't refuse to run at all without GNU sed, just skip tests
that need it
# gpg: Signature made Fri 04 Mar 2022 17:18:31 GMT
# gpg: using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
# gpg: issuer "kwolf@redhat.com"
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6
* remotes/kwolf-gitlab/tags/for-upstream: (50 commits)
block/amend: Keep strong reference to BDS
block/amend: Always call .bdrv_amend_clean()
tests/qemu-iotests: Rework the checks and spots using GNU sed
iotests/graph-changes-while-io: New test
iotests: Allow using QMP with the QSD
block: Make bdrv_refresh_limits() non-recursive
job.h: assertions in the callers of JobDriver function pointers
job.h: split function pointers in JobDriver
block-backend-common.h: split function pointers in BlockDevOps
block_int-common.h: assertions in the callers of BdrvChildClass function pointers
block_int-common.h: split function pointers in BdrvChildClass
block_int-common.h: assertions in the callers of BlockDriver function pointers
block_int-common.h: split function pointers in BlockDriver
block/coroutines: I/O and "I/O or GS" API
block/copy-before-write.h: global state API + assertions
include/block/snapshot: global state API + assertions
assertions for blockdev.h global state API
include/sysemu/blockdev.h: global state API
assertions for blockjob.h global state API
include/block/blockjob.h: global state API
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Some ISA v2.03 Vector Multiply instructions marked to be ISA v2.07 only.
This patch fixes it.
Fixes: 80eca687c8 ("target/ppc: moved vector even and odd multiplication to decodetree")
Reported-by: Howard Spoelstra <hsp.cat7@gmail.com>
Suggested-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20220304175156.2012315-2-matheus.ferst@eldorado.org.br>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
LLVM/Clang doesn't know the VSX registers when compiling with
-mabi=elfv1. Use only registers >= 32 and list them with their Altivec
name.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Message-Id: <20220304165417.1981159-6-matheus.ferst@eldorado.org.br>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Based on GCC docs[1], we use the '-mpower8-vector' flag at config-time
to detect the toolchain support to the bcdsub instruction. LLVM/Clang
supports this flag since version 3.6[2], but the instruction and related
builtins were only added in LLVM 14[3]. In the absence of other means to
detect this support at config-time, we resort to __has_builtin to
identify the presence of __builtin_bcdsub at compile-time. If the
builtin is not available, the instruction is emitted with a ".long".
[1] https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/PowerPC-AltiVec_002fVSX-Built-in-Functions.html
[2] 59eb767e11
[3] c933c2eb33
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Message-Id: <20220304165417.1981159-5-matheus.ferst@eldorado.org.br>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Using __int128 with inline asm constraints like "v" generates incorrect
code when compiling with LLVM/Clang (e.g., only one doubleword of the
VSR is loaded). Instead, use a GPR pair to pass the 128-bits value and
load the VSR with mtvsrd/xxmrghd.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Message-Id: <20220304165417.1981159-4-matheus.ferst@eldorado.org.br>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Change VSX Scalar Multiply-Add/Subtract Type-A/M Single Precision
helpers to use float64r32_muladd. This method should correctly handle
all rounding modes, so the workaround for float_round_nearest_even can
be dropped.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Message-Id: <20220304165417.1981159-3-matheus.ferst@eldorado.org.br>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
LLVM/Clang does not support __builtin_mtfsf.
Acked-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Message-Id: <20220304165417.1981159-2-matheus.ferst@eldorado.org.br>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
GCC options pairs -mlittle/-mlittle-endian and -mbig/-mbig-endian are
equivalent on ppc64 architecture. However, Clang supports only long
version of the options.
Use longer form in configure to properly support both GCC and Clang
compiler. In addition, fix this issue in tcg test configure.
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <20220131091714.4825-1-mrezanin@redhat.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
The general ternary logic operation can implement BITSEL.
Funnel the 4-operand operation into three variants of the
3-operand instruction, depending on input operand overlap.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
AVX512VL has a general ternary logic operation, VPTERNLOGQ,
which can implement NOT, ORC, NAND, NOR, EQV.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
AVX512DQ has VPMULLQ.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
AVX512VL has VPABSQ, VPMAXSQ, VPMAXUQ, VPMINSQ, VPMINUQ.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Expand 32-bit and 64-bit scalar rotate with VPRO[LR]V;
expand 16-bit scalar rotate with VPSHLDV.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
There is no such instruction on x86, so we should
not be pretending it has arguments.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
While there are no specific 16-bit rotate instructions, there
are double-word shifts, which can perform the same operation.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
We will use VPSHLD, VPSHLDV and VPSHRDV for 16-bit rotates.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
AVX512VL has VPROLVD and VPRORVQ.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
AVX512VL has VPROLD and VPROLQ, layered onto the same
opcode as PSHIFTD, but requires EVEX encoding and W1.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
AVX512 has VPSRAQ with immediate operand, in the same form as
with AVX, but requires EVEX encoding and W1.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
AVX512VL has VPSRAQ.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
AVX512VL has VPSRAVQ, and
AVX512BW has VPSLLVW, VPSRAVW, VPSRLVW.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
The condition for UMIN/UMAX availability is about to change;
use the canonical version.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
The evex encoding is added here, for use in a subsequent patch.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
There are some operation sizes in some subsets of AVX512 that
are missing from previous iterations of AVX. Detect them.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
We've had placeholders for these opcodes for a while,
and should have support on ppc, s390x and avx512 hosts.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
The last entry of DEF_HELPERS_FLAGS_n is DEF_HELPER_FLAGS_7 and
thus the MAX_OPC_PARAM_IARGS should be 7.
Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
Signed-off-by: Ziqiao Kong <ziqiaokong@gmail.com>
Message-Id: <20220227113127.414533-2-ziqiaokong@gmail.com>
Fixes: e6cadf49c3 ("tcg: Add support for a helper with 7 arguments")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
valgrind pointed out that arg_info()->val can be undefined which will
be the case if the arguments are not constant. The ordering of the
checks will have ensured we never relied on an undefined value but for
the sake of completeness re-order the code to be clear.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20220209112142.3367525-1-alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Otherwise, the BDS might be freed while the job is running, which would
cause a use-after-free.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220304153729.711387-5-hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
.bdrv_amend_clean() says block drivers can use it to clean up what was
done in .bdrv_amend_pre_run(). Therefore, it should always be called
after .bdrv_amend_pre_run(), which means we need it to call it in the
JobDriver.free() callback, not in JobDriver.clean().
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220304153729.711387-3-hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Instead of failing the iotests if GNU sed is not available (or skipping
them completely in the check-block.sh script), it would be better to
simply skip the bash-based tests that rely on GNU sed, so that the other
tests could still be run. Thus we now explicitely use "gsed" (either as
direct program or as a wrapper around "sed" if it's the GNU version)
in the spots that rely on the GNU sed behavior. Statements that use the
"-r" parameter of sed have been switched to use "-E" instead, since this
switch is supported by all sed versions on our supported build hosts
(most also support "-r", but macOS' sed only supports "-E"). With all
these changes in place, we then can also remove the sed checks from the
check-block.sh script, so that "make check-block" can now be run on
systems without GNU sed, too.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20220216125454.465041-1-thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Test the following scenario:
1. Some block node (null-co) attached to a user (here: NBD server) that
performs I/O and keeps the node in an I/O thread
2. Repeatedly run blockdev-add/blockdev-del to add/remove an overlay
to/from that node
Each blockdev-add triggers bdrv_refresh_limits(), and because
blockdev-add runs in the main thread, it does not stop the I/O requests.
I/O can thus happen while the limits are refreshed, and when such a
request sees a temporarily invalid block limit (e.g. alignment is 0),
this may easily crash qemu (or the storage daemon in this case).
The block layer needs to ensure that I/O requests to a node are paused
while that node's BlockLimits are refreshed.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20220216105355.30729-4-hreitz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Add a parameter to optionally open a QMP connection when creating a
QemuStorageDaemon instance.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220216105355.30729-3-hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
bdrv_refresh_limits() recurses down to the node's children. That does
not seem necessary: When we refresh limits on some node, and then
recurse down and were to change one of its children's BlockLimits, then
that would mean we noticed the changed limits by pure chance. The fact
that we refresh the parent's limits has nothing to do with it, so the
reason for the change probably happened before this point in time, and
we should have refreshed the limits then.
Consequently, we should actually propagate block limits changes upwards,
not downwards. That is a separate and pre-existing issue, though, and
so will not be addressed in this patch.
The problem with recursing is that bdrv_refresh_limits() is not atomic.
It begins with zeroing BDS.bl, and only then sets proper, valid limits.
If we do not drain all nodes whose limits are refreshed, then concurrent
I/O requests can encounter invalid request_alignment values and crash
qemu. Therefore, a recursing bdrv_refresh_limits() requires the whole
subtree to be drained, which is currently not ensured by most callers.
A non-recursive bdrv_refresh_limits() only requires the node in question
to not receive I/O requests, and this is done by most callers in some
way or another:
- bdrv_open_driver() deals with a new node with no parents yet
- bdrv_set_file_or_backing_noperm() acts on a drained node
- bdrv_reopen_commit() acts only on drained nodes
- bdrv_append() should in theory require the node to be drained; in
practice most callers just lock the AioContext, which should at least
be enough to prevent concurrent I/O requests from accessing invalid
limits
So we can resolve the bug by making bdrv_refresh_limits() non-recursive.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1879437
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20220216105355.30729-2-hreitz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Message-Id: <20220303151616.325444-32-eesposit@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
