aboutsummaryrefslogtreecommitdiff
path: root/block
AgeCommit message (Collapse)Author
2018-11-19file-posix: Fix shared locks on reopen commitMax Reitz
s->locked_shared_perm is the set of bits locked in the file, which is the inverse of the permissions actually shared. So we need to pass them as they are to raw_apply_lock_bytes() instead of inverting them again. Reported-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-19qcow2: Don't allow overflow during cluster allocationEric Blake
Our code was already checking that we did not attempt to allocate more clusters than what would fit in an INT64 (the physical maximimum if we can access a full off_t's worth of data). But this does not catch smaller limits enforced by various spots in the qcow2 image description: L1 and normal clusters of L2 are documented as having bits 63-56 reserved for other purposes, capping our maximum offset at 64PB (bit 55 is the maximum bit set). And for compressed images with 2M clusters, the cap drops the maximum offset to bit 48, or a maximum offset of 512TB. If we overflow that offset, we would write compressed data into one place, but try to decompress from another, which won't work. It's actually possible to prove that overflow can cause image corruption without this patch; I'll add the iotests separately in the next commit. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-19vvfat: Fix memory leakKevin Wolf
Don't leak 'cluster' in the mapping == NULL case. Found by Coverity (CID 1055918). Fixes: 8d9401c2791ee2d2805b741b1ee3006041edcd3e Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Liam Merwick <liam.merwick@oracle.com> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2018-11-12qcow2: Read outside array bounds in qcow2_pre_write_overlap_check()Liam Merwick
The commit for 0e4e4318eaa5 increments QCOW2_OL_MAX_BITNR but does not add an array entry for QCOW2_OL_BITMAP_DIRECTORY_BITNR to metadata_ol_names[]. As a result, an array dereference of metadata_ol_names[8] in qcow2_pre_write_overlap_check() could result in a read outside of the array bounds. Fixes: 0e4e4318eaa5 ('qcow2: add overlap check for bitmap directory') Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 1541453919-25973-6-git-send-email-Liam.Merwick@oracle.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2018-11-12block: Fix potential Null pointer dereferences in vvfat.cLiam Merwick
The calls to find_mapping_for_cluster() may return NULL but it isn't always checked for before dereferencing the value returned. Additionally, add some asserts to cover cases where NULL can't be returned but which might not be obvious at first glance. Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com> Message-id: 1541453919-25973-5-git-send-email-Liam.Merwick@oracle.com [mreitz: Dropped superfluous check of "mapping" following an assertion that it is not NULL, and fixed some indentation] Signed-off-by: Max Reitz <mreitz@redhat.com>
2018-11-12block: Null pointer dereference in blk_root_get_parent_desc()Liam Merwick
The dev_id returned by the call to blk_get_attached_dev_id() in blk_root_get_parent_desc() can be NULL (an internal call to object_get_canonical_path may have returned NULL). Instead of just checking this case before before dereferencing, adjust blk_get_attached_dev_id() to return the empty string if no object path can be found (similar to the case when blk->dev is NULL and an empty string is returned). Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com> Message-id: 1541453919-25973-3-git-send-email-Liam.Merwick@oracle.com Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2018-11-12block: Make more block drivers compile-time configurableJeff Cody
This adds configure options to control the following block drivers: * Bochs * Cloop * Dmg * Qcow (V1) * Vdi * Vvfat * qed * parallels * sheepdog Each of these defaults to being enabled. Signed-off-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-id: 20181107063644.2254-1-armbru@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2018-11-12file-posix: Drop s->lock_fdFam Zheng
The lock_fd field is not strictly necessary because transferring locked bytes from old fd to the new one shouldn't fail anyway. This spares the user one fd per image. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-12file-posix: Skip effectiveless OFD lock operationsFam Zheng
If we know we've already locked the bytes, don't do it again; similarly don't unlock a byte if we haven't locked it. This doesn't change the behavior, but fixes a corner case explained below. Libvirt had an error handling bug that an image can get its (ownership, file mode, SELinux) permissions changed (RHBZ 1584982) by mistake behind QEMU. Specifically, an image in use by Libvirt VM has: $ ls -lhZ b.img -rw-r--r--. qemu qemu system_u:object_r:svirt_image_t:s0:c600,c690 b.img Trying to attach it a second time won't work because of image locking. And after the error, it becomes: $ ls -lhZ b.img -rw-r--r--. root root system_u:object_r:virt_image_t:s0 b.img Then, we won't be able to do OFD lock operations with the existing fd. In other words, the code such as in blk_detach_dev: blk_set_perm(blk, 0, BLK_PERM_ALL, &error_abort); can abort() QEMU, out of environmental changes. This patch is an easy fix to this and the change is regardlessly reasonable, so do it. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-12file-posix: Use error API properlyFam Zheng
Use error_report for situations that affect user operation (i.e. we're actually returning error), and warn_report/warn_report_err when some less critical error happened but the user operation can still carry on. For raw_normalize_devicepath, add Error parameter to propagate to its callers. Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-05vdi: Use a literal number of bytes for DEFAULT_CLUSTER_SIZELeonid Bloch
If an expression is used to define DEFAULT_CLUSTER_SIZE, when compiled, it will be embedded as a literal expression in the binary (as the default value) because it is stringified to mark the size of the default value. Now this is fixed by using a defined number to define this value. Signed-off-by: Leonid Bloch <lbloch@janustech.com> Reviewed-by: Stefan Weil <sw@weilnetz.de> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-05iscsi: Support auto-read-only optionKevin Wolf
If read-only=off, but auto-read-only=on is given, open the volume read-write if we have the permissions, but instead of erroring out for read-only volumes, just degrade to read-only. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-11-05gluster: Support auto-read-only optionKevin Wolf
If read-only=off, but auto-read-only=on is given, open the file read-write if we have the permissions, but instead of erroring out for read-only files, just degrade to read-only. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Niels de Vos <ndevos@redhat.com>
2018-11-05curl: Support auto-read-only optionKevin Wolf
If read-only=off, but auto-read-only=on is given, just degrade to read-only. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-11-05file-posix: Support auto-read-only optionKevin Wolf
If read-only=off, but auto-read-only=on is given, open the file read-write if we have the permissions, but instead of erroring out for read-only files, just degrade to read-only. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-11-05nbd: Support auto-read-only optionKevin Wolf
If read-only=off, but auto-read-only=on is given, open a read-write NBD connection if the server provides a read-write export, but instead of erroring out for read-only exports, just degrade to read-only. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-11-05block: Require auto-read-only for existing fallbacksKevin Wolf
Some block drivers have traditionally changed their node to read-only mode without asking the user. This behaviour has been marked deprecated since 2.11, expecting users to provide an explicit read-only=on option. Now that we have auto-read-only=on, enable these drivers to make use of the option. This is the only use of bdrv_set_read_only(), so we can make it a bit more specific and turn it into a bdrv_apply_auto_read_only() that is more convenient for drivers to use. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-11-05rbd: Close image in qemu_rbd_open() error pathKevin Wolf
Commit e2b8247a322 introduced an error path in qemu_rbd_open() after calling rbd_open(), but neglected to close the image again in this error path. The error path should contain everything that the regular close function qemu_rbd_close() contains. This adds the missing rbd_close() call. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-11-05block: Add auto-read-only optionKevin Wolf
If a management application builds the block graph node by node, the protocol layer doesn't inherit its read-only option from the format layer any more, so it must be set explicitly. Backing files should work on read-only storage, but at the same time, a block job like commit should be able to reopen them read-write if they are on read-write storage. However, without option inheritance, reopen only changes the read-only option for the root node (typically the format layer), but not the protocol layer, so reopening fails (the format layer wants to get write permissions, but the protocol layer is still read-only). A simple workaround for the problem in the management tool would be to open the protocol layer always read-write and to make only the format layer read-only for backing files. However, sometimes the file is actually stored on read-only storage and we don't know whether the image can be opened read-write (for example, for NBD it depends on the server we're trying to connect to). This adds an option that makes QEMU try to open the image read-write, but allows it to degrade to a read-only mode without returning an error. The documentation for this option is consciously phrased in a way that allows QEMU to switch to a better model eventually: Instead of trying when the image is first opened, making the read-only flag dynamic and changing it automatically whenever the first BLK_PERM_WRITE user is attached or the last one is detached would be much more useful behaviour. Unfortunately, this more useful behaviour is also a lot harder to implement, and libvirt needs a solution now before it can switch to -blockdev, so let's start with this easier approach for now. Instead of adding a new auto-read-only option, turning the existing read-only into an enum (with a bool alternate for compatibility) was considered, but it complicated the implementation to the point that it didn't seem to be worth it. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-11-05quorum: Forbid adding children in blkverify modeAlberto Garcia
The blkverify mode of Quorum only works when the number of children is exactly two, so any attempt to add a new one must return an error. quorum_del_child() on the other hand doesn't need any additional check because decreasing the number of children would make it go under the vote threshold. Signed-off-by: Alberto Garcia <berto@igalia.com> Reported-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-05quorum: Return an error if the blkverify mode has invalid settingsAlberto Garcia
The blkverify mode of Quorum can only be enabled if the number of children is exactly two and the value of vote-threshold is also two. If the user tries to enable it but the other settings are incorrect then QEMU simply prints an error message to stderr and carries on disabling the blkverify setting. This patch makes quorum_open() fail and return an error in this case. Signed-off-by: Alberto Garcia <berto@igalia.com> Reported-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-05quorum: Remove quorum_err()Alberto Garcia
This is a static function with only one caller, so there's no need to keep it. Inlining the code in quorum_compare() makes it much simpler. Signed-off-by: Alberto Garcia <berto@igalia.com> Reported-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-05block/vdi: Don't take address of fields in packed structsPeter Maydell
Taking the address of a field in a packed struct is a bad idea, because it might not be actually aligned enough for that pointer type (and thus cause a crash on dereference on some host architectures). Newer versions of clang warn about this. Avoid the bug by not using the "modify in place" byte swapping functions. There are a few places where the in-place swap function is used on something other than a packed struct field; we convert those anyway, for consistency. Patch produced with scripts/coccinelle/inplace-byteswaps.cocci. There are other places where we take the address of a packed member in this file for other purposes than passing it to a byteswap function (all the calls to qemu_uuid_*()); we leave those for now. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-05block/vhdx: Don't take address of fields in packed structsPeter Maydell
Taking the address of a field in a packed struct is a bad idea, because it might not be actually aligned enough for that pointer type (and thus cause a crash on dereference on some host architectures). Newer versions of clang warn about this. Avoid the bug by not using the "modify in place" byte swapping functions. There are a few places where the in-place swap function is used on something other than a packed struct field; we convert those anyway, for consistency. Patch produced with scripts/coccinelle/inplace-byteswaps.cocci. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-05vpc: Don't leak opts in vpc_open()Kevin Wolf
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com>
2018-11-05block: change some function return type to boolLi Qiang
Signed-off-by: Li Qiang <liq3ea@163.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-05qcow2: Get the request alignment for encrypted images from QCryptoBlockAlberto Garcia
This doesn't have any practical effect at the moment because the values of BDRV_SECTOR_SIZE, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE and QCRYPTO_BLOCK_QCOW_SECTOR_SIZE are all the same (512 bytes), but future encryption methods could have different requirements. Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-05block/qcow2-bitmap: Don't take address of fields in packed structsPeter Maydell
Taking the address of a field in a packed struct is a bad idea, because it might not be actually aligned enough for that pointer type (and thus cause a crash on dereference on some host architectures). Newer versions of clang warn about this. Avoid the bug by not using the "modify in place" byte swapping functions. There are a few places where the in-place swap function is used on something other than a packed struct field; we convert those anyway, for consistency. This patch was produced with the following spatch script: @@ expression E; @@ -be16_to_cpus(&E); +E = be16_to_cpu(E); @@ expression E; @@ -be32_to_cpus(&E); +E = be32_to_cpu(E); @@ expression E; @@ -be64_to_cpus(&E); +E = be64_to_cpu(E); @@ expression E; @@ -cpu_to_be16s(&E); +E = cpu_to_be16(E); @@ expression E; @@ -cpu_to_be32s(&E); +E = cpu_to_be32(E); @@ expression E; @@ -cpu_to_be64s(&E); +E = cpu_to_be64(E); Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: John Snow <jsnow@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-05block/qcow: Don't take address of fields in packed structsPeter Maydell
Taking the address of a field in a packed struct is a bad idea, because it might not be actually aligned enough for that pointer type (and thus cause a crash on dereference on some host architectures). Newer versions of clang warn about this. Avoid the bug by not using the "modify in place" byte swapping functions. There are a few places where the in-place swap function is used on something other than a packed struct field; we convert those anyway, for consistency. This patch was produced with the following spatch script: @@ expression E; @@ -be16_to_cpus(&E); +E = be16_to_cpu(E); @@ expression E; @@ -be32_to_cpus(&E); +E = be32_to_cpu(E); @@ expression E; @@ -be64_to_cpus(&E); +E = be64_to_cpu(E); @@ expression E; @@ -cpu_to_be16s(&E); +E = cpu_to_be16(E); @@ expression E; @@ -cpu_to_be32s(&E); +E = cpu_to_be32(E); @@ expression E; @@ -cpu_to_be64s(&E); +E = cpu_to_be64(E); Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: John Snow <jsnow@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-05block/qcow2: Don't take address of fields in packed structsPeter Maydell
Taking the address of a field in a packed struct is a bad idea, because it might not be actually aligned enough for that pointer type (and thus cause a crash on dereference on some host architectures). Newer versions of clang warn about this. Avoid the bug by not using the "modify in place" byte swapping functions. There are a few places where the in-place swap function is used on something other than a packed struct field; we convert those anyway, for consistency. This patch was produced with the following spatch script (and hand-editing to fold a few resulting overlength lines): @@ expression E; @@ -be16_to_cpus(&E); +E = be16_to_cpu(E); @@ expression E; @@ -be32_to_cpus(&E); +E = be32_to_cpu(E); @@ expression E; @@ -be64_to_cpus(&E); +E = be64_to_cpu(E); @@ expression E; @@ -cpu_to_be16s(&E); +E = cpu_to_be16(E); @@ expression E; @@ -cpu_to_be32s(&E); +E = cpu_to_be32(E); @@ expression E; @@ -cpu_to_be64s(&E); +E = cpu_to_be64(E); Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: John Snow <jsnow@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-11-05block/vvfat: Fix crash when reporting error about too many files in directoryThomas Huth
When using the vvfat driver with a directory that contains too many files, QEMU currently crashes. This can be triggered like this for example: mkdir /tmp/vvfattest cd /tmp/vvfattest for ((x=0;x<=513;x++)); do mkdir $x; done qemu-system-x86_64 -drive \ file.driver=vvfat,file.dir=.,read-only=on,media=cdrom Seems like read_directory() is changing the mapping->path variable. Make sure we use the right pointer instead. Signed-off-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-10-29dirty-bitmaps: clean-up bitmaps loading and migration logicVladimir Sementsov-Ogievskiy
This patch aims to bring the following behavior: 1. We don't load bitmaps, when started in inactive mode. It's the case of incoming migration. In this case we wait for bitmaps migration through migration channel (if 'dirty-bitmaps' capability is enabled) or for invalidation (to load bitmaps from the image). 2. We don't remove persistent bitmaps on inactivation. Instead, we only remove bitmaps after storing. This is the only way to restore bitmaps, if we decided to resume source after [failed] migration with 'dirty-bitmaps' capability enabled (which means, that bitmaps were not stored). 3. We load bitmaps on open and any invalidation, it's ok for all cases: - normal open - migration target invalidation with dirty-bitmaps capability (bitmaps are migrating through migration channel, the are not stored, so they should have IN_USE flag set and will be skipped when loading. However, it would fail if bitmaps are read-only[1]) - migration target invalidation without dirty-bitmaps capability (normal load of the bitmaps, if migrated with shared storage) - source invalidation with dirty-bitmaps capability (skip because IN_USE) - source invalidation without dirty-bitmaps capability (bitmaps were dropped, reload them) [1]: to accurately handle this, migration of read-only bitmaps is explicitly forbidden in this patch. New mechanism for not storing bitmaps when migrate with dirty-bitmaps capability is introduced: migration filed in BdrvDirtyBitmap. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: John Snow <jsnow@redhat.com>
2018-10-29block/dirty-bitmaps: allow clear on disabled bitmapsJohn Snow
Similarly to merge, it's OK to allow clear operations on disabled bitmaps, as this condition only means that they are not recording new writes. We are free to clear it if the user requests it. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-id: 20181002230218.13949-4-jsnow@redhat.com Signed-off-by: John Snow <jsnow@redhat.com>
2018-10-29block/dirty-bitmaps: fix merge permissionsJohn Snow
In prior commits that made merge transactionable, we removed the assertion that merge cannot operate on disabled bitmaps. In addition, we want to make sure that we are prohibiting merges to "locked" bitmaps. Use the new user_locked function to check. Reported-by: Eric Blake <eblake@redhat.com> Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-id: 20181002230218.13949-3-jsnow@redhat.com Signed-off-by: John Snow <jsnow@redhat.com>
2018-10-29block/dirty-bitmaps: add user_locked status checkerJohn Snow
Instead of both frozen and qmp_locked checks, wrap it into one check. frozen implies the bitmap is split in two (for backup), and shouldn't be modified. qmp_locked implies it's being used by another operation, like being exported over NBD. In both cases it means we shouldn't allow the user to modify it in any meaningful way. Replace any usages where we check both frozen and qmp_locked with the new check. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 20181002230218.13949-2-jsnow@redhat.com [w/edits Suggested-By: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>] Signed-off-by: John Snow <jsnow@redhat.com>
2018-10-29bloc/qcow2: drop dirty_bitmaps_loaded state variableVladimir Sementsov-Ogievskiy
This variable doesn't work as it should, because it is actually cleared in qcow2_co_invalidate_cache() by memset(). Drop it, as the following patch will introduce new behavior. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com> Signed-off-by: John Snow <jsnow@redhat.com>
2018-10-29block/qcow2: improve error message in qcow2_inactivateVladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> [Maintainer edit -- touched up error message. --js] Reviewed-by: John Snow <jsnow@redhat.com> Signed-off-by: John Snow <jsnow@redhat.com>
2018-10-29dirty-bitmap: make it possible to restore bitmap after mergeVladimir Sementsov-Ogievskiy
Add backup parameter to bdrv_merge_dirty_bitmap() to be used then with bdrv_restore_dirty_bitmap() if it needed to restore the bitmap after merge operation. This is needed to implement bitmap merge transaction action in further commit. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com>
2018-10-29dirty-bitmap: rename bdrv_undo_clear_dirty_bitmapVladimir Sementsov-Ogievskiy
Use more generic names to reuse the function for bitmap merge in the following commit. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com>
2018-10-29dirty-bitmap: switch assert-fails to errors in bdrv_merge_dirty_bitmapVladimir Sementsov-Ogievskiy
Move checks from qmp_x_block_dirty_bitmap_merge() to bdrv_merge_dirty_bitmap(), to share them with dirty bitmap merge transaction action in future commit. Note: for now, only qmp_x_block_dirty_bitmap_merge() calls bdrv_merge_dirty_bitmap(). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com>
2018-10-19vpc: Fail open on bad header checksumMarkus Armbruster
vpc_open() merely prints a warning when it finds a bad header checksum. Turn that into a hard error. Cc: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20181017082702.5581-39-armbru@redhat.com> [Error message capitalized for local consistency] Reviewed-by: Kevin Wolf <kwolf@redhat.com>
2018-10-19block: Use warn_report() & friends to report warningsMarkus Armbruster
Calling error_report() in a function that takes an Error ** argument is suspicious. Convert a few that are actually warnings to warn_report(). While there, split warnings consisting of multiple sentences to conform to conventions spelled out in warn_report()'s contract, and improve a rather useless warning in sheepdog.c. Cc: Kevin Wolf <kwolf@redhat.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Peter Lieven <pl@kamp.de> Cc: Liu Yuan <namei.unix@gmail.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20181017082702.5581-4-armbru@redhat.com> Drop changes to "without an explicit read-only=on" warnings, because there's a series removing them pending. Also drop a cc: to a former Sheepdog maintainer. Reviewed-by: Kevin Wolf <kwolf@redhat.com>
2018-10-19error: Fix use of error_prepend() with &error_fatal, &error_abortMarkus Armbruster
From include/qapi/error.h: * Pass an existing error to the caller with the message modified: * error_propagate(errp, err); * error_prepend(errp, "Could not frobnicate '%s': ", name); Fei Li pointed out that doing error_propagate() first doesn't work well when @errp is &error_fatal or &error_abort: the error_prepend() is never reached. Since I doubt fixing the documentation will stop people from getting it wrong, introduce error_propagate_prepend(), in the hope that it lures people away from using its constituents in the wrong order. Update the instructions in error.h accordingly. Convert existing error_prepend() next to error_propagate to error_propagate_prepend(). If any of these get reached with &error_fatal or &error_abort, the error messages improve. I didn't check whether that's the case anywhere. Cc: Fei Li <fli@suse.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20181017082702.5581-2-armbru@redhat.com>
2018-10-12nvme: correct locking around completionPaolo Bonzini
nvme_poll_queues is already protected by q->lock, and AIO callbacks are invoked outside the AioContext lock. So remove the acquire/release pair in nvme_handle_event. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20180814062739.19640-1-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2018-10-01block-backend: Set werror/rerror defaults in blk_new()Kevin Wolf
Currently, the default values for werror and rerror have to be set explicitly with blk_set_on_error() by the callers of blk_new(). The only caller actually doing this is blockdev_init(), which is called for BlockBackends created using -drive. In particular, anonymous BlockBackends created with -device ...,drive=<node-name> didn't get the correct default set and instead defaulted to the integer value 0 (= BLOCKDEV_ON_ERROR_REPORT). This is the intended default for rerror anyway, but the default for werror should be BLOCKDEV_ON_ERROR_ENOSPC. Set the defaults in blk_new() instead so that they apply no matter what way the BlockBackend was created. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com>
2018-10-01qcow2: Explicit number replaced by a constantLeonid Bloch
Signed-off-by: Leonid Bloch <lbloch@janustech.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-10-01qcow2: Set the default cache-clean-interval to 10 minutesLeonid Bloch
The default cache-clean-interval is set to 10 minutes, in order to lower the overhead of the qcow2 caches (before the default was 0, i.e. disabled). * For non-Linux platforms the default is kept at 0, because cache-clean-interval is not supported there yet. Signed-off-by: Leonid Bloch <lbloch@janustech.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-10-01qcow2: Resize the cache upon image resizingLeonid Bloch
The caches are now recalculated upon image resizing. This is done because the new default behavior of assigning L2 cache relatively to the image size, implies that the cache will be adapted accordingly after an image resize. Signed-off-by: Leonid Bloch <lbloch@janustech.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-10-01qcow2: Increase the default upper limit on the L2 cache sizeLeonid Bloch
The upper limit on the L2 cache size is increased from 1 MB to 32 MB on Linux platforms, and to 8 MB on other platforms (this difference is caused by the ability to set intervals for cache cleaning on Linux platforms only). This is done in order to allow default full coverage with the L2 cache for images of up to 256 GB in size (was 8 GB). Note, that only the needed amount to cover the full image is allocated. The value which is changed here is just the upper limit on the L2 cache size, beyond which it will not grow, even if the size of the image will require it to. Signed-off-by: Leonid Bloch <lbloch@janustech.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-10-01qcow2: Assign the L2 cache relatively to the image sizeLeonid Bloch
Sufficient L2 cache can noticeably improve the performance when using large images with frequent I/O. Previously, unless 'cache-size' was specified and was large enough, the L2 cache was set to a certain size without taking the virtual image size into account. Now, the L2 cache assignment is aware of the virtual size of the image, and will cover the entire image, unless the cache size needed for that is larger than a certain maximum. This maximum is set to 1 MB by default (enough to cover an 8 GB image with the default cluster size) but can be increased or decreased using the 'l2-cache-size' option. This option was previously documented as the *maximum* L2 cache size, and this patch makes it behave as such, instead of as a constant size. Also, the existing option 'cache-size' can limit the sum of both L2 and refcount caches, as previously. Signed-off-by: Leonid Bloch <lbloch@janustech.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>