summaryrefslogtreecommitdiffstats
path: root/block.c
Commit message (Collapse)AuthorAgeFilesLines
* block: remove format defaults from QemuOpts in bdrv_create_file()Stefano Garzarella2021-03-191-1/+35
| | | | | | | | | | | | | | | | | | | | | | | QemuOpts is usually created merging the QemuOptsList of format and protocol. So, when the format calls bdr_create_file(), the 'opts' parameter contains a QemuOptsList with a combination of format and protocol default values. The format properly removes its options before calling bdr_create_file(), but the default values remain in 'opts->list'. So if the protocol has options with the same name (e.g. rbd has 'cluster_size' as qcow2), it will see the default values of the format, since for overlapping options, the format wins. To avoid this issue, lets convert QemuOpts to QDict, in this way we take only the set options, and then convert it back to QemuOpts, using the 'create_opts' of the protocol. So the new QemuOpts, will contain only the protocol defaults. Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-Id: <20210308161232.248833-1-sgarzare@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2021-03-09' into ↵Peter Maydell2021-03-111-4/+2Star
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | staging nbd patches for 2021-03-09 - Add Vladimir as NBD co-maintainer - Fix reporting of holes in NBD_CMD_BLOCK_STATUS - Improve command-line parsing accuracy of large numbers (anything going through qemu_strtosz), including the deprecation of hex+suffix - Improve some error reporting in the block layer # gpg: Signature made Tue 09 Mar 2021 15:38:10 GMT # 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-2021-03-09: block/qcow2: refactor qcow2_update_options_prepare error paths block/qed: bdrv_qed_do_open: deal with errp block/qcow2: simplify qcow2_co_invalidate_cache() block/qcow2: read_cache_sizes: return status value block/qcow2-bitmap: return status from qcow2_store_persistent_dirty_bitmaps block/qcow2-bitmap: improve qcow2_load_dirty_bitmaps() interface block/qcow2: qcow2_get_specific_info(): drop error propagation blockjob: return status from block_job_set_speed() block/mirror: drop extra error propagation in commit_active_start() block: drop extra error propagation for bdrv_set_backing_hd blockdev: fix drive_backup_prepare() missed error block: check return value of bdrv_open_child and drop error propagation utils: Deprecate hex-with-suffix sizes utils: Improve qemu_strtosz() to have 64 bits of precision utils: Enhance testsuite for do_strtosz() nbd: server: Report holes for raw images MAINTAINERS: add Vladimir as co-maintainer of NBD Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * block: drop extra error propagation for bdrv_set_backing_hdVladimir Sementsov-Ogievskiy2021-03-081-4/+2Star
| | | | | | | | | | | | | | | | | | | | bdrv_set_backing_hd now returns status, let's use it. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: Alberto Garcia <berto@igalia.com> Message-Id: <20210202124956.63146-6-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* | block: Clarify error messages pertaining to 'node-name'Connor Kuehl2021-03-081-4/+4
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some error messages contain ambiguous representations of the 'node-name' parameter. This can be particularly confusing when exchanging QMP messages (C = client, S = server): C: {"execute": "block_resize", "arguments": { "device": "my_file", "size": 26843545600 }} S: {"error": {"class": "GenericError", "desc": "Cannot find device=my_file nor node_name="}} ^^^^^^^^^ This error message suggests one could send a message with a key called 'node_name': C: {"execute": "block_resize", "arguments": { "node_name": "my_file", "size": 26843545600 }} ^^^^^^^^^ but using the underscore is actually incorrect, the parameter should be 'node-name': S: {"error": {"class": "GenericError", "desc": "Parameter 'node_name' is unexpected"}} This behavior was uncovered in bz1651437, but I ended up going down a rabbit hole looking for other areas where this miscommunication might occur and changing those accordingly as well. Fixes: https://bugzilla.redhat.com/1651437 Signed-off-by: Connor Kuehl <ckuehl@redhat.com> Message-Id: <20210305151929.1947331-2-ckuehl@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: add bdrv_co_delete_file_noerrMaxim Levitsky2021-02-151-0/+22
| | | | | | | | | | | | | | | This function wraps bdrv_co_delete_file for the common case of removing a file, which was just created by format driver, on an error condition. It hides the -ENOTSUPP error, and reports all other errors otherwise. Use it in luks driver Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Message-Id: <20201217170904.946013-3-mlevitsk@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: use return status of bdrv_append()Vladimir Sementsov-Ogievskiy2021-02-121-4/+2Star
| | | | | | | | | | Now bdrv_append returns status and we can drop all the local_err things around it. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Message-Id: <20210202124956.63146-3-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* block: return status from bdrv_append and friendsVladimir Sementsov-Ogievskiy2021-02-121-21/+37
| | | | | | | | | | | | | | | | The recommended use of qemu error api assumes returning status together with setting errp and avoid void functions with errp parameter. Let's improve bdrv_append and some friends to reduce error-propagation overhead in further patches. Choose int return status, because bdrv_replace_node_common() has call to bdrv_check_update_perm(), which reports int status, which seems correct to propagate. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20210202124956.63146-2-vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* block: move blk_exp_close_all() to qemu_cleanup()Sergio Lopez2021-02-021-1/+0Star
| | | | | | | | | | | | | | Move blk_exp_close_all() from bdrv_close() to qemu_cleanup(), before bdrv_drain_all_begin(). Export drivers may have coroutines yielding at some point in the block layer, so we need to shut them down before draining the block layer, as otherwise they may get stuck blk_wait_while_drained(). RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1900505 Signed-off-by: Sergio Lopez <slp@redhat.com> Message-Id: <20210201125032.44713-3-slp@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: Avoid processing BDS twice in bdrv_set_aio_context_ignore()Sergio Lopez2021-02-021-7/+27
| | | | | | | | | | | | | | | | | | Some graphs may contain an indirect reference to the first BDS in the chain that can be reached while walking it bottom->up from one its children. Doubling-processing of a BDS is especially problematic for the aio_notifiers, as they might attempt to work on both the old and the new AIO contexts. To avoid this problem, add every child and parent to the ignore list before actually processing them. Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Sergio Lopez <slp@redhat.com> Message-Id: <20210201125032.44713-2-slp@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: add API function to insert a nodeAndrey Shinkevich2021-01-261-0/+25
| | | | | | | | | | | Provide API for insertion a node to backing chain. Suggested-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20201216061703.70908-3-vsementsov@virtuozzo.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
* Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2020-12-19' into ↵Peter Maydell2021-01-011-11/+12
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | staging QAPI patches patches for 2020-12-19 # gpg: Signature made Sat 19 Dec 2020 09:40:05 GMT # gpg: using RSA key 354BC8B3D7EB2A6B68674E5F3870B400EB918653 # gpg: issuer "armbru@redhat.com" # gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" [full] # gpg: aka "Markus Armbruster <armbru@pond.sub.org>" [full] # Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867 4E5F 3870 B400 EB91 8653 * remotes/armbru/tags/pull-qapi-2020-12-19: (33 commits) qobject: Make QString immutable block: Use GString instead of QString to build filenames keyval: Use GString to accumulate value strings json: Use GString instead of QString to accumulate strings migration: Replace migration's JSON writer by the general one qobject: Factor JSON writer out of qobject_to_json() qobject: Factor quoted_str() out of to_json() qobject: Drop qstring_get_try_str() qobject: Drop qobject_get_try_str() Revert "qobject: let object_property_get_str() use new API" block: Avoid qobject_get_try_str() qmp: Fix tracing of non-string command IDs qobject: Move internals to qobject-internal.h hw/rdma: Replace QList by GQueue Revert "qstring: add qstring_free()" qobject: Change qobject_to_json()'s value to GString qobject: Use GString instead of QString to accumulate JSON qobject: Make qobject_to_json_pretty() take a pretty argument monitor: Use GString instead of QString for output buffer hmp: Simplify how qmp_human_monitor_command() gets output ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * block: Use GString instead of QString to build filenamesMarkus Armbruster2020-12-191-5/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | QString supports modifying its string, but it's quite limited: you can only append. Just one caller remains: bdrv_parse_filename_strip_prefix() uses it just for building an initial string. Change it to do build the initial string with GString. This is another step towards making QString immutable. Cc: Kevin Wolf <kwolf@redhat.com> Cc: Max Reitz <mreitz@redhat.com> Cc: qemu-block@nongnu.org Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20201211171152.146877-20-armbru@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
| * block: Avoid qobject_get_try_str()Markus Armbruster2020-12-191-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | I'm about to remove qobject_get_try_str(). Use qstring_get_str() instead. Safe because the argument is known to be a QString here. Cc: Kevin Wolf <kwolf@redhat.com> Cc: Max Reitz <mreitz@redhat.com> Cc: qemu-block@nongnu.org Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20201211171152.146877-11-armbru@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
| * qobject: Change qobject_to_json()'s value to GStringMarkus Armbruster2020-12-191-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | qobject_to_json() and qobject_to_json_pretty() build a GString, then covert it to QString. Just one of the callers actually needs a QString: qemu_rbd_parse_filename(). A few others need a string they can modify: qmp_send_response(), qga's send_response(), to_json_str(), and qmp_fd_vsend_fds(). The remainder just need a string. Change qobject_to_json() and qobject_to_json_pretty() to return the GString. qemu_rbd_parse_filename() now has to convert to QString. All others save a QString temporary. to_json_str() actually becomes a bit simpler, because GString provides more convenient modification functions. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20201211171152.146877-6-armbru@redhat.com>
* | block: bdrv_check_perm(): process children anywayVladimir Sementsov-Ogievskiy2020-12-181-2/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Do generic processing even for drivers which define .bdrv_check_perm handler. It's needed for further preallocate filter: it will need to do additional action on bdrv_check_perm, but don't want to reimplement generic logic. The patch doesn't change existing behaviour: the only driver that implements bdrv_check_perm is file-posix, but it never has any children. Also, bdrv_set_perm() don't stop processing if driver has .bdrv_set_perm handler as well. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20201021145859.11201-8-vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
* | block: drop tighten_restrictionsVladimir Sementsov-Ogievskiy2020-12-181-72/+17Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The only users of this thing are: 1. bdrv_child_try_set_perm, to ignore failures on loosen restrictions 2. assertion in bdrv_replace_child 3. assertion in bdrv_inactivate_recurse Assertions are not enough reason for overcomplication the permission update system. So, look at bdrv_child_try_set_perm. We are interested in tighten_restrictions only on failure. But on failure this field is not reliable: we may fail in the middle of permission update, some nodes are not touched and we don't know should their permissions be tighten or not. So, we rely on the fact that if we loose restrictions on some node (or BdrvChild), we'll not tighten restriction in the whole subtree as part of this update (assertions 2 and 3 rely on this fact as well). And, if we rely on this fact anyway, we can just check it on top, and don't pass additional pointer through the whole recursive infrastructure. Note also, that further patches will fix real bugs in permission update system, so now is good time to simplify it, as a help for further refactorings. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20201106124241.16950-8-vsementsov@virtuozzo.com> [mreitz: Fixed rebase conflict] Signed-off-by: Max Reitz <mreitz@redhat.com>
* | block: bdrv_child_set_perm() drop redundant parameters.Vladimir Sementsov-Ogievskiy2020-12-181-11/+4Star
| | | | | | | | | | | | | | | | | | | | We must set the permission used for _check_. Assert that we have backup and drop extra arguments. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20201106124241.16950-7-vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
* | block: bdrv_set_perm() drop redundant parameters.Vladimir Sementsov-Ogievskiy2020-12-181-14/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | We should never set permissions other than cumulative permissions of parents. During bdrv_reopen_multiple() we _check_ for synthetic permissions but when we do _set_ the graph is already updated. Add an assertion to bdrv_reopen_multiple(), other cases are more obvious. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20201106124241.16950-6-vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
* | block: add bdrv_refresh_perms() helperVladimir Sementsov-Ogievskiy2020-12-181-30/+31
|/ | | | | | | | | | Make separate function for common pattern. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20201106124241.16950-5-vsementsov@virtuozzo.com> [mreitz: Squashed in https://lists.nongnu.org/archive/html/qemu-block/2020-11/msg00299.html] Signed-off-by: Max Reitz <mreitz@redhat.com>
* block: introduce BDRV_MAX_LENGTHVladimir Sementsov-Ogievskiy2020-12-111-1/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We are going to modify block layer to work with 64bit requests. And first step is moving to int64_t type for both offset and bytes arguments in all block request related functions. It's mostly safe (when widening signed or unsigned int to int64_t), but switching from uint64_t is questionable. So, let's first establish the set of requests we want to work with. First signed int64_t should be enough, as off_t is signed anyway. Then, obviously offset + bytes should not overflow. And most interesting: (offset + bytes) being aligned up should not overflow as well. Aligned to what alignment? First thing that comes in mind is bs->bl.request_alignment, as we align up request to this alignment. But there is another thing: look at bdrv_mark_request_serialising(). It aligns request up to some given alignment. And this parameter may be bdrv_get_cluster_size(), which is often a lot greater than bs->bl.request_alignment. Note also, that bdrv_mark_request_serialising() uses signed int64_t for calculations. So, actually, we already depend on some restrictions. Happily, bdrv_get_cluster_size() returns int and bs->bl.request_alignment has 32bit unsigned type, but defined to be a power of 2 less than INT_MAX. So, we may establish, that INT_MAX is absolute maximum for any kind of alignment that may occur with the request. Note, that bdrv_get_cluster_size() is not documented to return power of 2, still bdrv_mark_request_serialising() behaves like it is. Also, backup uses bdi.cluster_size and is not prepared to it not being power of 2. So, let's establish that Qemu supports only power-of-2 clusters and alignments. So, alignment can't be greater than 2^30. Finally to be safe with calculations, to not calculate different maximums for different nodes (depending on cluster size and request_alignment), let's simply set QEMU_ALIGN_DOWN(INT64_MAX, 2^30) as absolute maximum bytes length for Qemu. Actually, it's not much less than INT64_MAX. OK, then, let's apply it to block/io. Let's consider all block/io entry points of offset/bytes: 4 bytes/offset interface functions: bdrv_co_preadv_part(), bdrv_co_pwritev_part(), bdrv_co_copy_range_internal() and bdrv_co_pdiscard() and we check them all with bdrv_check_request(). We also have one entry point with only offset: bdrv_co_truncate(). Check the offset. And one public structure: BdrvTrackedRequest. Happily, it has only three external users: file-posix.c: adopted by this patch write-threshold.c: only read fields test-write-threshold.c: sets obviously small constant values Better is to make the structure private and add corresponding interfaces.. Still it's not obvious what kind of interface is needed for file-posix.c. Let's keep it public but add corresponding assertions. After this patch we'll convert functions in block/io.c to int64_t bytes and offset parameters. We can assume that offset/bytes pair always satisfy new restrictions, and make corresponding assertions where needed. If we reach some offset/bytes point in block/io.c missing bdrv_check_request() it is considered a bug. As well, if block/io.c modifies a offset/bytes request, expanding it more then aligning up to request_alignment, it's a bug too. For all io requests except for discard we keep for now old restriction of 32bit request length. iotest 206 output error message changed, as now test disk size is larger than new limit. Add one more test case with new maximum disk size to cover too-big-L1 case. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20201203222713.13507-5-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* fuse: Allow exporting BDSs via FUSEMax Reitz2020-12-111-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | block-export-add type=fuse allows mounting block graph nodes via FUSE on some existing regular file. That file should then appears like a raw disk image, and accesses to it result in accesses to the exported BDS. Right now, we only implement the necessary block export functions to set it up and shut it down. We do not implement any access functions, so accessing the mount point only results in errors. This will be addressed by a followup patch. We keep a hash table of exported mount points, because we want to be able to detect when users try to use a mount point twice. This is because we invoke stat() to check whether the given mount point is a regular file, but if that file is served by ourselves (because it is already used as a mount point), then this stat() would have to be served by ourselves, too, which is impossible to do while we (as the caller) are waiting for it to settle. Therefore, keep track of mount point paths to at least catch the most obvious instances of that problem. Signed-off-by: Max Reitz <mreitz@redhat.com> Message-Id: <20201027190600.192171-3-mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: make bdrv_drop_intermediate() less wrongVladimir Sementsov-Ogievskiy2020-11-091-30/+24Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | First, permission update loop tries to do iterations transactionally, but the whole update is not transactional: nobody roll-back successful loop iterations when some iteration fails. Second, in the iteration we have nested permission update: c->klass->update_filename may point to bdrv_child_cb_update_filename() which calls bdrv_backing_update_filename(), which may do node reopen to RW. Permission update system is not prepared to nested updates, at least it has intermediate permission-update state stored in BdrvChild structures: has_backup_perm, backup_perm and backup_shared_perm. So, let's first do bdrv_replace_node_common() (which is more transactional than open-coded update in bdrv_drop_intermediate()) and then call update_filename() in separate. We still do not rollback changes in case of update_filename() failure but it's not much worse than pre-patch behavior. Note that bdrv_replace_node_common() does check for frozen children, so corresponding check is dropped in bdrv_drop_intermediate(). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20201106124241.16950-4-vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
* block: add bdrv_replace_node_common()Vladimir Sementsov-Ogievskiy2020-11-091-3/+22
| | | | | | | | | | | | | | | | | | Add new parameter to bdrv_replace_node(): auto_skip. With auto_skip=false we'll have stricter behavior: update _all_ from parents or fail. New behaviour will be used in the following commit in block.c, so keep original function name as public interface. Note: new error message is a bit funny in contrast with further "Cannot" in case of frozen child, but we'd better keep some difference to make it possible to distinguish one from another on failure. Still, actually we'd better refactor should_update_child() call to distinguish also different kinds of "should not". Let's do it later. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20201106124241.16950-3-vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
* block: add forgotten bdrv_abort_perm_update() to bdrv_co_invalidate_cache()Vladimir Sementsov-Ogievskiy2020-11-091-0/+1
| | | | | | | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20201106124241.16950-2-vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
* block: Fix integer promotion error in bdrv_getlength()Eric Blake2020-11-091-2/+7
| | | | | | | | | | | | | | | | Back in 2015, we attempted to fix error reporting for images that claimed to have more than INT64_MAX/512 sectors, but due to the type promotions caused by BDRV_SECTOR_SIZE being unsigned, this inadvertently forces all negative ret values to be slammed into -EFBIG rather than the original error. While we're at it, we can avoid the confusing ?: by spelling the logic more directly. Fixes: 4a9c9ea0d3 Reported-by: Guoyi Tu <tu.guoyi@h3c.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20201105155122.60943-1-eblake@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
* qapi: Add QAPI_LIST_PREPEND() macroEric Blake2020-10-301-16/+6Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | block.c has a useful macro QAPI_LIST_ADD() for inserting at the front of any QAPI-generated list; move it from block.c to qapi/util.h so more places can use it, including one earlier place in block.c, and rename it to something more obvious (since we also have a lot of places that append, rather than prepend, to a list). There are many more places in the codebase that can benefit from using the macro, but converting them will be left to later patches. In theory, all QAPI list types are child classes of GenericList; but in practice, that relationship is not explicitly spelled out in the C type declarations (rather, it is something that happens implicitly due to C compatible layouts), and the macro does not actually depend on the GenericList type. We considered moving GenericList from visitor.h into util.h to group related code; however, such a move would be awkward if we do not also move GenericAlternate. Unfortunately, moving GenericAlternate would introduce its own problems of declaration circularity (qapi-builtin-types.h needs a complete definition of QEnumLookup from util.h, but GenericAlternate needs a complete definition of QType from qapi-builtin-types.h). Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20201027050556.269064-3-eblake@redhat.com> [eblake: s/ADD/PREPEND/ per suggestion by Markus]
* block: Simplify QAPI_LIST_ADDEric Blake2020-10-301-1/+1
| | | | | | | | | | | | | | | | There is no need to rely on the verbosity of the gcc/clang compiler extension of g_new(typeof(X), 1) when we can instead use the standard g_malloc(sizeof(X)). In general, we like g_new over g_malloc for returning type X rather than void* to let the compiler catch more potential typing mistakes, but in this particular macro, our other use of typeof on the same line already ensures we are getting correct results. Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20201027050556.269064-2-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
* block: End quiescent sections when a BDS is deletedGreg Kurz2020-10-271-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If a BDS gets deleted during blk_drain_all(), it might miss a call to bdrv_do_drained_end(). This means missing a call to aio_enable_external() and the AIO context remains disabled for ever. This can cause a device to become irresponsive and to disrupt the guest execution, ie. hang, loop forever or worse. This scenario is quite easy to encounter with virtio-scsi on POWER when punching multiple blockdev-create QMP commands while the guest is booting and it is still running the SLOF firmware. This happens because SLOF disables/re-enables PCI devices multiple times via IO/MEM/MASTER bits of PCI_COMMAND register after the initial probe/feature negotiation, as it tends to work with a single device at a time at various stages like probing and running block/network bootloaders without doing a full reset in-between. This naturally generates many dataplane stops and starts, and thus many drain sections that can race with blockdev_create_run(). In the end, SLOF bails out. It is somehow reproducible on x86 but it requires to generate articial dataplane start/stop activity with stop/cont QMP commands. In this case, seabios ends up looping for ever, waiting for the virtio-scsi device to send a response to a command it never received. Add a helper that pairs all previously called bdrv_do_drained_begin() with a bdrv_do_drained_end() and call it from bdrv_close(). While at it, update the "/bdrv-drain/graph-change/drain_all" test in test-bdrv-drain so that it can catch the issue. BugId: https://bugzilla.redhat.com/show_bug.cgi?id=1874441 Signed-off-by: Greg Kurz <groug@kaod.org> Message-Id: <160346526998.272601.9045392804399803158.stgit@bahia.lan> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: Add bdrv_lock()/unlock()Kevin Wolf2020-10-091-0/+27
| | | | | | | | | | | | | | | Inside of coroutine context, we can't directly use aio_context_acquire() for the AioContext of a block node because we already own the lock of the current AioContext and we need to avoid double locking to prevent deadlocks. This provides helper functions to lock the AioContext of a node only if it's not the same as the current AioContext. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201005155855.256490-14-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
* block: Add bdrv_co_enter()/leave()Kevin Wolf2020-10-091-0/+23
| | | | | | | | | | Add a pair of functions to temporarily move the current coroutine to the AioContext of a given BlockDriverState. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201005155855.256490-13-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
* block: generate coroutine-wrapper codeVladimir Sementsov-Ogievskiy2020-10-051-73/+0Star
| | | | | | | | | | | Use code generation implemented in previous commit to generated coroutine wrappers in block.c and block/io.c Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20200924185414.28642-6-vsementsov@virtuozzo.com>
* block: declare some coroutine functions in block/coroutines.hVladimir Sementsov-Ogievskiy2020-10-051-4/+4
| | | | | | | | | | | | | | | | We are going to keep coroutine-wrappers code (structure-packing parameters, BDRV_POLL wrapper functions) in separate auto-generated files. So, we'll need a header with declaration of original _co_ functions, for those which are static now. As well, we'll need declarations for wrapper functions. Do these declarations now, as a preparation step. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20200924185414.28642-4-vsementsov@virtuozzo.com>
* block: return error-code from bdrv_invalidate_cacheVladimir Sementsov-Ogievskiy2020-10-051-14/+18
| | | | | | | | | | | | | | | | | | | | | This is the only coroutine wrapper from block.c and block/io.c which doesn't return a value, so let's convert it to the common behavior, to simplify moving to generated coroutine wrappers in a further commit. Also, bdrv_invalidate_cache is a void function, returning error only through **errp parameter, which is considered to be bad practice, as it forces callers to define and propagate local_err variable, so conversion is good anyway. This patch leaves the conversion of .bdrv_co_invalidate_cache() driver callbacks and bdrv_invalidate_cache_all() for another day. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20200924185414.28642-2-vsementsov@virtuozzo.com>
* block/export: Add blk_exp_close_all(_type)Kevin Wolf2020-10-021-1/+1
| | | | | | | | | | | | | | | | | This adds a function to shut down all block exports, and another one to shut down the block exports of a single type. The latter is used for now when stopping the NBD server. As soon as we implement support for multiple NBD servers, we'll need a per-server list of exports and it will be replaced by a function using that. As a side effect, the BlockExport layer has a list tracking all existing exports now. closed_exports loses its only user and can go away. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200924152717.287415-18-kwolf@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* qemu/atomic.h: rename atomic_ to qatomic_Stefan Hajnoczi2020-09-231-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | clang's C11 atomic_fetch_*() functions only take a C11 atomic type pointer argument. QEMU uses direct types (int, etc) and this causes a compiler error when a QEMU code calls these functions in a source file that also included <stdatomic.h> via a system header file: $ CC=clang CXX=clang++ ./configure ... && make ../util/async.c:79:17: error: address argument to atomic operation must be a pointer to _Atomic type ('unsigned int *' invalid) Avoid using atomic_*() names in QEMU's atomic.h since that namespace is used by <stdatomic.h>. Prefix QEMU's APIs with 'q' so that atomic.h and <stdatomic.h> can co-exist. I checked /usr/include on my machine and searched GitHub for existing "qatomic_" users but there seem to be none. This patch was generated using: $ git grep -h -o '\<atomic\(64\)\?_[a-z0-9_]\+' include/qemu/atomic.h | \ sort -u >/tmp/changed_identifiers $ for identifier in $(</tmp/changed_identifiers); do sed -i "s%\<$identifier\>%q$identifier%g" \ $(git grep -I -l "\<$identifier\>") done I manually fixed line-wrap issues and misaligned rST tables. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200923105646.47864-1-stefanha@redhat.com>
* qemu/: fix some comment spelling errorszhaolichang2020-09-171-1/+1
| | | | | | | | | | | I found that there are many spelling errors in the comments of qemu, so I used the spellcheck tool to check the spelling errors and finally found some spelling errors in the folder. Signed-off-by: zhaolichang <zhaolichang@huawei.com> Reviewed-by: Alex Bennee <alex.bennee@linaro.org> Message-Id: <20200917075029.313-2-zhaolichang@huawei.com> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
* block: Leave BDS.backing_{file,format} constantMax Reitz2020-09-071-8/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Parts of the block layer treat BDS.backing_file as if it were whatever the image header says (i.e., if it is a relative path, it is relative to the overlay), other parts treat it like a cache for bs->backing->bs->filename (relative paths are relative to the CWD). Considering bs->backing->bs->filename exists, let us make it mean the former. Among other things, this now allows the user to specify a base when using qemu-img to commit an image file in a directory that is not the CWD (assuming, everything uses relative filenames). Before this patch: $ ./qemu-img create -f qcow2 foo/bot.qcow2 1M $ ./qemu-img create -f qcow2 -b bot.qcow2 foo/mid.qcow2 $ ./qemu-img create -f qcow2 -b mid.qcow2 foo/top.qcow2 $ ./qemu-img commit -b mid.qcow2 foo/top.qcow2 qemu-img: Did not find 'mid.qcow2' in the backing chain of 'foo/top.qcow2' $ ./qemu-img commit -b foo/mid.qcow2 foo/top.qcow2 qemu-img: Did not find 'foo/mid.qcow2' in the backing chain of 'foo/top.qcow2' $ ./qemu-img commit -b $PWD/foo/mid.qcow2 foo/top.qcow2 qemu-img: Did not find '[...]/foo/mid.qcow2' in the backing chain of 'foo/top.qcow2' After this patch: $ ./qemu-img commit -b mid.qcow2 foo/top.qcow2 Image committed. $ ./qemu-img commit -b foo/mid.qcow2 foo/top.qcow2 qemu-img: Did not find 'foo/mid.qcow2' in the backing chain of 'foo/top.qcow2' $ ./qemu-img commit -b $PWD/foo/mid.qcow2 foo/top.qcow2 Image committed. With this change, bdrv_find_backing_image() must look at whether the user has overridden a BDS's backing file. If so, it can no longer use bs->backing_file, but must instead compare the given filename against the backing node's filename directly. Note that this changes the QAPI output for a node's backing_file. We had very inconsistent output there (sometimes what the image header said, sometimes the actual filename of the backing image). This inconsistent output was effectively useless, so we have to decide one way or the other. Considering that bs->backing_file usually at runtime contained the path to the image relative to qemu's CWD (or absolute), this patch changes QAPI's backing_file to always report the bs->backing->bs->filename from now on. If you want to receive the image header information, you have to refer to full-backing-filename. This necessitates a change to iotest 228. The interesting information it really wanted is the image header, and it can get that now, but it has to use full-backing-filename instead of backing_file. Because of this patch's changes to bs->backing_file's behavior, we also need some reference output changes. Along with the changes to bs->backing_file, stop updating BDS.backing_format in bdrv_backing_attach() as well. This way, ImageInfo's backing-filename and backing-filename-format fields will represent what the image header says and nothing else. iotest 245 changes in behavior: With the backing node no longer overriding the parent node's backing_file string, you can now omit the @backing option when reopening a node with neither a default nor a current backing file even if it used to have a backing node at some point. 273 also changes: The base image is opened without a format layer, so ImageInfo.backing-filename-format used to report "file" for the base image's overlay after blockdev-snapshot. However, the image header never says "file" anywhere, so it now reports $IMGFMT. Signed-off-by: Max Reitz <mreitz@redhat.com>
* block: Improve get_allocated_file_size's defaultMax Reitz2020-09-071-3/+39
| | | | | | | | | | | | | | | | | | | There are two practical problems with bdrv_get_allocated_file_size()'s default right now: (1) For drivers with children, we should generally sum all their sizes instead of just passing the request through to bs->file. The latter is good for filters, but not so much for format drivers. (2) Filters need not have bs->file, so we should actually go to the filtered child instead of hard-coding bs->file. Fix this by splitting the default implementation into three branches: (1) For filter drivers: Return the size of the filtered child (2) For protocol drivers: Return -ENOTSUP, because the default implementation cannot make a guess (3) For other drivers: Sum all data-bearing children's sizes Signed-off-by: Max Reitz <mreitz@redhat.com>
* block: Use CAFs for debug breakpointsMax Reitz2020-09-071-13/+3Star
| | | | | | | | | | When looking for a blkdebug node (which implements debug breakpoints), use bdrv_primary_bs() to iterate through the graph, because that is where a blkdebug node would be. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
* block: Use CAFs in bdrv_refresh_filename()Max Reitz2020-09-071-8/+21
| | | | | | | | | bdrv_refresh_filename() and the kind of related bdrv_dirname() should look to the primary child when they wish to copy the underlying file's filename. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
* block: Re-evaluate backing file handling in reopenMax Reitz2020-09-071-9/+40
| | | | | | | | | | | Reopening a node's backing child needs a bit of special handling because the "backing" child has different defaults than all other children (among other things). Adding filter support here is a bit more difficult than just using the child access functions. In fact, we often have to directly use bs->backing because these functions are about the "backing" child (which may or may not be the COW backing file). Signed-off-by: Max Reitz <mreitz@redhat.com>
* block: Use CAFs when working with backing chainsMax Reitz2020-09-071-14/+30
| | | | | | | | | | | Use child access functions when iterating through backing chains so filters do not break the chain. In addition, bdrv_find_overlay() will now always return the actual overlay; that is, it will never return a filter node but only one with a COW backing file (there may be filter nodes between that node and @bs). Signed-off-by: Max Reitz <mreitz@redhat.com>
* block: Use bdrv_filter_(bs|child) where obviousMax Reitz2020-09-071-11/+20
| | | | | | | | | | | | | | | | | | | | | Places that use patterns like if (bs->drv->is_filter && bs->file) { ... something about bs->file->bs ... } should be BlockDriverState *filtered = bdrv_filter_bs(bs); if (filtered) { ... something about @filtered ... } instead. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
* block: Add bdrv_supports_compressed_writes()Max Reitz2020-09-071-0/+23
| | | | | | | | | | | | | | | Filters cannot compress data themselves but they have to implement .bdrv_co_pwritev_compressed() still (or they cannot forward compressed writes). Therefore, checking whether bs->drv->bdrv_co_pwritev_compressed is non-NULL is not sufficient to know whether the node can actually handle compressed writes. This function looks down the filter chain to see whether there is a non-filter that can actually convert the compressed writes into compressed data (and thus normal writes). Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
* block: Drop bdrv_is_encrypted()Max Reitz2020-09-071-8/+0Star
| | | | | | | | | | | | | | | | | | | | | | | | | | The original purpose of bdrv_is_encrypted() was to inquire whether a BDS can be used without the user entering a password or not. It has not been used for that purpose for quite some time. Actually, it is not even fit for that purpose, because to answer that question, it would have recursively query all of the given node's children. So now we have to decide in which direction we want to fix bdrv_is_encrypted(): Recursively query all children, or drop it and just use bs->encrypted to get the current node's status? Nowadays, its only purpose is to report through bdrv_query_image_info() whether the given image is encrypted or not. For this purpose, it is probably more interesting to see whether a given node itself is encrypted or not (otherwise, a management application cannot discern for certain which nodes are really encrypted and which just have encrypted children). Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
* block: Include filters when freezing backing chainMax Reitz2020-09-071-21/+39
| | | | | | | | | | | | | | In order to make filters work in backing chains, the associated functions must be able to deal with them and freeze both COW and filter child links. While at it, add some comments that note which functions require their caller to ensure that a given child link is not frozen, and how the callers do so. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
* block: bdrv_set_backing_hd() is about bs->backingMax Reitz2020-09-071-2/+2
| | | | | | | | | | | bdrv_set_backing_hd() is a function that explicitly cares about the bs->backing child. Highlight that in its description and use child_bs(bs->backing) instead of backing_bs(bs) to make it more obvious. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
* block: bdrv_cow_child() for bdrv_has_zero_init()Max Reitz2020-09-071-1/+1
| | | | | | | | | bdrv_has_zero_init() should use bdrv_cow_child() if it wants to check whether the given BDS has a COW backing file. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
* block: Add chain helper functionsMax Reitz2020-09-071-0/+62
| | | | | | | Add some helper functions for skipping filters in a chain of block nodes. Signed-off-by: Max Reitz <mreitz@redhat.com>
* block: Add child access functionsMax Reitz2020-09-071-0/+91
| | | | | | | | | | | | | | | | | There are BDS children that the general block layer code can access, namely bs->file and bs->backing. Since the introduction of filters and external data files, their meaning is not quite clear. bs->backing can be a COW source, or it can be a filtered child; bs->file can be a filtered child, it can be data and metadata storage, or it can be just metadata storage. This overloading really is not helpful. This patch adds functions that retrieve the correct child for each exact purpose. Later patches in this series will make use of them. Doing so will allow us to handle filter nodes in a meaningful way. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
generated by cgit v1.2.3-55-g7522 (git 2.39.0) at 2024-12-18 07:17:50 +0100