The offset field of an uncompressed cluster's L2 entry must be aligned
to the cluster size, otherwise it is invalid. If the cluster has no
data then it means that the offset points to a preallocation, so we
can clear the offset field without affecting the guest-visible data.
This is what 'qemu-img check' does when run in repair mode.
On traditional qcow2 images this can only happen when QCOW_OFLAG_ZERO
is set, and repairing such entries turns the clusters from ZERO_ALLOC
into ZERO_PLAIN.
Extended L2 entries have no ZERO_ALLOC clusters and no QCOW_OFLAG_ZERO
but the idea is the same: if none of the subclusters are allocated
then we can clear the offset field and leave the bitmap untouched.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <9f4ed1d0a34b0a545b032c31ecd8c14734065342.1594396418.git.berto@igalia.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200820150725.68687-4-mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Eric Blake <eblake@redhat.com>
[eblake: fold in python cleanups recommended by Vladimir]
Signed-off-by: Eric Blake <eblake@redhat.com>
Let wait_migration() return on failure (with the return value indicating
whether the migration was completed or has failed), so we can use it for
migrations that are expected to fail, too.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200820150725.68687-3-mreitz@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Extend the test case #303 by dumping QCOW2 image metadata in JSON
format.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <1596742557-320265-12-git-send-email-andrey.shinkevich@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
As __dict__ is being extended with class members we do not want to
print, add the to_json() method to classes that returns a json-dumpable
object with desired fields and their values. Extend it in subclass when
necessary to print the final dictionary in the JSON output which
follows.
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <1596742557-320265-10-git-send-email-andrey.shinkevich@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Add the command key to the qcow2.py arguments list to dump QCOW2
metadata in JSON format. Here is the suggested way to do that. The
implementation of the dump in JSON format is in the patch that follows.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <1596742557-320265-9-git-send-email-andrey.shinkevich@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
The cluster size of an image is the QcowHeader class member and may be
obtained by dependent extension structures such as Qcow2BitmapExt for
further bitmap table details print.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <1596742557-320265-7-git-send-email-andrey.shinkevich@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Introduce the class BitmapFlags that parses a bitmap flags mask.
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <1596742557-320265-5-git-send-email-andrey.shinkevich@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
There are two ways to initialize a class derived from Qcow2Struct:
1. Pass a block of binary data to the constructor.
2. Pass the file descriptor to allow reading the file from constructor.
Let's change the Qcow2BitmapExt initialization method from 1 to 2 to
support a scattered reading in the initialization chain.
The implementation comes with the patch that follows.
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <1596742557-320265-4-git-send-email-andrey.shinkevich@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Let us differ binary data type from string one for the extension data
variable and keep the string as the QcowHeaderExtension class member.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <1596742557-320265-3-git-send-email-andrey.shinkevich@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
The simple script creates a QCOW2 image and fills it with some data.
Two bitmaps are created as well. Then the script reads the image header
with extensions from the disk by running the script qcow2.py and dumps
the information to the output. Other entities, such as snapshots, may
be added to the test later.
Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <1596742557-320265-2-git-send-email-andrey.shinkevich@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
The binaries move to the root directory, e.g. qemu-system-i386 or
qemu-arm. This requires changes to qtests, CI, etc.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Start a VM with a 4097 byte image attached, add a 4096 byte granularity
dirty bitmap, mark it dirty, and then do a backup.
This used to run into an assert and fail, check that it works as
expected and also check the created image to ensure that misaligned
backups in general work correctly.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Message-Id: <20200810095523.15071-2-s.reiter@proxmox.com>
[mreitz: Drop bitmap, and do not write past the image's end]
Signed-off-by: Max Reitz <mreitz@redhat.com>
Test migrating from a VM with a persistent bitmap in the backing chain,
and then continuing that VM after the migration
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200730120234.49288-3-mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Add test for "qemu-img convert -O qcow2 -c" to NBD target. The tests
create a OVA file and write compressed qcow2 disk content directly into
the OVA file via qemu-nbd.
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
Message-Id: <20200727215846.395443-5-nsoffer@redhat.com>
Tested-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Add 2 helpers for measuring and checking images:
- qemu_img_measure()
- qemu_img_check()
Both use --output-json and parse the returned json to make easy to use
in other tests. I'm going to use them in a new test, and I hope they
will be useful in may other tests.
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
Message-Id: <20200727215846.395443-4-nsoffer@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Instead of duplicating the code to wait until the server is ready and
remember to terminate the server and wait for it, make it possible to
use like this:
with qemu_nbd_popen('-k', sock, image):
# Access image via qemu-nbd socket...
Only test 264 used this helper, but I had to modify the output since it
did not consistently when starting and stopping qemu-nbd.
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
Message-Id: <20200727215846.395443-3-nsoffer@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
- Improve handling of various post-copy bitmap migration scenarios. A lost
bitmap should merely mean that the next backup must be full rather than
incremental, rather than abruptly breaking the entire guest migration.
- Associated iotest improvements
-----BEGIN PGP SIGNATURE-----
iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAl8fPRkACgkQp6FrSiUn
Q2qanQf/dRTrqZ7/hs8aENySf44o0dBzOLZr+FBcrqEj2sd0c6jPzV2X5CVtnA1v
gBgKJJGLpti3mSeNQDbaXZIQrsesBAuxvJsc6vZ9npDCdMYnK/qPE3Zfw1bx12qR
cb39ba28P4izgs216h92ZACtUewnvjkxyJgN7zfmCJdNcwZINMUItAS183tSbQjn
n39Wb7a+umsRgV9HQv/6cXlQIPqFMyAOl5kkzV3evuw7EBoHFnNq4cjPrUnjkqiD
xf2pcSomaedYd37SpvoH57JxfL3z/90OBcuXhFvbqFk4FgQ63rJ32nRve2ZbIDI0
XPbohnYjYoFv6Xs/jtTzctZCbZ+jTg==
=1dmz
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/ericb/tags/pull-bitmaps-2020-07-27' into staging
bitmaps patches for 2020-07-27
- Improve handling of various post-copy bitmap migration scenarios. A lost
bitmap should merely mean that the next backup must be full rather than
incremental, rather than abruptly breaking the entire guest migration.
- Associated iotest improvements
# gpg: Signature made Mon 27 Jul 2020 21:46:17 BST
# gpg: using RSA key 71C2CC22B1C4602927D2F3AAA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>" [full]
# gpg: aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>" [full]
# gpg: aka "[jpeg image of size 6874]" [full]
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2 F3AA A7A1 6B4A 2527 436A
* remotes/ericb/tags/pull-bitmaps-2020-07-27: (24 commits)
migration: Fix typos in bitmap migration comments
iotests: Adjust which migration tests are quick
qemu-iotests/199: add source-killed case to bitmaps postcopy
qemu-iotests/199: add early shutdown case to bitmaps postcopy
qemu-iotests/199: check persistent bitmaps
qemu-iotests/199: prepare for new test-cases addition
migration/savevm: don't worry if bitmap migration postcopy failed
migration/block-dirty-bitmap: cancel migration on shutdown
migration/block-dirty-bitmap: relax error handling in incoming part
migration/block-dirty-bitmap: keep bitmap state for all bitmaps
migration/block-dirty-bitmap: simplify dirty_bitmap_load_complete
migration/block-dirty-bitmap: rename finish_lock to just lock
migration/block-dirty-bitmap: refactor state global variables
migration/block-dirty-bitmap: move mutex init to dirty_bitmap_mig_init
migration/block-dirty-bitmap: rename dirty_bitmap_mig_cleanup
migration/block-dirty-bitmap: rename state structure types
migration/block-dirty-bitmap: fix dirty_bitmap_mig_before_vm_start
qemu-iotests/199: increase postcopy period
qemu-iotests/199: change discard patterns
qemu-iotests/199: improve performance: set bitmap by discard
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
While 197 is very much a qcow2 test, and it looks like the partial
cluster case at the end (introduced in b0ddcbbb36) is specifically
a qcow2 case, the whole test scripts actually marks itself to work with
generic formats (and generic protocols, even).
Said partial cluster case happened to work with non-qcow2 formats as
well (mostly by accident), but 1855536256 broke that, because it sets
the compat option, which does not work for non-qcow2 formats.
So go the whole way and force IMGFMT=qcow2 and IMGPROTO=file, as done in
other places in this test.
Fixes: 1855536256
("iotests/197: Fix for compat=0.10")
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200728131134.902519-1-mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
A quick run of './check -qcow2 -g migration' shows that test 169 is
NOT quick, but meanwhile several other tests ARE quick. Let's adjust
the test designations accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200727195117.132151-1-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Previous patches fixes behavior of bitmaps migration, so that errors
are handled by just removing unfinished bitmaps, and not fail or try to
recover postcopy migration. Add corresponding test.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Tested-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200727194236.19551-22-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Previous patches fixed two crashes which may occur on shutdown prior to
bitmaps postcopy finished. Check that it works now.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Tested-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200727194236.19551-21-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Check that persistent bitmaps are not stored on source and that bitmaps
are persistent on destination.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <20200727194236.19551-20-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Move future common part to start_postcopy() method. Move checking
number of bitmaps to check_bitmap().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <20200727194236.19551-19-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
The test wants to force a bitmap postcopy. Still, the resulting
postcopy period is very small. Let's increase it by adding more
bitmaps to migrate. Also, test disabled bitmaps migration.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Tested-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200727194236.19551-7-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
iotest 199 works too long because of many discard operations. At the
same time, postcopy period is very short, in spite of all these
efforts.
So, let's use less discards (and with more interesting patterns) to
reduce test timing. In the next commit we'll increase postcopy period.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Tested-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200727194236.19551-6-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Discard dirties dirty-bitmap as well as write, but works faster. Let's
use it instead.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Tested-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200727194236.19551-5-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
The test aims to test _postcopy_ migration, and wants to do some write
operations during postcopy time.
Test considers migrate status=complete event on source as start of
postcopy. This is completely wrong, completion is completion of the
whole migration process. Let's instead consider destination start as
start of postcopy, and use RESUME event for it.
Next, as migration finish, let's use migration status=complete event on
target, as such method is closer to what libvirt or another user will
do, than tracking number of dirty-bitmaps.
Finally, add a possibility to dump events for debug. And if
set debug to True, we see, that actual postcopy period is very small
relatively to the whole test duration time (~0.2 seconds to >40 seconds
for me). This means, that test is very inefficient in what it supposed
to do. Let's improve it in following commits.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Tested-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200727194236.19551-4-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
We don't need any specific format constraints here. Still keep qcow2
for two reasons:
1. No extra calls of format-unrelated test
2. Add some check around persistent bitmap in future (require qcow2)
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Tested-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200727194236.19551-3-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Writing zeroes to a qcow2 v2 images without a backing file results in an
unallocated cluster as of 61b3043965. 197 has a test for COR-ing a
cluster on an image without a backing file, which means that the data
will be zero, so now on a v2 image that cluster will just stay
unallocated, and so the test fails. Just force compat=1.1 for that
particular case to enforce the cluster to get allocated.
Fixes: 61b3043965
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200727135237.1096841-1-mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
If you are building only with either the new rx-softmmu or avr-softmmu
target, "make check-block" fails a couple of tests since there is no
default machine defined in these new targets. We have to select a machine
in the "check" script for these, just like we already do for the arm- and
tricore-softmmu targets.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20200722161908.25383-1-thuth@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200721135520.72355-3-kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Test that we can't write-share raw luks images by default,
but we still can with share-rw=on
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Message-Id: <20200719122059.59843-3-mlevitsk@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Test shutdown when bitmap is exported through NBD and active client
exists. The previous patch fixes a crash, provoked by this scenario.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Tested-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200714162234.13113-3-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
It can happen that the throttling of the stream job doesn't make it slow
enough that we can be sure that it still exists when it is referenced
again. Just use a much smaller speed to make this very unlikely to
happen again.
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200716132829.20127-1-kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The unit tests using the x509 crypto functionality have started
failing in Fedora 33 rawhide with a message like
The certificate uses an insecure algorithm
This is result of Fedora changes to support strong crypto [1]. RSA
with 1024 bit key is viewed as legacy and thus insecure. Generate
a new private key which is 3072 bits long and reasonable future
proof.
[1] https://fedoraproject.org/wiki/Changes/StrongCryptoSettings2
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20200715154701.1041325-1-berrange@redhat.com>
Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The vxhs code doesn't compile since v2.12.0. There's no point in fixing
and then adding CI for a config that our users have demonstrated that
they do not use; better to just remove it.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200711065926.2204721-1-marcandre.lureau@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
- Reduce race conditions on QEMUMachine::shutdown()
1. Remove the "bare except" pattern in the existing shutdown code,
which can mask problems and make debugging difficult.
2. Ensure that post-shutdown cleanup is always performed, even when
graceful termination fails.
3. Unify cleanup paths such that no matter how the VM is terminated,
the same functions and steps are always taken to reset the object
state.
4. Rewrite shutdown() such that any error encountered when attempting
a graceful shutdown will be raised as an AbnormalShutdown exception.
The pythonic idiom is to allow the caller to decide if this is a
problem or not.
- Modify part of the python/qemu library to comply with:
. mypy --strict
. pylint
. flake8
- Script for the TCG Continuous Benchmarking project that uses
callgrind to dissect QEMU execution into three main phases:
. code generation
. JIT execution
. helpers execution
CI jobs results:
. https://cirrus-ci.com/build/5421349961203712
. https://gitlab.com/philmd/qemu/-/pipelines/166556001
. https://travis-ci.org/github/philmd/qemu/builds/708102347
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE+qvnXhKRciHc/Wuy4+MsLN6twN4FAl8OGCUACgkQ4+MsLN6t
wN52ZQ//cB8pRhOeirMzPTamtAePAHhU9kRKSQ4fFzVM/AHxaO3qO3KrMKAv4JnR
1fwdyAbum0Dv+HrRAXbIVz/lgrlTb6tLqwk20YWlmNUR2CtMmno9fPQzg6LeFWBe
+qr2/anXWBqRZu5uHzQ3Ly3LxlVXE3qmStxlttNxed9wK9wdhsQ4ijOrExXPmMI5
IQi9tKijsGvOczcSpIwJET6Px0L/GIYHSFGXfHv4LiJCMQUX4gqY1akr+/2adqlr
ElcOHlvWG90GHftTC+hq+KbSNmpkwJEnXLHC+HwMaYmQuhH4+U6f9AcrnzYGHjoQ
84muuYt5CBvT/Q38qO3dUsr42oD8uiDZe2cpD5DmcAHj6U9sza4UwU2Nkd4cNvRD
OiYRWuHQDzQhwPGbzBym1GRxRlejmPXCpvZ4XqaYODkzhEHi8UVq5AOsNhpTGWVk
WoDZ3NtnZb9WbU93wDordhWTtHKHOLHNk4CuJLbdRvkW+sg8aFqUObSRvc6RqyU8
1DXWMMQr9UfCDfMMbp/OCymphn0VrAaixIRIjmYc6nL5mQlyugMwij55jzyugqBH
rN5bdcsqxAhBTD03lu0lbDn1BYIqbZFXABXeDnJNbxmY/6gD0E1HKWRvIKjxzH+t
L19QXsOWLoVUpUIjxlOgMlb9QJLPc9m/N3yVOanO0zVrMSBmOKQ=
=Jp6n
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/philmd-gitlab/tags/python-next-20200714' into staging
Python patches for 5.1
- Reduce race conditions on QEMUMachine::shutdown()
1. Remove the "bare except" pattern in the existing shutdown code,
which can mask problems and make debugging difficult.
2. Ensure that post-shutdown cleanup is always performed, even when
graceful termination fails.
3. Unify cleanup paths such that no matter how the VM is terminated,
the same functions and steps are always taken to reset the object
state.
4. Rewrite shutdown() such that any error encountered when attempting
a graceful shutdown will be raised as an AbnormalShutdown exception.
The pythonic idiom is to allow the caller to decide if this is a
problem or not.
- Modify part of the python/qemu library to comply with:
. mypy --strict
. pylint
. flake8
- Script for the TCG Continuous Benchmarking project that uses
callgrind to dissect QEMU execution into three main phases:
. code generation
. JIT execution
. helpers execution
CI jobs results:
. https://cirrus-ci.com/build/5421349961203712
. https://gitlab.com/philmd/qemu/-/pipelines/166556001
. https://travis-ci.org/github/philmd/qemu/builds/708102347
# gpg: Signature made Tue 14 Jul 2020 21:40:05 BST
# gpg: using RSA key FAABE75E12917221DCFD6BB2E3E32C2CDEADC0DE
# gpg: Good signature from "Philippe Mathieu-Daudé (F4BUG) <f4bug@amsat.org>" [full]
# Primary key fingerprint: FAAB E75E 1291 7221 DCFD 6BB2 E3E3 2C2C DEAD C0DE
* remotes/philmd-gitlab/tags/python-next-20200714:
python/qmp.py: add QMPProtocolError
python/qmp.py: add casts to JSON deserialization
python/qmp.py: Do not return None from cmd_obj
python/qmp.py: re-absorb MonitorResponseError
iotests.py: use qemu.qmp type aliases
python/qmp.py: Define common types
python/machine.py: change default wait timeout to 3 seconds
python/machine.py: re-add sigkill warning suppression
python/machine.py: split shutdown into hard and soft flavors
tests/acceptance: Don't test reboot on cubieboard
tests/acceptance: wait() instead of shutdown() where appropriate
python/machine.py: Make wait() call shutdown()
python/machine.py: Add a configurable timeout to shutdown()
python/machine.py: Prohibit multiple shutdown() calls
python/machine.py: Perform early cleanup for wait() calls, too
python/machine.py: Add _early_cleanup hook
python/machine.py: Close QMP socket in cleanup
python/machine.py: consolidate _post_shutdown()
scripts/performance: Add dissect.py script
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
iotests.py should use the type definitions from qmp.py instead of its
own.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200710052220.3306-3-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
- file-posix: Mitigate file fragmentation with extent size hints
- Tighten qemu-img rules on missing backing format
- qemu-img map: Don't limit block status request size
- Fix crash with virtio-scsi and iothreads
-----BEGIN PGP SIGNATURE-----
iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAl8NsgMRHGt3b2xmQHJl
ZGhhdC5jb20ACgkQfwmycsiPL9Z0tA//eqauxD7cTEpwrtLNrRtpiBtMG64BBpxz
QfkURzB38bMVahHlwq3Gt7Zcov8V4V7vxK66h688Z/fhw3vmqIeVe8+P6+Y5s9FL
jil8lewHuLTa6xELeugoV7SZXH8AAh1W2fQmiR7EPiOmpSE0wf7C5IShVlX8A04E
r0n09+61qGjRIe1hNTwTtldqQEfx6UGnxQWcQb81JUPA1lZhX3cnPg/j94Bofr+m
v/DbVTfsmUtTMjc0PdU7n4DKTWu8OS5B/X0unF21rTtO//cYBrhAeY3ax2jbFBWi
CIZK8HLI5m9/HFyltql1LOsd+B5TtfnXMfSdvDh2jaVUlto7wTeTnWU1fv4wxUB5
hk7XgJo/y203ebFNHpTmW8tvLfGTP8uqCVfOEFxzjy+JHGrarlbWkwL2LMOFFAZ2
s2WcwlfqiYGFTG4+OFdhPf9qPWKSqMr+jTdZJTse64/c6+YXWHk+pP9lfYEUOgSi
OYwdQUY9uiZ1K13q5Tif2TbFvs+c118xdTgVhAV7VtfPnWc3c647dX7iaq8Szknc
IT93670Iqf/PzEj+L7XUbbLIIsAcmxD0sr7QAQEt7bfiYIDRIQLiVPyzXplETFg2
SEkvtqBovm84ct7pWQzqA6lFvr3oIFDNquR40XFGozHNnlBeNi5s7pXQnqUBLElr
wDDuEi+z5QM=
=DB0q
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging
Block layer patches:
- file-posix: Mitigate file fragmentation with extent size hints
- Tighten qemu-img rules on missing backing format
- qemu-img map: Don't limit block status request size
- Fix crash with virtio-scsi and iothreads
# gpg: Signature made Tue 14 Jul 2020 14:24:19 BST
# 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/kevin/tags/for-upstream:
block: Avoid stale pointer dereference in blk_get_aio_context()
qemu-img: Deprecate use of -b without -F
block: Add support to warn on backing file change without format
iotests: Specify explicit backing format where sensible
qcow2: Deprecate use of qemu-img amend to change backing file
block: Error if backing file fails during creation without -u
qcow: Tolerate backing_fmt=
vmdk: Add trivial backing_fmt support
sheepdog: Add trivial backing_fmt support
block: Finish deprecation of 'qemu-img convert -n -o'
qemu-img: Flush stdout before before potential stderr messages
file-posix: Mitigate file fragmentation with extent size hints
iotests/059: Filter out disk size with more standard filter
qemu-img map: Don't limit block status request size
iotests: Simplify _filter_img_create() a bit
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Creating an image that requires format probing of the backing image is
potentially unsafe (we've had several CVEs over the years based on
probes leaking information to the guest on a subsequent boot, although
these days tools like libvirt are aware of the issue enough to prevent
the worst effects). For example, if our probing algorithm ever
changes, or if other tools like libvirt determine a different probe
result than we do, then subsequent use of that backing file under a
different format will present corrupted data to the guest.
Fortunately, the worst effects occur only when the backing image is
originally raw, and we at least prevent commit into a probed raw
backing file that would change its probed type.
Still, it is worth starting a deprecation clock so that future
qemu-img can refuse to create backing chains that would rely on
probing, to encourage clients to avoid unsafe practices. Most
warnings are intentionally emitted from bdrv_img_create() in the block
layer, but qemu-img convert uses bdrv_create() which cannot emit its
own warning without causing spurious warnings on other code paths. In
the end, all command-line image creation or backing file rewriting now
performs a check.
Furthermore, if we probe a backing file as non-raw, then it is safe to
explicitly record that result (rather than relying on future probes);
only where we probe a raw image do we care about further warnings to
the user when using such an image (for example, commits into a
probed-raw backing file are prevented), to help them improve their
tooling. But whether or not we make the probe results explicit, we
still warn the user to remind them to upgrade their workflow to supply
-F always.
iotest 114 specifically wants to create an unsafe image for later
amendment rather than defaulting to our new default of recording a
probed format, so it needs an update. While touching it, expand it to
cover all of the various warnings enabled by this patch. iotest 301
also shows a change to qcow messages.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-11-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
There are many existing qcow2 images that specify a backing file but
no format. This has been the source of CVEs in the past, but has
become more prominent of a problem now that libvirt has switched to
-blockdev. With older -drive, at least the probing was always done by
qemu (so the only risk of a changed format between successive boots of
a guest was if qemu was upgraded and probed differently). But with
newer -blockdev, libvirt must specify a format; if libvirt guesses raw
where the image was formatted, this results in data corruption visible
to the guest; conversely, if libvirt guesses qcow2 where qemu was
using raw, this can result in potential security holes, so modern
libvirt instead refuses to use images without explicit backing format.
The change in libvirt to reject images without explicit backing format
has pointed out that a number of tools have been far too reliant on
probing in the past. It's time to set a better example in our own
iotests of properly setting this parameter.
iotest calls to create, rebase, and convert are all impacted to some
degree. It's a bit annoying that we are inconsistent on command line
- while all of those accept -o backing_file=...,backing_fmt=..., the
shortcuts are different: create and rebase have -b and -F, while
convert has -B but no -F. (amend has no shortcuts, but the previous
patch just deprecated the use of amend to change backing chains).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-9-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The use of 'qemu-img amend' to change qcow2 backing files is not
tested very well. In particular, our implementation has a bug where
if a new backing file is provided without a format, then the prior
format is blindly reused, even if this results in data corruption, but
this is not caught by iotests.
There are also situations where amending other options needs access to
the original backing file (for example, on a downgrade to a v2 image,
knowing whether a v3 zero cluster must be allocated or may be left
unallocated depends on knowing whether the backing file already reads
as zero), but the command line does not have a nice way to tell us
both the backing file to use for opening the image as well as the
backing file to install after the operation is complete.
Even if we do allow changing the backing file, it is redundant with
the existing ability to change backing files via 'qemu-img rebase -u'.
It is time to deprecate this support (leaving the existing behavior
intact, even if it is buggy), and at a point in the future, require
the use of only 'qemu-img rebase' for adjusting backing chain
relations, saving 'qemu-img amend' for changes unrelated to the
backing chain.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-8-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Back in commit 6e6e55f5 (Jul 2017, v2.10), we tweaked the code to warn
if the backing file could not be opened but the user gave a size,
unless the user also passes the -u option to bypass the open of the
backing file. As one common reason for failure to open the backing
file is when there is mismatch in the requested backing format in
relation to what the backing file actually contains, we actually want
to open the backing file and ensure that it has the right format in as
many cases as possible. iotest 301 for qcow demonstrates how
detecting explicit format mismatch is useful to prevent the creation
of an image that would probe differently than the user requested. Now
is the time to finally turn the warning an error, as promised.
Note that the original warning was added prior to our documentation of
an official deprecation policy (eb22aeca, also Jul 2017), and because
the warning didn't mention the word "deprecated", we never actually
remembered to document it as such. But the warning has been around
long enough that I don't see prolonging it another two releases.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-7-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
qcow has no space in the metadata to store a backing format, and there
are existing qcow images backed both by raw or by other formats
(usually qcow) images, reliant on probing to tell the difference. On
the bright side, because we probe every time, raw files are marked as
probed and we thus forbid a commit action into the backing file where
guest-controlled contents could change the result of the probe next
time around (the iotest added here proves that).
Still, allowing the user to specify the backing format during
creation, even if we can't record it, is a good thing. This patch
blindly allows any value that resolves to a known driver, even if the
user's request is a mismatch from what probing finds; then the next
patch will further enhance things to verify that the user's request
matches what we actually probe. With this and the next patch in
place, we will finally be ready to deprecate the creation of images
where a backing format was not explicitly specified by the user.
Note that this is only for QemuOpts usage; there is no change to the
QAPI to allow a format through -blockdev.
Add a new iotest 301 just for qcow, to demonstrate the latest
behavior, and to make it easier to show the improvements made in the
next patch.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-6-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
It's been two releases since we started warning; time to make the
combination an error as promised. There was no iotest coverage, so
add some.
While touching the documentation, tweak another section heading for
consistent style.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-3-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
During 'qemu-img create ... 2>&1', if --quiet is not in force, we can
end up with buffered I/O in stdout that was produced before failure,
but which appears in output after failure. This is confusing; the fix
is to flush stdout prior to attempting anything that might produce an
error message. Several iotests demonstrate the resulting ordering
change now that the merged outputs now reflect chronology. (An even
better fix would be to avoid printf from within block.c altogether,
but that's much more invasive...)
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-2-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Especially when O_DIRECT is used with image files so that the page cache
indirection can't cause a merge of allocating requests, the file will
fragment on the file system layer, with a potentially very small
fragment size (this depends on the requests the guest sent).
On Linux, fragmentation can be reduced by setting an extent size hint
when creating the file (at least on XFS, it can't be set any more after
the first extent has been allocated), basically giving raw files a
"cluster size" for allocation.
This adds a create option to set the extent size hint, and changes the
default from not setting a hint to setting it to 1 MB. The main reason
why qcow2 defaults to smaller cluster sizes is that COW becomes more
expensive, which is not an issue with raw files, so we can choose a
larger size. The tradeoff here is only potentially wasted disk space.
For qcow2 (or other image formats) over file-posix, the advantage should
even be greater because they grow sequentially without leaving holes, so
there won't be wasted space. Setting even larger extent size hints for
such images may make sense. This can be done with the new option, but
let's keep the default conservative for now.
The effect is very visible with a test that intentionally creates a
badly fragmented file with qemu-img bench (the time difference while
creating the file is already remarkable) and then looks at the number of
extents and the time a simple "qemu-img map" takes.
Without an extent size hint:
$ ./qemu-img create -f raw -o extent_size_hint=0 ~/tmp/test.raw 10G
Formatting '/home/kwolf/tmp/test.raw', fmt=raw size=10737418240 extent_size_hint=0
$ ./qemu-img bench -f raw -t none -n -w ~/tmp/test.raw -c 1000000 -S 8192 -o 0
Sending 1000000 write requests, 4096 bytes each, 64 in parallel (starting at offset 0, step size 8192)
Run completed in 25.848 seconds.
$ ./qemu-img bench -f raw -t none -n -w ~/tmp/test.raw -c 1000000 -S 8192 -o 4096
Sending 1000000 write requests, 4096 bytes each, 64 in parallel (starting at offset 4096, step size 8192)
Run completed in 19.616 seconds.
$ filefrag ~/tmp/test.raw
/home/kwolf/tmp/test.raw: 2000000 extents found
$ time ./qemu-img map ~/tmp/test.raw
Offset Length Mapped to File
0 0x1e8480000 0 /home/kwolf/tmp/test.raw
real 0m1,279s
user 0m0,043s
sys 0m1,226s
With the new default extent size hint of 1 MB:
$ ./qemu-img create -f raw -o extent_size_hint=1M ~/tmp/test.raw 10G
Formatting '/home/kwolf/tmp/test.raw', fmt=raw size=10737418240 extent_size_hint=1048576
$ ./qemu-img bench -f raw -t none -n -w ~/tmp/test.raw -c 1000000 -S 8192 -o 0
Sending 1000000 write requests, 4096 bytes each, 64 in parallel (starting at offset 0, step size 8192)
Run completed in 11.833 seconds.
$ ./qemu-img bench -f raw -t none -n -w ~/tmp/test.raw -c 1000000 -S 8192 -o 4096
Sending 1000000 write requests, 4096 bytes each, 64 in parallel (starting at offset 4096, step size 8192)
Run completed in 10.155 seconds.
$ filefrag ~/tmp/test.raw
/home/kwolf/tmp/test.raw: 178 extents found
$ time ./qemu-img map ~/tmp/test.raw
Offset Length Mapped to File
0 0x1e8480000 0 /home/kwolf/tmp/test.raw
real 0m0,061s
user 0m0,040s
sys 0m0,014s
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200707142329.48303-1-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The actual disk space used by an image can vary between filesystems and
depending on other settings like an extent size hint. Replace the one
call of "$QEMU_IMG info" and the associated one-off sed filter with the
more standard "_img_info" and the standard filter from common.filter.
Apart from turning "vmdk" into "IMGFMT" and changing the placeholder for
cid fields, this only removes the "disk size" line.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Not only is it a bit stupid to try to filter multi-line "Formatting"
output (because we only need it for a single test, which can easily be
amended to no longer need it), it is also problematic when there can be
output after a "Formatting" line that we do not want to filter as if it
were part of it.
So rename _filter_img_create to _do_filter_img_create, let it filter
only a single line, and let _filter_img_create loop over all input
lines, calling _do_filter_img_create only on those that match
/^Formatting/ (basically, what _filter_img_create_in_qmp did already).
(And fix 020 to work with that.)
Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200709110205.310942-1-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200701105331.121670-5-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Make it simpler to debug when qemu-io fails due to wrong arguments or
environment.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200701105331.121670-3-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
The only user (iotest 205) of QemuIoInteractive provides -f argument,
so it's a bit inefficient to use qemu_io_args, which contains -f too.
And we are going to add one more test, which wants to specify -f by
hand. Let's use qemu_io_args_no_fmt.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200701105331.121670-2-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Otherwise the result is basically unpredictable.
(Note that the precise environment variable to control sorting order is
LC_COLLATE, but LC_ALL overrides LC_COLLATE, and we do not want the
sorting order to be messed up if LC_ALL is set in the environment.)
Reported-by: John Snow <jsnow@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200710163253.381630-3-mreitz@redhat.com>
In case when get_image_offset() returns -1, we do zero out the
corresponding chunk of qiov. So, this should be reported as ZERO.
Note that this changes visible output of "qemu-img map --output=json"
and "qemu-io -c map" commands. For qemu-img map, the change is obvious:
we just mark as zero what is really zero. For qemu-io it's less
obvious: what was unallocated now is allocated.
There is an inconsistency in understanding of unallocated regions in
Qemu: backing-supporting format-drivers return 0 block-status to report
go-to-backing logic for this area. Some protocol-drivers (iscsi) return
0 to report fs-unallocated-non-zero status (i.e., don't occupy space on
disk, read result is undefined).
BDRV_BLOCK_ALLOCATED is defined as something more close to
go-to-backing logic. Still it is calculated as ZERO | DATA, so 0 from
iscsi is treated as unallocated. It doesn't influence backing-chain
behavior, as iscsi can't have backing file. But it does influence
"qemu-io -c map".
We should solve this inconsistency at some future point. Now, let's
just make backing-not-supporting format drivers (vdi in the previous
patch and vpc now) to behave more like backing-supporting drivers
and not report 0 block-status. More over, returning ZERO status is
absolutely valid thing, and again, corresponds to how the other
format-drivers (backing-supporting) work.
After block-status update, it never reports 0, so setting
unallocated_blocks_are_zero doesn't make sense (as the only user of it
is bdrv_co_block_status and it checks unallocated_blocks_are_zero only
for unallocated areas). Drop it.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200528094405.145708-5-vsementsov@virtuozzo.com>
[mreitz: qemu-io -c map as used by iotest 146 now reports everything as
allocated; in order to make the test do something useful, we
use qemu-img map --output=json now]
Signed-off-by: Max Reitz <mreitz@redhat.com>
This commit adds two tests that cover the
new blockdev-amend functionality of luks and qcow2 driver
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
[mreitz: Let 295 verify that LUKS works; drop 295 and 296 from the auto
group]
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200625125548.870061-20-mreitz@redhat.com>
This commit adds two tests, which test the new amend interface
of both luks raw images and qcow2 luks encrypted images.
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
[mreitz: Let 293 verify that LUKS works; drop $(seq) usage from 293;
drop 293 and 294 from the auto group]
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200625125548.870061-16-mreitz@redhat.com>
Now that we have all the infrastructure in place,
wire it in the qcow2 driver and expose this to the user.
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200608094030.670121-9-mlevitsk@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Some qcow2 create options can't be used for amend.
Remove them from the qcow2 create options and add generic logic to detect
such options in qemu-img
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
[mreitz: Dropped some iotests reference output hunks that became
unnecessary thanks to
"iotests: Make _filter_img_create more active"]
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200625125548.870061-12-mreitz@redhat.com>
Whenever running an iotest for the luks format, we should check whether
luks actually really works.
Tests that try to create luks-encrypted qcow2 images should do the same.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200625125548.870061-7-mreitz@redhat.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Similar to _require_working_luks for bash tests, these functions can be
used to check whether our luks driver can actually create images.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200625125548.870061-6-mreitz@redhat.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
This function will be used by the next patch, which intends to check
both the exit code and qemu-img's output.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200625125548.870061-5-mreitz@redhat.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
[mreitz: Rebased on 49438972b8]
Signed-off-by: Max Reitz <mreitz@redhat.com>
That the luks driver is present is little indication on whether it is
actually working. Without the crypto libraries linked in, it does not
work. So add this function, which tries to create a luks image to see
whether that actually works.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200625125548.870061-4-mreitz@redhat.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
This allows more tests to be able to have same output on both qcow2 luks encrypted images
and raw luks images
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Message-Id: <20200625125548.870061-3-mreitz@redhat.com>
Right now, _filter_img_create just filters out everything that looks
format-dependent, and applies some filename filters. That means that we
have to add another filter line every time some format gets a new
creation option. This can be avoided by instead discarding everything
and just keeping what we know is format-independent (format, size,
backing file, encryption information[1], preallocation) or just
interesting to have in the reference output (external data file path).
Furthermore, we probably want to sort these options. Format drivers are
not required to define them in any specific order, so the output is
effectively random (although this has never bothered us until now). We
need a specific order for our reference outputs, though. Unfortunately,
just using a plain "sort" would change a lot of existing reference
outputs, so we have to pre-filter the option keys to keep our existing
order (fmt, size, backing*, data, encryption info, preallocation).
Finally, this makes it difficult for _filter_img_create to automagically
work for QMP output. Thus, this patch adds a separate
_filter_img_create_for_qmp function that echos every line verbatim that
does not start with "Formatting", and pipes those "Formatting" lines to
_filter_img_create.
[1] Actually, the only thing that is really important is whether
encryption is enabled or not. A patch by Maxim thus removes all
other "encrypt.*" options from the output:
https://lists.nongnu.org/archive/html/qemu-block/2020-06/msg00339.html
But that patch needs to come later so we can get away with changing
as few reference outputs in this patch here as possible.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200625125548.870061-2-mreitz@redhat.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
When resizing an image with qcow2_co_truncate() using the falloc or
full preallocation modes the code assumes that both the old and new
sizes are cluster-aligned.
There are two problems with this:
1) The calculation of how many clusters are involved does not always
get the right result.
Example: creating a 60KB image and resizing it (with
preallocation=full) to 80KB won't allocate the second cluster.
2) No copy-on-write is performed, so in the previous example if
there is a backing file then the first 60KB of the first cluster
won't be filled with data from the backing file.
This patch fixes both issues.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20200617140036.20311-1-berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Commit 96927c744 replaced qdev_init_nofail() call by
isa_realize_and_unref() which has a different error
message. Update the test output accordingly.
Gitlab CI error after merging b77b5b3dc7:
https://gitlab.com/qemu-project/qemu/-/jobs/597414772#L4375
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20200616154949.6586-1-philmd@redhat.com>
Message-Id: <20200624140446.15380-2-alex.bennee@linaro.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Waiting on a process for which we have a pipe will stall if the process
outputs more data than fits into the OS-provided buffer. We must use
communicate() before wait(), and in fact, communicate() perfectly
replaces wait() already.
We have to drop the stderr=subprocess.STDOUT parameter from
subprocess.Popen() in qemu_nbd_early_pipe(), because stderr is passed on
to the child process, so if we do not drop this parameter, communicate()
will hang (because the pipe is not closed).
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200630083711.40567-1-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
820c6bee53 added testing of qcow2.py into 291, and it breaks 291
with external data file. Actually, 291 is bad place for qcow2.py
testing, better add a separate test.
For now, drop qcow2.py testing from 291 to fix the regression.
Fixes: 820c6bee53
Reported-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200618154052.8629-1-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Commit 96927c744 replaced qdev_init_nofail() call by
isa_realize_and_unref() which has a different error
message. Update the test output accordingly.
Gitlab CI error after merging b77b5b3dc7:
https://gitlab.com/qemu-project/qemu/-/jobs/597414772#L4375
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200616154949.6586-1-philmd@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
qdev_prop_set_drive() screws up when the property already has a
non-null value: it neglects to release the old value. Both the old
and the new backend become attached to the same device.
Example (taken from iotest 172): -fda ... -drive if=none,... -global
floppy.drive=none0.
Special case: attempting to use the same backend both times fails.
Example (also from iotest 172): -fda ... -global floppy.drive=floppy0.
Yet another example: -device with multiple drive=... (but not
device_add, which silently drops all but the last duplicate property).
Perhaps drive property override could be made to work. Perhaps it
should. I can't afford the time to figure this out now. What I can
do is reject usage that leaves backends in unhealthy states. For what
it's worth, we've long done the same for netdev properties.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200622094227.1271650-12-armbru@redhat.com>
Deprecate
-global isa-fdc.driveA=...
-global isa-fdc.driveB=...
in favour of
-device floppy,unit=0,drive=...
-device floppy,unit=1,drive=...
Same for the other floppy controller devices.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: John Snow <jsnow@redhat.com>
Message-Id: <20200622094227.1271650-7-armbru@redhat.com>
The floppy controller devices desugar their drive properties into
floppy devices (since commit a92bd191a4 "fdc: Move qdev properties to
FloppyDrive", v2.8.0). This involves some bad magic in
fdctrl_connect_drives(), and exists for backward compatibility.
The functions for boards to create floppy controller devices
fdctrl_init_isa(), fdctrl_init_sysbus(), and sun4m_fdctrl_init()
desugar -drive if=floppy to these floppy controller drive properties.
If you use both -drive if=floppy (or its -fda / -fdb sugar) and
-global isa-fdc for the same floppy device, -global silently loses the
conflict, and both backends involved end up with the floppy device
frontend attached, as demonstrated by iotest 172 (see commit before
previous). This is wrong.
Desugar -drive if=floppy straight to floppy devices instead, with
helper fdctrl_init_drives(). The conflict now gets rejected cleanly:
first, fdctrl_connect_drives() creates the floppy for the controller's
property, then fdctrl_init_drives() attempts to create the floppy for
-drive if=floppy, but fails because the unit is already in use.
Output of iotest 172 changes in three ways:
1. The clash gets rejected.
2. In one test case, "info qtree" has the floppy devices swapped, and
"info block" has their QOM paths swapped. This is because the
floppy device for -fda now gets created after the one for -global
isa-fdc.driveB.
3. The error message for -global floppy.drive=floppy0 changes. Before
the patch, we set isa-fdc.driveA to -fda's block backend, then
create the floppy device for it, then move the backend from
isa-fdc.driveA to floppy.drive. Floppy creation fails when
applying -global floppy.drive=floppy0, because floppy0 is still
attached to isa-fdc. After the patch, we create the floppy for
-fda, then set its drive property to floppy0. Now floppy creation
succeeds, but setting the drive property fails, because -global
already set it. Yes, this is exasperatingly complicated.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200622094227.1271650-5-armbru@redhat.com>
Use of -global to set a default backend for non-singleton devices is a
bad idea. But as long as we permit it, we better test it.
Test output demonstrates we screw up when -global floppy clashes with
-fda or with -device floppy: according to "info qtree", only the
latter backend is attached, but according to "info block", both are.
Here's the clash with -device:
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global floppy.drive=none0 -device floppy,drive=none1,unit=0
dev: isa-fdc, id ""
[...]
driveA = ""
driveB = ""
[...]
bus: floppy-bus.0
type floppy-bus
dev: floppy, id ""
unit = 0 (0x0)
---> drive = "none1"
[...]
none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
---> Attached to: /machine/peripheral-anon/device[0]
Cache mode: writeback
none1 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
---> Attached to: /machine/peripheral-anon/device[0]
Removable device: not locked, tray closed
Cache mode: writeback
/machine/peripheral-anon/device[0] is the floppy created with -device.
Test output further demonstrates the "Drive 'FOO' is already in use
because it has been automatically connected to another device" error
message can be misleading. With '-fda "" -global
floppy.drive=floppy0', it's in use because -global reuses -fda's
backend. There is no other device involved.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200622094227.1271650-4-armbru@redhat.com>
The additional output demonstrates we screw up when -global isa-fdc
clashes with -drive if=floppy or its sugared forms: according to "info
qtree", only the latter backend is attached, but according to "info
block", both are. For instance:
Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0
dev: isa-fdc, id ""
[...]
driveA = ""
driveB = ""
[...]
bus: floppy-bus.0
type floppy-bus
dev: floppy, id ""
unit = 0 (0x0)
---> drive = "floppy0"
[...]
floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
---> Attached to: /machine/unattached/device[15]
Removable device: not locked, tray closed
Cache mode: writeback
none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
---> Attached to: /machine/unattached/device[14]
Cache mode: writeback
/machine/unattached/device[15] is floppy, and
/machine/unattached/device[14] is isa-fdc.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200622094227.1271650-2-armbru@redhat.com>
The file qcow2.py was originally contributed in 2012 by Kevin Wolf,
but was not given traditional boilerplate headers at the time. The
missing license was just rectified (commit 16306a7b39) using the
project-default GPLv2+, but as Vladimir is not at Red Hat, he did not
add a Copyright line. All earlier contributions have come from CC'd
authors, where all but Stefan used a Red Hat address at the time of
the contribution, and that copyright carries over to the split to
qcow2_format.py (d5262c7124).
CC: Kevin Wolf <kwolf@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Eduardo Habkost <ehabkost@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Philippe Mathieu-Daudé <philmd@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200609205944.3549240-1-eblake@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fixes: 5d72c68b49
Fixes: cf2d1203dc
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200617104822.27525-6-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fixes: d89ac3cf30
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200617104822.27525-5-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fixes: e4d7019e1a
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200617104822.27525-4-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
qed does not support shrinking images, so the test_small_target method
should be skipped to keep 041 passing.
Fixes: 16cea4ee1c
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200617104822.27525-3-mreitz@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Sometimes, we want to skip some test methods for certain formats. This
decorator allows that.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200617104822.27525-2-mreitz@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Add getter for size32, and use it for blocksize, too.
In its human-readable branch, it reports approximate size in
human-readable units next to the exact byte value, like the getter for
64bit size does.
Adjust the expected test output accordingly.
Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200528225516.1676602-8-rvkagan@yandex-team.ru>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Several block device properties related to blocksize configuration must
be in certain relationship WRT each other: physical block must be no
smaller than logical block; min_io_size, opt_io_size, and
discard_granularity must be a multiple of a logical block.
To ensure these requirements are met, add corresponding consistency
checks to blkconf_blocksizes, adjusting its signature to communicate
possible error to the caller. Also remove the now redundant consistency
checks from the specific devices.
Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Message-Id: <20200528225516.1676602-3-rvkagan@yandex-team.ru>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Ever since commit 36683283 (v2.8), the server code asserts that error
strings sent to the client are well-formed per the protocol by not
exceeding the maximum string length of 4096. At the time the server
first started sending error messages, the assertion could not be
triggered, because messages were completely under our control.
However, over the years, we have added latent scenarios where a client
could trigger the server to attempt an error message that would
include the client's information if it passed other checks first:
- requesting NBD_OPT_INFO/GO on an export name that is not present
(commit 0cfae925 in v2.12 echoes the name)
- requesting NBD_OPT_LIST/SET_META_CONTEXT on an export name that is
not present (commit e7b1948d in v2.12 echoes the name)
At the time, those were still safe because we flagged names larger
than 256 bytes with a different message; but that changed in commit
93676c88 (v4.2) when we raised the name limit to 4096 to match the NBD
string limit. (That commit also failed to change the magic number
4096 in nbd_negotiate_send_rep_err to the just-introduced named
constant.) So with that commit, long client names appended to server
text can now trigger the assertion, and thus be used as a denial of
service attack against a server. As a mitigating factor, if the
server requires TLS, the client cannot trigger the problematic paths
unless it first supplies TLS credentials, and such trusted clients are
less likely to try to intentionally crash the server.
We may later want to further sanitize the user-supplied strings we
place into our error messages, such as scrubbing out control
characters, but that is less important to the CVE fix, so it can be a
later patch to the new nbd_sanitize_name.
Consideration was given to changing the assertion in
nbd_negotiate_send_rep_verr to instead merely log a server error and
truncate the message, to avoid leaving a latent path that could
trigger a future CVE DoS on any new error message. However, this
merely complicates the code for something that is already (correctly)
flagging coding errors, and now that we are aware of the long message
pitfall, we are less likely to introduce such errors in the future,
which would make such error handling dead code.
Reported-by: Xueqiang Wei <xuwei@redhat.com>
CC: qemu-stable@nongnu.org
Fixes: https://bugzilla.redhat.com/1843684 CVE-2020-10761
Fixes: 93676c88d7
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200610163741.3745251-2-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
It is possible, that shutdown on target occurs earlier than migration
finish. In this case we crash in bdrv_release_dirty_bitmap_locked()
on assertion "assert(!bdrv_dirty_bitmap_busy(bitmap));" as we do have
busy bitmap, as bitmap migration is ongoing.
We'll fix bitmap migration to gracefully cancel on early shutdown soon.
Now let's fix iotest 194 to wait migration completion before shutdown.
Note that in this test dest_vm.shutdown() is called implicitly, as vms
used as context-providers, see __exit__() method of QEMUMachine class.
Actually, not waiting migration finish is a wrong thing, but the test
started to crash after commit ae00aa2398
"iotests: 194: test also migration of dirty bitmap", which added dirty
bitmaps here. So, Fixes: tag won't hurt.
Fixes: ae00aa2398
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: grammar tweak]
Message-Id: <20200604083341.26978-1-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Depending on the granularity of holes and amount of metadata consumed
by a file, the 'disk size:' number of 'qemu-img info' is not reliable.
Adjust our test to use a different set of filters to avoid spurious
failures.
Reported-by: Kevin Wolf <kwolf@redhat.com>
Fixes: cf2d1203dc
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200608195629.3299649-1-eblake@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
[eblake: fix merge conflict]
Signed-off-by: Eric Blake <eblake@redhat.com>
Add class for bitmap extension and dump its fields. Further work is to
dump bitmap directory.
Test new functionality inside 291 iotest.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <20200606081806.23897-14-vsementsov@virtuozzo.com>
[eblake: fix iotest output]
Signed-off-by: Eric Blake <eblake@redhat.com>
