Mea culpa. Dan's patch wound up with the wrong import path because I
re-ordered my most recent pull request and missed that this needed a fix
on rebase.
Fixes: 43912529
Reported-by: Kashyap Chamarthy <kchamart@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Tested-by: Kashyap Chamarthy <kchamart@redhat.com>
Message-id: 20220225170828.3418305-1-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
_bind_hack() was a quick fix to allow async QMP to call bind(2) prior to
calling listen(2) and accept(2). This wasn't sufficient to fully address
the race condition present in synchronous clients.
With the race condition in legacy.py fixed (see the previous commit),
there are no longer any users of _bind_hack(). Drop it.
Fixes: b0b662bb2b
Signed-off-by: John Snow <jsnow@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20220225205948.3693480-11-jsnow@redhat.com
[Expanded commit message. --js]
Signed-off-by: John Snow <jsnow@redhat.com>
legacy.py provides a synchronous model. iotests frequently uses this
paradigm:
- create QMP client object
- start QEMU process
- await connection from QEMU process
In the switch from sync to async QMP, the QMP client object stopped
calling bind() and listen() during the QMP object creation step, which
creates a race condition if the QEMU process dials in too quickly.
With refactoring out of the way, restore the former behavior of calling
bind() and listen() during __init__() to fix this race condition.
Signed-off-by: John Snow <jsnow@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20220225205948.3693480-10-jsnow@redhat.com
[Expanded commit message. --js]
Signed-off-by: John Snow <jsnow@redhat.com>
Add start_server() and accept() methods that can be used instead of
start_server_and_accept() to allow more fine-grained control over the
incoming connection process.
(Eagle-eyed reviewers will surely notice that it's a bit weird that
"CONNECTING" is a state that's shared between both the start_server()
and connect() states. That's absolutely true, and it's very true that
checking on the presence of _accepted as an indicator of state is a
hack. That's also very certainly true. But ... this keeps client code an
awful lot simpler, as it doesn't have to care exactly *how* the
connection is being made, just that it *is*. Is it worth disrupting that
simplicity in order to provide a better state guard on `accept()`? Hm.)
Signed-off-by: John Snow <jsnow@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20220225205948.3693480-9-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
Before we allow the full separation of starting the server and accepting
new connections, make sure that the disconnect cleans up the server and
its new state, too.
Signed-off-by: John Snow <jsnow@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20220225205948.3693480-8-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
Refactor _do_accept() into _do_start_server() and _do_accept(). As of
this commit, the former calls the latter, but in subsequent commits
they'll be split apart.
(So please forgive the misnomer for _do_start_server(); it will live up
to its name shortly, and the docstring will be updated then too. I'm
just cutting down on some churn.)
Signed-off-by: John Snow <jsnow@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20220225205948.3693480-7-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
I would really like to keep this under 1000 lines, I promise. Doesn't
look like it's gonna happen.
Signed-off-by: John Snow <jsnow@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20220225205948.3693480-6-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
As part of disentangling the monolithic nature of _do_accept(), split
out the incoming callback to prepare for factoring out the "wait for a
peer" step. Namely, this means using an event signal we can wait on from
outside of this method.
Signed-off-by: John Snow <jsnow@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20220225205948.3693480-5-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
These two methods attempted to entirely envelop the logic of
establishing a connection to a peer start to finish. However, we need to
break apart the incoming connection step into more granular steps. We
will no longer be able to reasonably constrain the logic inside of these
helper functions.
So, remove them - with _session_guard(), they no longer serve a real
purpose.
Although the public API doesn't change, the internal API does. Now that
there are no intermediary methods between e.g. connect() and
_do_connect(), there's no hook where the runstate is set. As a result,
the test suite changes a little to cope with the new semantics of
_do_accept() and _do_connect().
Lastly, take some pieces of the now-deleted docstrings and move
them up to the public interface level. They were a little more detailed,
and it won't hurt to keep them.
Signed-off-by: John Snow <jsnow@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20220225205948.3693480-4-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
Previously, I had a method named "accept()" that under-the-hood calls
bind(2), listen(2) *and* accept(2). I meant this as a simplification and
counterpart to the one-shot "connect()" method.
This is confusing to readers who expect accept() to mean *just*
accept(2). Since I need to split apart the "accept()" method into
multiple methods anyway (one of which strongly resembling accept(2)), it
feels pertinent to rename this method *now*.
Rename this all-in-one method "start_server_and_accept()" instead.
Signed-off-by: John Snow <jsnow@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20220225205948.3693480-3-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
In _new_session, there's a fairly complex except clause that's used to
give semantic errors to callers of accept() and connect(). We need to
create a new two-step replacement for accept(), so factoring out this
piece of logic will be useful.
Bolster the comments and docstring here to try and demystify what's
going on in this fairly delicate piece of Python magic.
(If we were using Python 3.7+, this would be an @asynccontextmanager. We
don't have that very nice piece of magic, however, so this must take an
Awaitable to manage the Exception contexts properly. We pay the price
for platform compatibility.)
Signed-off-by: John Snow <jsnow@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20220225205948.3693480-2-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
- New fleecing backup scheme
- iotest fixes
- Fixes for the curl block driver
- Fix for the preallocate block driver
- IDE fix for zero-length TRIM requests
-----BEGIN PGP SIGNATURE-----
iQJGBAABCAAwFiEEy2LXoO44KeRfAE00ofpA0JgBnN8FAmIl33sSHGhyZWl0ekBy
ZWRoYXQuY29tAAoJEKH6QNCYAZzfD60P/0xXwKmXiyq7GkoVdkZaI4ErMn29rnzl
APoM6Q3GbMII6ipkqVIFM2BIhGDCl8X+KbzvDTm23jvXR7SAnjel+GBcFYTZ4/3r
LbS/mW3SGPjEG6CldXjHDxD/k+3sw1wXOGW30+PhEkpokNSJKh3evsNxUA+liPQT
MqrNWilh28S4CMM2YnHEJNX/e0WujYagE0PN4UKrr/+bw8mN+UrirCC/BwH0RsAz
H+OVek/SaGUer4MAXNH+LVWwm3/F+sZQBqyLZc0ORavKnXEPlf/fKbGx5+IkV2ty
h/GX+oxLSsfnXDVhqvP71RbSkLIOCsrSPzTqz3vicBmwtqaIzjor7wVS/vJlK1i5
cVEYa2te3Ccbpijb9VOBwTu+t8YB8x/Ot8S22p++uC0CY/Gf1paCe8uNqXHQYhfD
Sv2Xw9vQsCIvB+7vmJ+iMSf5LFPO0I4Hh7zil6ecKgtxtkVLtqYa7t1UE/5cOUtO
mF3mzS7bQhcU569z3+hZEBiCrH+rSNvHvnnAV/IFN4Y1PqSBk6uwoqHzUFZQq6Dr
6IdduqUmGs9ooDZuxKgShr81Sn18T6BkawcsQMMHOI4gKRik3l7kxUnPeTIYWi2T
j0ihaZyH5rRfa80CBnN18Q+90buYb0OafKZ9y4/RDOScoHeR8Yx/RjUwKpn+6yGI
TOUr+3PvGOPl
=OH/S
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/hreitz-gitlab/tags/pull-block-2022-03-07' into staging
Block patches for 7.0-rc0:
- New fleecing backup scheme
- iotest fixes
- Fixes for the curl block driver
- Fix for the preallocate block driver
- IDE fix for zero-length TRIM requests
# gpg: Signature made Mon 07 Mar 2022 10:33:31 GMT
# gpg: using RSA key CB62D7A0EE3829E45F004D34A1FA40D098019CDF
# gpg: issuer "hreitz@redhat.com"
# gpg: Good signature from "Hanna Reitz <hreitz@redhat.com>" [marginal]
# gpg: WARNING: This key is not certified with sufficiently trusted signatures!
# gpg: It is not certain that the signature belongs to the owner.
# Primary key fingerprint: CB62 D7A0 EE38 29E4 5F00 4D34 A1FA 40D0 9801 9CDF
* remotes/hreitz-gitlab/tags/pull-block-2022-03-07: (23 commits)
iotests/image-fleecing: test push backup with fleecing
iotests/image-fleecing: add test case with bitmap
iotests.py: add qemu_io_pipe_and_status()
iotests/image-fleecing: add test-case for fleecing format node
block: copy-before-write: realize snapshot-access API
block: introduce snapshot-access block driver
block/io: introduce block driver snapshot-access API
block/reqlist: add reqlist_wait_all()
block/dirty-bitmap: introduce bdrv_dirty_bitmap_status()
block/reqlist: reqlist_find_conflict(): use ranges_overlap()
block: intoduce reqlist
block/block-copy: add block_copy_reset()
block/copy-before-write: add bitmap open parameter
block/block-copy: block_copy_state_new(): add bitmap parameter
block/dirty-bitmap: bdrv_merge_dirty_bitmap(): add return value
block/block-copy: move copy_bitmap initialization to block_copy_state_new()
iotests: Write test output to TEST_DIR
tests/qemu-iotests/testrunner: Quote "case not run" lines in TAP mode
tests/qemu-iotests/040: Skip TestCommitWithFilters without 'throttle'
block: fix preallocate filter: don't do unaligned preallocate requests
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20220303194349.2304213-17-vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Note that reads zero areas (not dirty in the bitmap) fails, that's
correct.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20220303194349.2304213-16-vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Add helper that returns both status and output, to be used in the
following commit
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20220303194349.2304213-15-vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220303194349.2304213-14-vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Current scheme of image fleecing looks like this:
[guest] [NBD export]
| |
|root | root
v v
[copy-before-write] -----> [temp.qcow2]
| target |
|file |backing
v |
[active disk] <-------------+
- On guest writes copy-before-write filter copies old data from active
disk to temp.qcow2. So fleecing client (NBD export) when reads
changed regions from temp.qcow2 image and unchanged from active disk
through backing link.
This patch makes possible new image fleecing scheme:
[guest] [NBD export]
| |
| root | root
v file v
[copy-before-write]<------[snapshot-access]
| |
| file | target
v v
[active-disk] [temp.img]
- copy-before-write does CBW operations and also provides
snapshot-access API. The API may be accessed through
snapshot-access driver.
Benefits of new scheme:
1. Access control: if remote client try to read data that not covered
by original dirty bitmap used on copy-before-write open, client gets
-EACCES.
2. Discard support: if remote client do DISCARD, this additionally to
discarding data in temp.img informs block-copy process to not copy
these clusters. Next read from discarded area will return -EACCES.
This is significant thing: when fleecing user reads data that was
not yet copied to temp.img, we can avoid copying it on further guest
write.
3. Synchronisation between client reads and block-copy write is more
efficient. In old scheme we just rely on BDRV_REQ_SERIALISING flag
used for writes to temp.qcow2. New scheme is less blocking:
- fleecing reads are never blocked: if data region is untouched or
in-flight, we just read from active-disk, otherwise we read from
temp.img
- writes to temp.img are not blocked by fleecing reads
- still, guest writes of-course are blocked by in-flight fleecing
reads, that currently read from active-disk - it's the minimum
necessary blocking
4. Temporary image may be of any format, as we don't rely on backing
feature.
5. Permission relation are simplified. With old scheme we have to share
write permission on target child of copy-before-write, otherwise
backing link conflicts with copy-before-write file child write
permissions. With new scheme we don't have backing link, and
copy-before-write node may have unshared access to temporary node.
(Not realized in this commit, will be in future).
6. Having control on fleecing reads we'll be able to implement
alternative behavior on failed copy-before-write operations.
Currently we just break guest request (that's a historical behavior
of backup). But in some scenarios it's a bad behavior: better
is to drop the backup as failed but don't break guest request.
With new scheme we can simply unset some bits in a bitmap on CBW
failure and further fleecing reads will -EACCES, or something like
this. (Not implemented in this commit, will be in future)
Additional application for this is implementing timeout for CBW
operations.
Iotest 257 output is updated, as two more bitmaps now live in
copy-before-write filter.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20220303194349.2304213-13-vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
The new block driver simply utilizes snapshot-access API of underlying
block node.
In further patches we want to use it like this:
[guest] [NBD export]
| |
| root | root
v file v
[copy-before-write]<------[snapshot-access]
| |
| file | target
v v
[active-disk] [temp.img]
This way, NBD client will be able to read snapshotted state of active
disk, when active disk is continued to be written by guest. This is
known as "fleecing", and currently uses another scheme based on qcow2
temporary image which backing file is active-disk. New scheme comes
with benefits - see next commit.
The other possible application is exporting internal snapshots of
qcow2, like this:
[guest] [NBD export]
| |
| root | root
v file v
[qcow2]<---------[snapshot-access]
For this, we'll need to implement snapshot-access API handlers in
qcow2 driver, and improve snapshot-access block driver (and API) to
make it possible to select snapshot by name. Another thing to improve
is size of snapshot. Now for simplicity we just use size of bs->file,
which is OK for backup, but for qcow2 snapshots export we'll need to
imporve snapshot-access API to get size of snapshot.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20220303194349.2304213-12-vsementsov@virtuozzo.com>
[hreitz: Rebased on block GS/IO split]
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Add new block driver handlers and corresponding generic wrappers.
It will be used to allow copy-before-write filter to provide
reach fleecing interface in further commit.
In future this approach may be used to allow reading qcow2 internal
snapshots, for example to export them through NBD.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220303194349.2304213-11-vsementsov@virtuozzo.com>
[hreitz: Rebased on block GS/IO split]
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Add function to wait for all intersecting requests.
To be used in the further commit.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Nikita Lapshin <nikita.lapshin@virtuozzo.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220303194349.2304213-10-vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Add a convenient function similar with bdrv_block_status() to get
status of dirty bitmap.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220303194349.2304213-9-vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Split intersecting-requests functionality out of block-copy to be
reused in copy-before-write filter.
Note: while being here, fix tiny typo in MAINTAINERS.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220303194349.2304213-7-vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Split block_copy_reset() out of block_copy_reset_unallocated() to be
used separately later.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220303194349.2304213-6-vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
This brings "incremental" mode to copy-before-write filter: user can
specify bitmap so that filter will copy only "dirty" areas.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20220303194349.2304213-5-vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
This will be used in the following commit to bring "incremental" mode
to copy-before-write filter.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220303194349.2304213-4-vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
That simplifies handling failure in existing code and in further new
usage of bdrv_merge_dirty_bitmap().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220303194349.2304213-3-vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
We are going to complicate bitmap initialization in the further
commit. And in future, backup job will be able to work without filter
(when source is immutable), so we'll need same bitmap initialization in
copy-before-write filter and in backup job. So, it's reasonable to do
it in block-copy.
Note that for now cbw_open() is the only caller of
block_copy_state_new().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220303194349.2304213-2-vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Drop the use of OUTPUT_DIR (test/qemu-iotests under the build
directory), and instead write test output files (.out.bad, .notrun, and
.casenotrun) to TEST_DIR.
With this, the same test can be run concurrently without the separate
instances interfering, because they will need separate TEST_DIRs anyway.
Running the same test separately is useful when running the iotests with
various format/protocol combinations in parallel, or when you just want
to aggressively exercise a single test (e.g. when it fails only
sporadically).
Putting this output into TEST_DIR means that it will stick around for
inspection after the test run is done (though running the same test in
the same TEST_DIR will overwrite it, just as it used to be); but given
that TEST_DIR is a scratch directory, it should be clear that users can
delete all of its content at any point. (And if TEST_DIR is on tmpfs,
it will just disappear on shutdown.) Contrarily, alternative approaches
that would put these output files into OUTPUT_DIR with some prefix to
differentiate between separate test runs might easily lead to cluttering
OUTPUT_DIR.
(This change means OUTPUT_DIR is no longer written to by the iotests, so
we can drop its usage altogether.)
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220221172909.762858-1-hreitz@redhat.com>
[hreitz: Simplified `Path(os.path.join(x, y))` to `Path(x, y)`, as
suggested by Vladimir; and rebased on 9086c76398
("tests/qemu-iotests: Rework the checks and spots using GNU
sed")]
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
In TAP mode, the stdout is reserved for the TAP protocol, so we
have to make sure to mark other lines with a comment '#' character
at the beginning to avoid that the TAP parser at the other end
gets confused.
To test this condition, run "configure" for example with:
--block-drv-rw-whitelist=copy-before-write,qcow2,raw,file,host_device,blkdebug,null-co,copy-on-read
so that iotest 041 will report that some tests are not run due to
the missing "quorum" driver. Without this change, "make check-block"
fails since the meson tap parser gets confused by these messages.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20220223124353.3273898-1-thuth@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
iotest 040 already has some checks for the availability of the 'throttle'
driver, but some new code has been added in the course of time that
depends on 'throttle' but does not check for its availability. Add
a check to the TestCommitWithFilters class so that this iotest now
also passes again if 'throttle' has not been enabled in the QEMU
binaries.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20220223123127.3206042-1-thuth@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
There is a bug in handling BDRV_REQ_NO_WAIT flag: we still may wait in
wait_serialising_requests() if request is unaligned. And this is
possible for the only user of this flag (preallocate filter) if
underlying file is unaligned to its request_alignment on start.
So, we have to fix preallocate filter to do only aligned preallocate
requests.
Next, we should fix generic block/io.c somehow. Keeping in mind that
preallocate is the only user of BDRV_REQ_NO_WAIT and that we have to
fix its behavior now, it seems more safe to just assert that we never
use BDRV_REQ_NO_WAIT with unaligned requests and add corresponding
comment. Let's do so.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Message-Id: <20220215121609.38570-1-vsementsov@virtuozzo.com>
[hreitz: Rebased on block GS/IO split]
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Coverity points out that we aren't checking the return value
from curl_easy_setopt() for any of the calls to it we make
in block/curl.c.
Some of these options are documented as always succeeding (e.g.
CURLOPT_VERBOSE) but others have documented failure cases (e.g.
CURLOPT_URL). For consistency we check every call, even the ones
that theoretically cannot fail.
Fixes: Coverity CID 1459336, 1459482, 1460331
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20220222152341.850419-3-peter.maydell@linaro.org>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
In curl_open(), the 'out' label assumes that the state->errmsg string
has been set (either by curl_easy_perform() or by manually copying a
string into it); however if curl_init_state() fails we will jump to
that label without setting the string. Add the missing error string
setup.
(We can't be specific about the cause of failure: the documentation
of curl_easy_init() just says "If this function returns NULL,
something went wrong".)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20220222152341.850419-2-peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
When we still have an AIOCB registered for DMA operations, we try to
settle the respective operation by draining the BlockBackend associated
with the IDE device.
However, this assumes that every DMA operation is associated with an
increment of the BlockBackend’s in-flight counter (e.g. through some
ongoing I/O operation), so that draining the BB until its in-flight
counter reaches 0 will settle all DMA operations. That is not the case:
For TRIM, the guest can issue a zero-length operation that will not
result in any I/O operation forwarded to the BlockBackend, and also not
increment the in-flight counter in any other way. In such a case,
blk_drain() will be a no-op if no other operations are in flight.
It is clear that if blk_drain() is a no-op, the value of
s->bus->dma->aiocb will not change between checking it in the `if`
condition and asserting that it is NULL after blk_drain().
The particular problem is that ide_issue_trim() creates a BH
(ide_trim_bh_cb()) to settle the TRIM request: iocb->common.cb() is
ide_dma_cb(), which will either create a new request, or find the
transfer to be done and call ide_set_inactive(), which clears
s->bus->dma->aiocb. Therefore, the blk_drain() must wait for
ide_trim_bh_cb() to run, which currently it will not always do.
To fix this issue, we increment the BlockBackend's in-flight counter
when the TRIM operation begins (in ide_issue_trim(), when the
ide_trim_bh_cb() BH is created) and decrement it when ide_trim_bh_cb()
is done.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2029980
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220120142259.120189-1-hreitz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Tested-by: John Snow <jsnow@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>
