aboutsummaryrefslogtreecommitdiff
path: root/block
AgeCommit message (Collapse)AuthorFilesLines
2017-12-21block/iscsi: only report an iSCSI Failure if we don't handle it gracefullyPeter Lieven1-11/+32
we currently report an "iSCSI Failure" in iscsi_co_generic_cb if the task hasn't completed with SCSI_STATUS_GOOD. However, we expect a failure in some cases and handle it gracefully. This is the case for misaligned UNMAPs and WRITESAME10/16 calls without UNMAP. In this case a failure in the logs can be quite misleading. While we are at it improve the logging to reveal which operation failed at what LBA. Signed-off-by: Peter Lieven <pl@kamp.de> Message-Id: <1512733868-9009-3-git-send-email-pl@kamp.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-12-21block/iscsi: dont leave allocmap in an invalid state on UNMAP failurePeter Lieven1-4/+4
we forgot to set the allocmap to invalid if an UNMAP call fails. Cc: qemu-stable@nongnu.org Signed-off-by: Peter Lieven <pl@kamp.de> Message-Id: <1512733868-9009-2-git-send-email-pl@kamp.de> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-12-20Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into ↵Peter Maydell2-4/+2
staging Pull request v2: * Fixed incorrect virtio_blk_data_plane_create() local_err refactoring in "hw/block: Use errp directly rather than local_err" that broke virtio-blk over virtio-mmio [Peter] # gpg: Signature made Tue 19 Dec 2017 15:08:14 GMT # gpg: using RSA key 0x9CA4ABB381AB73C8 # gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>" # gpg: aka "Stefan Hajnoczi <stefanha@gmail.com>" # Primary key fingerprint: 8695 A8BF D3F9 7CDA AC35 775A 9CA4 ABB3 81AB 73C8 * remotes/stefanha/tags/block-pull-request: (23 commits) qemu-iotests: add 203 savevm with IOThreads test iothread: fix iothread_stop() race condition iotests: add VM.add_object() blockdev: add x-blockdev-set-iothread force boolean docs: mark nested AioContext locking as a legacy API block: avoid recursive AioContext acquire in bdrv_inactivate_all() virtio-blk: reject configs with logical block size > physical block size virtio-blk: make queue size configurable qemu-iotests: add 202 external snapshots IOThread test blockdev: add x-blockdev-set-iothread testing command iothread: add iothread_by_id() API block: drop unused BlockDirtyBitmapState->aio_context field block: don't keep AioContext acquired after internal_snapshot_prepare() block: don't keep AioContext acquired after blockdev_backup_prepare() block: don't keep AioContext acquired after drive_backup_prepare() block: don't keep AioContext acquired after external_snapshot_prepare() blockdev: hold AioContext for bdrv_unref() in external_snapshot_clean() qdev: drop unused #include "sysemu/iothread.h" dev-storage: Fix the unusual function name hw/block: Use errp directly rather than local_err ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org> # Conflicts: # hw/core/qdev-properties-system.c
2017-12-19Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into stagingPeter Maydell4-141/+172
# gpg: Signature made Mon 18 Dec 2017 21:05:53 GMT # gpg: using RSA key 0xBDBE7B27C0DE3057 # gpg: Good signature from "Jeffrey Cody <jcody@redhat.com>" # gpg: aka "Jeffrey Cody <jeff@codyprime.org>" # gpg: aka "Jeffrey Cody <codyprime@gmail.com>" # Primary key fingerprint: 9957 4B4D 3474 90E7 9D98 D624 BDBE 7B27 C0DE 3057 * remotes/cody/tags/block-pull-request: block/curl: fix minor memory leaks block/curl: check error return of curl_global_init() block/sheepdog: code beautification block/sheepdog: remove spurious NULL check blockjob: kick jobs on set-speed backup: use copy_bitmap in incremental backup backup: simplify non-dirty bits progress processing backup: init copy_bitmap from sync_bitmap for incremental backup: move from done_bitmap to copy_bitmap hbitmap: add next_zero function Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-12-19coroutine: simplify co_aio_sleep_ns() prototypeStefan Hajnoczi2-4/+2
The AioContext pointer argument to co_aio_sleep_ns() is only used for the sleep timer. It does not affect where the caller coroutine is resumed. Due to changes to coroutine and AIO APIs it is now possible to drop the AioContext pointer argument. This is safe to do since no caller has specific requirements for which AioContext the timer must run in. This patch drops the AioContext pointer argument and renames the function to simplify the API. Reported-by: Paolo Bonzini <pbonzini@redhat.com> Reported-by: Eric Blake <eblake@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 20171109102652.6360-1-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2017-12-18block/curl: fix minor memory leaksJeff Cody1-0/+6
Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-12-18block/curl: check error return of curl_global_init()Jeff Cody1-6/+12
If curl_global_init() fails, per the documentation no other curl functions may be called, so make sure to check the return value. Also, some minor changes to the initialization latch variable 'inited': - Make it static in the file, for clarity - Change the name for clarity - Make it a bool Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-12-18block/sheepdog: code beautificationJeff Cody1-82/+82
No functional changes, just whitespace manipulation. Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-12-18block/sheepdog: remove spurious NULL checkJeff Cody1-1/+1
'tag' is already checked in the lines immediately preceding this check, and set to non-NULL if NULL. No need to check again, it hasn't changed. Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-12-18backup: use copy_bitmap in incremental backupVladimir Sementsov-Ogievskiy1-38/+17
We can use copy_bitmap instead of sync_bitmap. copy_bitmap is initialized from sync_bitmap and it is more informative: we will not try to process data, that is already in progress (by write notifier). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 20171012135313.227864-6-vsementsov@virtuozzo.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-12-18backup: simplify non-dirty bits progress processingVladimir Sementsov-Ogievskiy1-15/+3
Set fake progress for non-dirty clusters in copy_bitmap initialization, to. It simplifies code and allows further refactoring. This patch changes user's view of backup progress, but formally it doesn't changed: progress hops are just moved to the beginning. Actually it's just a point of view: when do we actually skip clusters? We can say in the very beginning, that we skip these clusters and do not think about them later. Of course, if go through disk sequentially, it's logical to say, that we skip clusters between copied portions to the left and to the right of them. But even now copying progress is not sequential because of write notifiers. Future patches will introduce new backup architecture which will do copying in several coroutines in parallel, so it will make no sense to publish fake progress by parts in parallel with other copying requests. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 20171012135313.227864-5-vsementsov@virtuozzo.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-12-18backup: init copy_bitmap from sync_bitmap for incrementalVladimir Sementsov-Ogievskiy1-1/+43
We should not copy non-dirty clusters in write notifiers. So, initialize copy_bitmap from sync_bitmap. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20171012135313.227864-4-vsementsov@virtuozzo.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-12-18backup: move from done_bitmap to copy_bitmapVladimir Sementsov-Ogievskiy1-9/+14
Use HBitmap copy_bitmap instead of done_bitmap. This is needed to improve incremental backup in following patches and to unify backup loop for full/incremental modes in future patches. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 20171012135313.227864-3-vsementsov@virtuozzo.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-12-18hbitmap: add next_zero functionVladimir Sementsov-Ogievskiy1-0/+5
The function searches for next zero bit. Also add interface for BdrvDirtyBitmap and unit test. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 20171012135313.227864-2-vsementsov@virtuozzo.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-12-18block: remove "qemu/osdep.h" from header filePhilippe Mathieu-Daudé1-1/+0
applied using ./scripts/clean-includes Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
2017-11-29block/nfs: fix nfs_client_open for filesize greater than 1TBPeter Lieven1-4/+3
DIV_ROUND_UP(st.st_size, BDRV_SECTOR_SIZE) was overflowing ret (int) if st.st_size is greater than 1TB. Cc: qemu-stable@nongnu.org Signed-off-by: Peter Lieven <pl@kamp.de> Message-id: 1511798407-31129-1-git-send-email-pl@kamp.de Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-29blockjob: remove clock argument from block_job_sleep_nsPaolo Bonzini4-7/+7
All callers are using QEMU_CLOCK_REALTIME, and it will not be possible to support more than one clock when block_job_sleep_ns switches to a single timer stored in the BlockJob struct. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Tested-By: Jeff Cody <jcody@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-11-29block: Expect graph changes in bdrv_parent_drained_begin/endKevin Wolf1-4/+4
The .drained_begin/end callbacks can (directly or indirectly via aio_poll()) cause block nodes to be removed or the current BdrvChild to point to a different child node. Use QLIST_FOREACH_SAFE() to make sure we don't access invalid BlockDriverStates or accidentally continue iterating the parents of the new child node instead of the node we actually came from. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Tested-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-11-21block: Error out on load_vm with active dirty bitmapsKevin Wolf1-12/+3
Loading a snapshot invalidates the bitmap. Just marking all blocks dirty is not a useful response in practice, instead the user needs to be aware that we switch to a completely different state. If they are okay with losing the dirty bitmap, they can just explicitly delete it. This effectively reverts commit 04dec3c3ae5. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Denis V. Lunev <den@openvz.org> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com>
2017-11-21block: Add errp to bdrv_all_goto_snapshot()Kevin Wolf1-5/+6
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Denis V. Lunev <den@openvz.org> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com>
2017-11-21block: Add errp to bdrv_snapshot_goto()Kevin Wolf1-6/+17
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com>
2017-11-21block: Don't request I/O permission with BDRV_O_NO_IOKevin Wolf1-4/+6
'qemu-img info' makes sense even when BLK_PERM_CONSISTENT_READ cannot be granted because of a block job in a running qemu process. It already sets BDRV_O_NO_IO to indicate that it doesn't access the guest visible data at all. Check the BDRV_O_NO_IO flags in blk_new_open(), so that I/O related permissions are not unnecessarily requested and 'qemu-img info' can work even if BLK_PERM_CONSISTENT_READ cannot be granted. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com>
2017-11-17Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into stagingPeter Maydell15-39/+244
Block layer patches for 2.11.0-rc2 # gpg: Signature made Fri 17 Nov 2017 17:58:36 GMT # gpg: using RSA key 0x7F09B272C88F2FD6 # gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" # Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6 * remotes/kevin/tags/for-upstream: (25 commits) iotests: Make 087 pass without AIO enabled block: Make bdrv_next() keep strong references qcow2: Fix overly broad madvise() qcow2: Refuse to get unaligned offsets from cache qcow2: Add bounds check to get_refblock_offset() block: Guard against NULL bs->drv qcow2: Unaligned zero cluster in handle_alloc() qcow2: check_errors are fatal qcow2: reject unaligned offsets in write compressed iotests: Add test for failing qemu-img commit tests: Add check-qobject for equality tests iotests: Add test for non-string option reopening block: qobject_is_equal() in bdrv_reopen_prepare() qapi: Add qobject_is_equal() qapi/qlist: Add qlist_append_null() macro qapi/qnull: Add own header qcow2: fix image corruption on commit with persistent bitmap iotests: test clearing unknown autoclear_features by qcow2 block: Fix permissions in image activation qcow2: fix image corruption after committing qcow2 image into base ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-11-17block: Make bdrv_next() keep strong referencesMax Reitz2-2/+52
On one hand, it is a good idea for bdrv_next() to return a strong reference because ideally nearly every pointer should be refcounted. This fixes intermittent failure of iotest 194. On the other, it is absolutely necessary for bdrv_next() itself to keep a strong reference to both the BB (in its first phase) and the BDS (at least in the second phase) because when called the next time, it will dereference those objects to get a link to the next one. Therefore, it needs these objects to stay around until then. Just storing the pointer to the next in the iterator is not really viable because that pointer might become invalid as well. Both arguments taken together means we should probably just invoke bdrv_ref() and blk_ref() in bdrv_next(). This means we have to assert that bdrv_next() is always called from the main loop, but that was probably necessary already before this patch and judging from the callers, it also looks to actually be the case. Keeping these strong references means however that callers need to give them up if they decide to abort the iteration early. They can do so through the new bdrv_next_cleanup() function. Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20171110172545.32609-1-mreitz@redhat.com Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-17qcow2: Fix overly broad madvise()Max Reitz1-1/+1
@mem_size and @offset are both size_t, thus subtracting them from one another will just return a big size_t if mem_size < offset -- even more obvious here because the result is stored in another size_t. Checking that result to be positive is therefore not sufficient to exclude the case that offset > mem_size. Thus, we currently sometimes issue an madvise() over a very large address range. This is triggered by iotest 163, but with -m64, this does not result in tangible problems. But with -m32, this test produces three segfaults, all of which are fixed by this patch. Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20171114184127.24238-1-mreitz@redhat.com Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-17qcow2: Refuse to get unaligned offsets from cacheMax Reitz1-0/+21
Instead of using an assertion, it is better to emit a corruption event here. Checking all offsets for correct alignment can be tedious and it is easily possible to forget to do so. qcow2_cache_do_get() is a function every L2 and refblock access has to go through, so this is a good central point to add such a check. And for good measure, let us also add an assertion that the offset is non-zero. Making this a corruption event is not feasible, because a zero offset usually means something special (such as the cluster is unused), so all callers should be checking this anyway. If they do not, it is their fault, hence the assertion here. Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20171110203111.7666-6-mreitz@redhat.com Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-17qcow2: Add bounds check to get_refblock_offset()Max Reitz2-7/+25
Reported-by: R. Nageswara Sastry <nasastry@in.ibm.com> Buglink: https://bugs.launchpad.net/qemu/+bug/1728661 Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20171110203111.7666-5-mreitz@redhat.com Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-17block: Guard against NULL bs->drvMax Reitz4-2/+59
We currently do not guard everywhere against a NULL bs->drv where we should be doing so. Most of the places fixed here just do not care about that case at all. Some care implicitly, e.g. through a prior function call to bdrv_getlength() which would always fail for an ejected BDS. Add an assert there to make it more obvious. Other places seem to care, but do so insufficiently: Freeing clusters in a qcow2 image is an error-free operation, but it may leave the image in an unusable state anyway. Giving qcow2_free_clusters() an error code is not really viable, it is much easier to note that bs->drv may be NULL even after a successful driver call. This concerns bdrv_co_flush(), and the way the check is added to bdrv_co_pdiscard() (in every iteration instead of only once). Finally, some places employ at least an assert(bs->drv); somewhere, that may be reasonable (such as in the reopen code), but in bdrv_has_zero_init(), it is definitely not. Returning 0 there in case of an ejected BDS saves us much headache instead. Reported-by: R. Nageswara Sastry <nasastry@in.ibm.com> Buglink: https://bugs.launchpad.net/qemu/+bug/1728660 Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20171110203111.7666-4-mreitz@redhat.com Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-17qcow2: Unaligned zero cluster in handle_alloc()Max Reitz1-1/+12
We should check whether the cluster offset we are about to use is actually valid; that is, whether it is aligned to cluster boundaries. Reported-by: R. Nageswara Sastry <nasastry@in.ibm.com> Buglink: https://bugs.launchpad.net/qemu/+bug/1728643 Buglink: https://bugs.launchpad.net/qemu/+bug/1728657 Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20171110203111.7666-3-mreitz@redhat.com Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-17qcow2: check_errors are fatalMax Reitz1-1/+4
When trying to repair a dirty image, qcow2_check() may apparently succeed (no really fatal error occurred that would prevent the check from continuing), but if check_errors in the result object is non-zero, we cannot trust the image to be usable. Reported-by: R. Nageswara Sastry <nasastry@in.ibm.com> Buglink: https://bugs.launchpad.net/qemu/+bug/1728639 Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20171110203111.7666-2-mreitz@redhat.com Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-17qcow2: reject unaligned offsets in write compressedAnton Nefedov1-0/+4
Misaligned compressed write is not supported. Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com> Message-id: 1510654613-47868-2-git-send-email-anton.nefedov@virtuozzo.com Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-17qcow2: fix image corruption on commit with persistent bitmapEric Blake1-7/+10
If an image contains persistent bitmaps, we cannot use the fast path of bdrv_make_empty() to clear the image during qemu-img commit, because that will lose the clusters related to the bitmaps. Also leave a comment in qcow2_read_extensions to remind future feature additions to think about fast-path removal, since we just barely fixed the same bug for LUKS encryption. It's a pain that qemu-img has not yet been taught to manipulate, or even at a very minimum display, information about persistent bitmaps; instead, we have to use QMP commands. It's also a pain that only qeury-block and x-debug-block-dirty-bitmap-sha256 will allow bitmap introspection; but the former requires the node to be hooked to a block device, and the latter is experimental. Signed-off-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-11-17nbd: Don't crash when server reports NBD_CMD_READ failureEric Blake1-2/+2
If a server fails a read, for example with EIO, but the connection is still live, then we would crash trying to print a non-existent error message in nbd_client_co_preadv(). For consistency, also change the error printout in nbd_read_reply_entry(), although that instance does not crash. Bug introduced in commit f140e300. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171112013936.5942-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2017-11-17qcow2: fix image corruption after committing qcow2 image into baseDaniel P. Berrange1-2/+4
After committing the qcow2 image contents into the base image, qemu-img will call bdrv_make_empty to drop the payload in the layered image. When this is done for qcow2 images, it blows away the LUKS encryption header, making the resulting image unusable. There are two codepaths for emptying a qcow2 image, and the second (slower) codepath leaves the LUKS header intact, so force use of that codepath. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-11-17block: Deprecate bdrv_set_read_only() and usersKevin Wolf5-14/+44
bdrv_set_read_only() is used by some block drivers to override the read-only option given by the user. This is not how read-only images generally work in QEMU: Instead of second guessing what the user really meant (which currently includes making an image read-only even if the user didn't only use the default, but explicitly said read-only=off), we should error out if we can't provide what the user requested. This adds deprecation warnings to all callers of bdrv_set_read_only() so that the behaviour can be corrected after the usual deprecation period. Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-11-17qcow2: don't permit changing encryption parametersDaniel P. Berrange1-0/+3
Currently if trying to change encryption parameters on a qcow2 image, qemu-img will abort. We already explicitly check for attempt to change encrypt.format but missed other parameters like encrypt.key-secret. Rather than list each parameter, just blacklist changing of all parameters with a 'encrypt.' prefix. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-11-17replication: Fix replication open failWang Guang1-4/+7
replication_child_perm request write permissions for all child which will lead bdrv_check_perm fail. replication_child_perm() should request write permissions only if it is writable itself. Signed-off-by: Wang Guang <wang.guang55@zte.com.cn> Signed-off-by: Wang Yong <wang.yong155@zte.com.cn> Reviewed-by: Xie Changlong <xiechanglong@cmss.chinamobile.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-11-16throttle-groups: forget timer and schedule next TGM on detachStefan Hajnoczi1-0/+12
tg->any_timer_armed[] must be cleared when detaching pending timers from the AioContext. Failure to do so leads to hung I/O because it looks like there are still timers pending when in fact they have been removed. Other ThrottleGroupMembers might have requests pending too so it's necessary to schedule the next TGM so it can set a timer. This patch fixes hung I/O when QEMU is launched with drives that are in the same throttling group: (guest)$ dd if=/dev/zero of=/dev/vdb oflag=direct bs=512 & (guest)$ dd if=/dev/zero of=/dev/vdc oflag=direct bs=512 & (qemu) stop (qemu) cont ...I/O is stuck... Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20171116112150.27607-1-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2017-11-14block/parallels: add migration blockerJeff Cody1-0/+15
Migration does not work for parallels, and has been broken for a while (see patch 'block/parallels: Do not update header or truncate image when INMIGRATE'). The bdrv_invalidate_cache() method needs to be added for migration to be supported. Until this is done, prohibit migration. Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Message-id: 5e04a7c8a3089913fa58d484af42dab7993984ad.1510059970.git.jcody@redhat.com Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-14block/parallels: Do not update header or truncate image when INMIGRATEJeff Cody1-5/+2
If we write or modify the image file while the QEMU run state is INMIGRATE, then the BDRV_O_INACTIVE BDS flag is set. This will cause an assert, since the image is marked inactive. Make sure we obey this flag. Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru> Signed-off-by: Jeff Cody <jcody@redhat.com> Message-id: 3996c930fa8cde8570b7a63032720d76a28fd78b.1510059970.git.jcody@redhat.com Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-14block/vhdx.c: Don't blindly update the headerJeff Cody1-7/+0
The VHDX specification requires that before user data modification of the vhdx image, the VHDX header file and data GUIDs need to be updated. In vhdx_open(), if the image is set to RDWR, we go ahead and update the header. However, just because the image is set to RDWR does not mean we can go ahead and write at this point - specifically, if the QEMU run state is INMIGRATE, the underlying file BS may be set to inactive via the BDS open flag of BDRV_O_INACTIVE. Attempting to write under this condition will cause an assert in bdrv_co_pwritev(). We can alternatively latch the first time the image is written. And lo and behold, we do just that, via vhdx_user_visible_write() in vhdx_co_writev(). This means the call to vhdx_update_headers() in vhdx_open() is likely just vestigial, and can be removed. Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru> Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru> Signed-off-by: Jeff Cody <jcody@redhat.com> Message-id: 659e4cdba6ef4c651737852777c8c93d27b38040.1510059970.git.jcody@redhat.com Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-14block/snapshot: dirty all dirty bitmaps on snapshot-switchVladimir Sementsov-Ogievskiy1-0/+14
Snapshot-switch actually changes active state of disk so it should reflect on dirty bitmaps. Otherwise next incremental backup using these bitmaps will be invalid. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-id: 20171023092945.54532-1-vsementsov@virtuozzo.com Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-14qcow2: Assert that the crypto header does not overlap other metadataAlberto Garcia1-0/+1
The crypto header is initialized only when QEMU is creating a new image, so there's no chance of this happening on a corrupted image. If QEMU is really trying to allocate the header overlapping other existing metadata sections then this is a serious bug in QEMU itself so let's add an assertion. Signed-off-by: Alberto Garcia <berto@igalia.com> Message-id: ae3d77f312fc0c5e0ac2bbd71676c0112eebe2e5.1509718618.git.berto@igalia.com Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-14qcow2: Don't open images with header.refcount_table_clusters == 0Alberto Garcia1-0/+6
qcow2_do_open() is checking that header.refcount_table_clusters is not too large, but it doesn't check that it's greater than zero. Apart from the fact that an image like that is obviously corrupted, trying to use it crashes QEMU since we end up with a null s->refcount_table after qcow2_refcount_init(). These images can however be repaired, so allow opening them if the BDRV_O_CHECK flag is set. Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: f9750f50c80359babba11062e88f5075a47e8e16.1509718618.git.berto@igalia.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-14qcow2: Prevent allocating compressed clusters at offset 0Alberto Garcia1-0/+7
If the refcount data is corrupted then we can end up trying to allocate a new compressed cluster at offset 0 in the image, triggering an assertion in qcow2_alloc_bytes() that would crash QEMU: qcow2_alloc_bytes: Assertion `offset' failed. This patch adds an explicit check for this scenario and a new test case. Signed-off-by: Alberto Garcia <berto@igalia.com> Message-id: fb53467cf48e95ff3330def1cf1003a5b862b7d9.1509718618.git.berto@igalia.com Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-14qcow2: Prevent allocating L2 tables at offset 0Alberto Garcia1-0/+8
If the refcount data is corrupted then we can end up trying to allocate a new L2 table at offset 0 in the image, triggering an assertion in the qcow2 cache that would crash QEMU: qcow2_cache_entry_mark_dirty: Assertion `c->entries[i].offset != 0' failed This patch adds an explicit check for this scenario and a new test case. Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 92dac37191ae7844a2da22c122204eb493cc3133.1509718618.git.berto@igalia.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-14qcow2: Prevent allocating refcount blocks at offset 0Alberto Garcia1-0/+7
Each entry in the qcow2 cache contains an offset field indicating the location of the data in the qcow2 image. If the offset is 0 then it means that the entry contains no data and is available to be used when needed. Because of that it is not possible to store in the cache the first cluster of the qcow2 image (offset = 0). This is not a problem because that cluster always contains the qcow2 header and we're not using this cache for that. However, if the qcow2 image is corrupted it can happen that we try to allocate a new refcount block at offset 0, triggering this assertion and crashing QEMU: qcow2_cache_entry_mark_dirty: Assertion `c->entries[i].offset != 0' failed This patch adds an explicit check for this scenario and a new test case. This problem was originally reported here: https://bugs.launchpad.net/qemu/+bug/1728615 Reported-by: R.Nageswara Sastry <nasastry@in.ibm.com> Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 92a2fadd10d58b423f269c1d1a309af161cdc73f.1509718618.git.berto@igalia.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-11-14Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into ↵Peter Maydell2-12/+30
staging Pull request The following disk I/O throttling fixes solve recent bugs. # gpg: Signature made Tue 14 Nov 2017 10:37:12 GMT # gpg: using RSA key 0x9CA4ABB381AB73C8 # gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>" # gpg: aka "Stefan Hajnoczi <stefanha@gmail.com>" # Primary key fingerprint: 8695 A8BF D3F9 7CDA AC35 775A 9CA4 ABB3 81AB 73C8 * remotes/stefanha/tags/block-pull-request: qemu-iotests: Test I/O limits with removable media block: Leave valid throttle timers when removing a BDS from a backend block: Check for inserted BlockDriverState in blk_io_limits_disable() throttle-groups: drain before detaching ThrottleState block: all I/O should be completed before removing throttle timers. Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-11-13block: Leave valid throttle timers when removing a BDS from a backendAlberto Garcia1-8/+8
If a BlockBackend has I/O limits set then its ThrottleGroupMember structure uses the AioContext from its attached BlockDriverState. Those two contexts must be kept in sync manually. This is not ideal and will be fixed in the future by removing the throttling configuration from the BlockBackend and storing it in an implicit filter node instead, but for now we have to live with this. When you remove the BlockDriverState from the backend then the throttle timers are destroyed. If a new BlockDriverState is later inserted then they are created again using the new AioContext. There are a couple of problems with this: a) The code manipulates the timers directly, leaving the ThrottleGroupMember.aio_context field in an inconsisent state. b) If you remove the I/O limits (e.g by destroying the backend) when the timers are gone then throttle_group_unregister_tgm() will attempt to destroy them again, crashing QEMU. While b) could be fixed easily by allowing the timers to be freed twice, this would result in a situation in which we can no longer guarantee that a valid ThrottleState has a valid AioContext and timers. This patch ensures that the timers and AioContext are always valid when I/O limits are set, regardless of whether the BlockBackend has a BlockDriverState inserted or not. [Fixed "There'a" typo as suggested by Max Reitz <mreitz@redhat.com> --Stefan] Reported-by: sochin jiang <sochin.jiang@huawei.com> Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: e089c66e7c20289b046d782cea4373b765c5bc1d.1510339534.git.berto@igalia.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2017-11-13block: Check for inserted BlockDriverState in blk_io_limits_disable()Alberto Garcia1-4/+10
When you set I/O limits using block_set_io_throttle or the command line throttling.* options they are kept in the BlockBackend regardless of whether a BlockDriverState is attached to the backend or not. Therefore when removing the limits using blk_io_limits_disable() we need to check if there's a BDS before attempting to drain it, else it will crash QEMU. This can be reproduced very easily using HMP: (qemu) drive_add 0 if=none,throttling.iops-total=5000 (qemu) drive_del none0 Reported-by: sochin jiang <sochin.jiang@huawei.com> Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 0d3a67ce8d948bb33e08672564714dcfb76a3d8c.1510339534.git.berto@igalia.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
generated by cgit v1.1 at 2024-11-03 16:24:43 +0000