summaryrefslogtreecommitdiff
path: root/tests/qemu-iotests
AgeCommit message (Collapse)Author
2020-07-06block/vpc: return ZERO block-status when appropriateVladimir Sementsov-Ogievskiy
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>
2020-07-06iotests: add tests for blockdev-amendMaxim Levitsky
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>
2020-07-06iotests: qemu-img tests for luks key managementMaxim Levitsky
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>
2020-07-06block/qcow2: extend qemu-img amend interface with crypto optionsMaxim Levitsky
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>
2020-07-06block/amend: refactor qcow2 amend optionsMaxim Levitsky
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>
2020-07-06iotests: Check whether luks worksMax Reitz
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>
2020-07-06iotests.py: Add (verify|has)_working_luks()Max Reitz
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>
2020-07-06iotests.py: Add qemu_img_pipe_and_status()Max Reitz
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 49438972b8c2e] Signed-off-by: Max Reitz <mreitz@redhat.com>
2020-07-06iotests/common.rc: Add _require_working_luksMax Reitz
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>
2020-07-06iotests: filter few more luks specific create optionsMaxim Levitsky
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>
2020-07-06iotests: Make _filter_img_create more activeMax Reitz
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>
2020-07-06qcow2: Fix preallocation on images with unaligned sizesAlberto Garcia
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>
2020-07-03Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into stagingPeter Maydell
Block layer patches: - qemu-img convert: Don't pre-zero images (removes nowadays counterproductive optimisation) - qemu-storage-daemon: Fix object-del, cleaner shutdown - vvfat: Check that the guest doesn't escape the given host directory with read-write vvfat drives - vvfat: Fix crash by out-of-bounds array writes for read-write drives - iotests fixes # gpg: Signature made Fri 03 Jul 2020 10:20:46 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: iotests: Fix 051 output after qdev_init_nofail() removal iotests.py: Do not wait() before communicate() vvfat: Fix array_remove_slice() vvfat: Check that updated filenames are valid qemu-storage-daemon: add missing cleanup calls qemu-storage-daemon: remember to add qemu_object_opts qemu-img convert: Don't pre-zero images Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-07-03Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-06-24' ↵Peter Maydell
into staging Block patches: - Two iotest fixes # gpg: Signature made Wed 24 Jun 2020 09:00:51 BST # gpg: using RSA key 91BEB60A30DB3E8857D11829F407DB0061D5CF40 # gpg: issuer "mreitz@redhat.com" # gpg: Good signature from "Max Reitz <mreitz@redhat.com>" [full] # Primary key fingerprint: 91BE B60A 30DB 3E88 57D1 1829 F407 DB00 61D5 CF40 * remotes/maxreitz/tags/pull-block-2020-06-24: iotests: don't test qcow2.py inside 291 iotests: Fix 051 output after qdev_init_nofail() removal Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-07-03iotests: Fix 051 output after qdev_init_nofail() removalPhilippe Mathieu-Daudé
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>
2020-07-03iotests.py: Do not wait() before communicate()Max Reitz
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>
2020-06-24iotests: don't test qcow2.py inside 291Vladimir Sementsov-Ogievskiy
820c6bee534ec3b 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: 820c6bee534ec3b 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>
2020-06-24iotests: Fix 051 output after qdev_init_nofail() removalPhilippe Mathieu-Daudé
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>
2020-06-23qdev: Reject drive property overrideMarkus Armbruster
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>
2020-06-23fdc: Deprecate configuring floppies with -global isa-fdcMarkus Armbruster
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>
2020-06-23fdc: Reject clash between -drive if=floppy and -global isa-fdcMarkus Armbruster
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>
2020-06-23iotests/172: Cover -global floppy.drive=...Markus Armbruster
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>
2020-06-23iotests/172: Cover empty filename and multiple use of drivesMarkus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200622094227.1271650-3-armbru@redhat.com>
2020-06-23iotests/172: Include "info block" in test outputMarkus Armbruster
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>
2020-06-17iotests: Add copyright line in qcow2.pyEric Blake
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>
2020-06-17iotests/{190,291}: compat=0.10 is unsupportedMax Reitz
Fixes: 5d72c68b49769c927e90b78af6d90f6a384b26ac Fixes: cf2d1203dcfc2bf964453d83a2302231ce77f2dc Signed-off-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200617104822.27525-6-mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-06-17iotests/229: data_file is unsupportedMax Reitz
Fixes: d89ac3cf305b28c024a76805a84d75c0ee1e786f Signed-off-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200617104822.27525-5-mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-06-17iotests/292: data_file is unsupportedMax Reitz
Fixes: e4d7019e1a81c61de6a925c3ac5bb6e62ea21b29 Signed-off-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200617104822.27525-4-mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-06-17iotests/041: Skip test_small_target for qedMax Reitz
qed does not support shrinking images, so the test_small_target method should be skipped to keep 041 passing. Fixes: 16cea4ee1c8e5a69a058e76f426b2e17974d8d7d 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>
2020-06-17iotests.py: Add skip_for_formats() decoratorMax Reitz
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>
2020-06-17qdev-properties: add getter for size32 and blocksizeRoman Kagan
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>
2020-06-17block: consolidate blocksize properties consistency checksRoman Kagan
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>
2020-06-11Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2020-06-09-v2' ↵Peter Maydell
into staging NBD patches for 2020-06-09 - fix iotest 194 race - fix CVE-2020-10761: server DoS from assertion on long NBD error messages # gpg: Signature made Wed 10 Jun 2020 18:59:19 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-nbd-2020-06-09-v2: block: Call attention to truncation of long NBD exports nbd/server: Avoid long error message assertions CVE-2020-10761 iotests: 194: wait for migration completion on target too Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-06-10nbd/server: Avoid long error message assertions CVE-2020-10761Eric Blake
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>
2020-06-09iotests: 194: wait for migration completion on target tooVladimir Sementsov-Ogievskiy
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 ae00aa239847682 "iotests: 194: test also migration of dirty bitmap", which added dirty bitmaps here. So, Fixes: tag won't hurt. Fixes: ae00aa2398476824f0eca80461da215e7cdc1c3b 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>
2020-06-09iotests: Fix 291 across more file systemsEric Blake
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>
2020-06-09qcow2_format.py: dump bitmaps header extensionVladimir Sementsov-Ogievskiy
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>
2020-06-09qcow2: QcowHeaderExtension print names for extension magicsVladimir Sementsov-Ogievskiy
Suggested-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200606081806.23897-13-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09qcow2_format: refactor QcowHeaderExtension as a subclass of Qcow2StructVladimir Sementsov-Ogievskiy
Only two fields we can parse by generic code, but that is better than nothing. Keep further refactoring of variable-length fields for another day. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Message-Id: <20200606081806.23897-12-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09qcow2_format.py: QcowHeaderExtension: add dump methodVladimir Sementsov-Ogievskiy
Obviously, for-loop body in dump_extensions should be the dump method of extension. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Message-Id: <20200606081806.23897-11-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09qcow2_format.py: add field-formatting classVladimir Sementsov-Ogievskiy
Allow formatter class in structure definition instead of hacking with 'mask'. This will simplify further introduction of new formatters. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Message-Id: <20200606081806.23897-10-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09qcow2_format.py: separate generic functionality of structure classesVladimir Sementsov-Ogievskiy
We are going to introduce more Qcow2 structure types, defined like QcowHeader. Move generic functionality into base class to be reused for further structure classes. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Message-Id: <20200606081806.23897-9-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09qcow2_format.py: use strings to specify c-type of struct fieldsVladimir Sementsov-Ogievskiy
We are going to move field-parsing to super-class, this will be simpler with simple string specifiers instead of variables. For some reason, python doesn't allow the definition of ctypes variable in the class alongside fields: it would not be available then for use by the 'for' operator. Don't worry: ctypes will be moved to metaclass soon. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Message-Id: <20200606081806.23897-8-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09qcow2_format.py: use modern string formattingVladimir Sementsov-Ogievskiy
Use .format and f-strings instead of old %style. Also, the file uses both '' and "" quotes, for consistency let's use '', except for cases when we need '' inside the string (use "" to avoid extra escaping). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Message-Id: <20200606081806.23897-7-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09qcow2_format.py: use tuples instead of lists for fieldsVladimir Sementsov-Ogievskiy
No need in lists: it's a constant variable. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Message-Id: <20200606081806.23897-6-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09qcow2_format.py: drop new line printing at end of dump()Vladimir Sementsov-Ogievskiy
This will simplify further conversion. To compensate, print this empty line directly in cmd_dump_header(). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Message-Id: <20200606081806.23897-5-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09qcow2.py: move qcow2 format classes to separate moduleVladimir Sementsov-Ogievskiy
We are going to enhance qcow2 format parsing by adding more structure classes. Let's split format parsing from utility code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200606081806.23897-4-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09qcow2.py: add licensing blurbVladimir Sementsov-Ogievskiy
Add classic heading, which is missing here. Keep copyright place empty, prior authors may add a line later. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200606081806.23897-3-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: tweak commit message] Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09qcow2.py: python style fixesVladimir Sementsov-Ogievskiy
Fix flake8 complaints. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200606081806.23897-2-vsementsov@virtuozzo.com> Tested-by: Eric Blake <eblake@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: commit message improved] Signed-off-by: Eric Blake <eblake@redhat.com>
2020-05-31Merge remote-tracking branch ↵Peter Maydell
'remotes/philmd-gitlab/tags/python-next-20200531' into staging Python queue: * migration acceptance test fix * introduce pylintrc & flake8 config * various cleanups (Python3, style) * vm-test can set QEMU_LOCAL=1 to use locally built binaries * refactored BootLinuxBase & LinuxKernelTest acceptance classes https://gitlab.com/philmd/qemu/pipelines/151323210 https://travis-ci.org/github/philmd/qemu/builds/693157969 # gpg: Signature made Sun 31 May 2020 17:37:35 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-20200531: (25 commits) tests/acceptance: refactor boot_linux to allow code reuse tests/acceptance: refactor boot_linux_console test to allow code reuse tests/acceptance: allow console interaction with specific VMs tests/acceptance/migration.py: Wait for both sides tests/migration/guestperf: Use Python 3 interpreter tests/vm: allow wait_ssh() to specify command tests/vm: Add ability to select QEMU from current build tests/vm: Pass --debug through for vm-boot-ssh python/qemu/qtest: Check before accessing _qtest python/qemu/qmp: assert sockfile is not None python/qemu/qmp: use True/False for non/blocking modes python/qemu: Adjust traceback typing python/qemu: fix socket.makefile() typing python/qemu: remove Python2 style super() calls python/qemu: delint; add flake8 config python/qemu: delint and add pylintrc python/qemu/machine: remove logging configuration python/qemu/machine: add kill() method python: remove more instances of sys.version_info scripts/qmp: Fix shebang and imports ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-15 10:20:05 +0000