aboutsummaryrefslogtreecommitdiff
path: root/blockdev.c
AgeCommit message (Collapse)AuthorFilesLines
2021-01-28qapi: Use QAPI_LIST_APPEND in trivial casesEric Blake1-8/+5
The easiest spots to use QAPI_LIST_APPEND are where we already have an obvious pointer to the tail of a list. While at it, consistently use the variable name 'tail' for that purpose. 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: <20210113221013.390592-5-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-01-26qapi: backup: disable copy_range by defaultVladimir Sementsov-Ogievskiy1-1/+1
Further commit will add a benchmark (scripts/simplebench/bench-backup.py), which will show that backup works better with async parallel requests (previous commit) and disabled copy_range. So, let's disable copy_range by default. Note: the option was added several commits ago with default to true, to follow old behavior (the feature was enabled unconditionally), and only now we are going to change the default behavior. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20210116214705.822267-19-vsementsov@virtuozzo.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2021-01-26qapi: backup: add max-chunk and max-workers to x-perf structVladimir Sementsov-Ogievskiy1-1/+7
Add new parameters to configure future backup features. The patch doesn't introduce aio backup requests (so we actually have only one worker) neither requests larger than one cluster. Still, formally we satisfy these maximums anyway, so add the parameters now, to facilitate further patch which will really change backup job behavior. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20210116214705.822267-11-vsementsov@virtuozzo.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2021-01-26qapi: backup: add perf.use-copy-range parameterVladimir Sementsov-Ogievskiy1-0/+8
Experiments show, that copy_range is not always making things faster. So, to make experimentation simpler, let's add a parameter. Some more perf parameters will be added soon, so here is a new struct. For now, add new backup qmp parameter with x- prefix for the following reasons: - We are going to add more performance parameters, some will be related to the whole block-copy process, some only to background copying in backup (ignored for copy-before-write operations). - On the other hand, we are going to use block-copy interface in other block jobs, which will need performance options as well.. And it should be the same structure or at least somehow related. So, there are too much unclean things about how the interface and now we need the new options mostly for testing. Let's keep them experimental for a while. In do_backup_common() new x-perf parameter handled in a way to make further options addition simpler. We add use-copy-range with default=true, and we'll change the default in further patch, after moving backup to use block-copy. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20210116214705.822267-2-vsementsov@virtuozzo.com> [mreitz: s/5\.2/6.0/] Signed-off-by: Max Reitz <mreitz@redhat.com>
2021-01-26qapi: block-stream: add "bottom" argumentVladimir Sementsov-Ogievskiy1-10/+49
The code already don't freeze base node and we try to make it prepared for the situation when base node is changed during the operation. In other words, block-stream doesn't own base node. Let's introduce a new interface which should replace the current one, which will in better relations with the code. Specifying bottom node instead of base, and requiring it to be non-filter gives us the following benefits: - drop difference between above_base and base_overlay, which will be renamed to just bottom, when old interface dropped - clean way to work with parallel streams/commits on the same backing chain, which otherwise become a problem when we introduce a filter for stream job - cleaner interface. Nobody will surprised the fact that base node may disappear during block-stream, when there is no word about "base" in the interface. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20201216061703.70908-11-vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2021-01-26stream: rework backing-file changingAndrey Shinkevich1-7/+1
Stream in stream_prepare calls bdrv_change_backing_file() to change backing-file in the metadata of bs. It may use either backing-file parameter given by user or just take filename of base on job start. Backing file format is determined by base on job finish. There are some problems with this design, we solve only two by this patch: 1. Consider scenario with backing-file unset. Current concept of stream supports changing of the base during the job (we don't freeze link to the base). So, we should not save base filename at job start, - let's determine name of the base on job finish. 2. Using direct base to determine filename and format is not very good: base node may be a filter, so its filename may be JSON, and format_name is not good for storing into qcow2 metadata as backing file format. - let's use unfiltered_base Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> [vsementsov: change commit subject, change logic in stream_prepare] Message-Id: <20201216061703.70908-10-vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2021-01-26qapi: add filter-node-name to block-streamAndrey Shinkevich1-1/+3
Provide the possibility to pass the 'filter-node-name' parameter to the block-stream job as it is done for the commit block job. Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> [vsementsov: comment indentation, s/Since: 5.2/Since: 6.0/] Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20201216061703.70908-5-vsementsov@virtuozzo.com> [mreitz: s/commit/stream/] Signed-off-by: Max Reitz <mreitz@redhat.com>
2020-12-13blockdev: Fix a memleak in drive_backup_prepare()Pan Nengyuan1-0/+1
'local_err' seems forgot to propagate in error path, it'll cause a memleak. Fix it. Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Li Qiang <liq3ea@gmail.com> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> Message-Id: <20201023061218.2080844-7-kuhn.chenqun@huawei.com> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
2020-12-11block: Fix locking in qmp_block_resize()Kevin Wolf1-1/+4
The drain functions assume that we hold the AioContext lock of the drained block node. Make sure to actually take the lock. Cc: qemu-stable@nongnu.org Fixes: eb94b81a94bce112e6b206df846c1551aaf6cab6 Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201203172311.68232-3-kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-12-11block: Simplify qmp_block_resize() error pathsKevin Wolf1-5/+4
The only thing that happens after the 'out:' label is blk_unref(blk). However, blk = NULL in all of the error cases, so instead of jumping to 'out:', we can just return directly. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201203172311.68232-2-kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-12-10Tweak a few "Parameter 'NAME' expects THING" error messageMarkus Armbruster1-1/+1
Change to "expects a THING" where that's an obvious improvement Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20201113082626.2725812-11-armbru@redhat.com>
2020-12-10block: Improve some block-commit, block-stream error messagesMarkus Armbruster1-6/+9
block-commit defaults @base-node to the deepest backing image. When there is none, it fails with "Base 'NULL' not found". Improve to "There is no backing image". block-commit and block-stream reject a @base argument that doesn't resolve with "Base 'BASE' not found". Commit 6b33f3ae8b "qemu-img: Improve commit invalid base message" improved this message in qemu-img. Improve it here, too: "Can't find '%s' in the backing chain". QERR_BASE_NOT_FOUND is now unused. Drop. 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: <20201113082626.2725812-4-armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2020-10-09block: Convert 'block_resize' to coroutineKevin Wolf1-8/+8
block_resize performs some I/O that could potentially take quite some time, so use it as an example for the new 'coroutine': true annotation in the QAPI schema. bdrv_truncate() requires that we're already in the right AioContext for the BlockDriverState if called in coroutine context. So instead of just taking the AioContext lock, move the QMP handler coroutine to the context. Call blk_unref() only after switching back because blk_unref() may only be called in the main thread. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201005155855.256490-15-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-06migration: introduce icount field for snapshotsPavel Dovgalyuk1-0/+10
Saving icount as a parameters of the snapshot allows navigation between them in the execution replay scenario. This information can be used for finding a specific snapshot for proceeding the recorded execution to the specific moment of the time. E.g., 'reverse step' action (introduced in one of the following patches) needs to load the nearest snapshot which is prior to the current moment of time. This patch also updates snapshot test which verifies qemu monitor output. Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru> Acked-by: Markus Armbruster <armbru@redhat.com> Acked-by: Kevin Wolf <kwolf@redhat.com> -- v4 changes: - squashed format update with test output update v7 changes: - introduced the spaces between the fields in snapshot info output - updated the test to match new field widths Message-Id: <160174518865.12451.14327573383978752463.stgit@pasha-ThinkPad-X280> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-09-23qemu/atomic.h: rename atomic_ to qatomic_Stefan Hajnoczi1-1/+1
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>
2020-09-07blockdev: Fix active commit choiceMax Reitz1-5/+30
We have to perform an active commit whenever the top node has a parent that has taken the WRITE permission on it. This means that block-commit's @backing-file parameter is no longer allowed for such nodes, and that users will have to issue a block-job-complete command. Neither should pose a problem in practice, because this case was basically just broken until now. (Since this commit already touches block-commit's documentation, it also moves up the chunk explaining general block-commit behavior that for some reason was situated under @backing-file.) Signed-off-by: Max Reitz <mreitz@redhat.com>
2020-09-07commit: Deal with filtersMax Reitz1-1/+3
This includes some permission limiting (for example, we only need to take the RESIZE permission if the base is smaller than the top). Signed-off-by: Max Reitz <mreitz@redhat.com>
2020-09-07backup: Deal with filtersMax Reitz1-4/+15
Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
2020-09-07mirror: Deal with filtersMax Reitz1-9/+23
This includes some permission limiting (for example, we only need to take the RESIZE permission for active commits where the base is smaller than the top). base_overlay is introduced so we can query bdrv_is_allocated_above() on it - we cannot do that with base itself, because a filter's block_status is the same as its child node, so if there are filters on base, bdrv_is_allocated_above() on base would return information including base. Use this opportunity to rename qmp_drive_mirror()'s "source" BDS to "target_backing_bs", because that is what it really refers to. Signed-off-by: Max Reitz <mreitz@redhat.com>
2020-09-07blockdev: Use CAF in external_snapshot_prepare()Max Reitz1-1/+6
This allows us to differentiate between filters and nodes with COW backing files: Filters cannot be used as overlays at all (for this function). Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
2020-09-07stream: Deal with filtersMax Reitz1-1/+3
Because of the (not so recent anymore) changes that make the stream job independent of the base node and instead track the node above it, we have to split that "bottom" node into two cases: The bottom COW node, and the node directly above the base node (which may be an R/W filter or the bottom COW node). Signed-off-by: Max Reitz <mreitz@redhat.com>
2020-07-14block: Add support to warn on backing file change without formatEric Blake1-1/+2
For now, this is a mechanical addition; all callers pass false. But the next patch will use it to improve 'qemu-img rebase -u' when selecting a backing file with no format. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Message-Id: <20200706203954.341758-10-eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-07-10error: Eliminate error_propagate() manuallyMarkus Armbruster1-12/+4
When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. The previous two commits did that for sufficiently simple cases with Coccinelle. Do it for several more manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-37-armbru@redhat.com>
2020-07-10error: Eliminate error_propagate() with Coccinelle, part 2Markus Armbruster1-2/+1
When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. The previous commit did that with a Coccinelle script I consider fairly trustworthy. This commit uses the same script with the matching of return taken out, i.e. we convert if (!foo(..., &err)) { ... error_propagate(errp, err); ... } to if (!foo(..., errp)) { ... ... } This is unsound: @err could still be read between afterwards. I don't know how to express "no read of @err without an intervening write" in Coccinelle. Instead, I manually double-checked for uses of @err. Suboptimal line breaks tweaked manually. qdev_realize() simplified further to placate scripts/checkpatch.pl. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-36-armbru@redhat.com>
2020-07-10error: Eliminate error_propagate() with Coccinelle, part 1Markus Armbruster1-4/+2
When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. Convert if (!foo(..., &err)) { ... error_propagate(errp, err); ... return ... } to if (!foo(..., errp)) { ... ... return ... } where nothing else needs @err. Coccinelle script: @rule1 forall@ identifier fun, err, errp, lbl; expression list args, args2; binary operator op; constant c1, c2; symbol false; @@ if ( ( - fun(args, &err, args2) + fun(args, errp, args2) | - !fun(args, &err, args2) + !fun(args, errp, args2) | - fun(args, &err, args2) op c1 + fun(args, errp, args2) op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; ) } @rule2 forall@ identifier fun, err, errp, lbl; expression list args, args2; expression var; binary operator op; constant c1, c2; symbol false; @@ - var = fun(args, &err, args2); + var = fun(args, errp, args2); ... when != err if ( ( var | !var | var op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; | return var; ) } @depends on rule1 || rule2@ identifier err; @@ - Error *err = NULL; ... when != err Not exactly elegant, I'm afraid. The "when != lbl:" is necessary to avoid transforming if (fun(args, &err)) { goto out } ... out: error_propagate(errp, err); even though other paths to label out still need the error_propagate(). For an actual example, see sclp_realize(). Without the "when strict", Coccinelle transforms vfio_msix_setup(), incorrectly. I don't know what exactly "when strict" does, only that it helps here. The match of return is narrower than what I want, but I can't figure out how to express "return where the operand doesn't use @err". For an example where it's too narrow, see vfio_intx_enable(). Silently fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Converted manually. Line breaks tidied up manually. One nested declaration of @local_err deleted manually. Preexisting unwanted blank line dropped in hw/riscv/sifive_e.c. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-35-armbru@redhat.com>
2020-07-10qemu-option: Use returned bool to check for failureMarkus Armbruster1-7/+4
The previous commit enables conversion of foo(..., &err); if (err) { ... } to if (!foo(..., &err)) { ... } for QemuOpts functions that now return true / false on success / error. Coccinelle script: @@ identifier fun = { opts_do_parse, parse_option_bool, parse_option_number, parse_option_size, qemu_opt_parse, qemu_opt_rename, qemu_opt_set, qemu_opt_set_bool, qemu_opt_set_number, qemu_opts_absorb_qdict, qemu_opts_do_parse, qemu_opts_from_qdict_entry, qemu_opts_set, qemu_opts_validate }; expression list args, args2; typedef Error; Error *err; @@ - fun(args, &err, args2); - if (err) + if (!fun(args, &err, args2)) { ... } A few line breaks tidied up manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-15-armbru@redhat.com> [Conflict with commit 0b6786a9c1 "block/amend: refactor qcow2 amend options" resolved by rerunning Coccinelle on master's version]
2020-07-10qemu-option: Make functions taking Error ** return bool, not voidMarkus Armbruster1-2/+3
See recent commit "error: Document Error API usage rules" for rationale. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-14-armbru@redhat.com>
2020-07-10qemu-option: Check return value instead of @err where convenientMarkus Armbruster1-3/+2
Convert uses like opts = qemu_opts_create(..., &err); if (err) { ... } to opts = qemu_opts_create(..., errp); if (!opts) { ... } Eliminate error_propagate() that are now unnecessary. Delete @err that are now unused. Note that we can't drop parallels_open()'s error_propagate() here. We continue to execute it even in the converted case. It's a no-op then: local_err is null. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20200707160613.848843-8-armbru@redhat.com>
2020-06-23blockdev: Deprecate -drive with bogus interface typeMarkus Armbruster1-2/+25
Drives with interface types other than if=none are for onboard devices. Unfortunately, any such drives the board doesn't pick up can still be used with -device, like this: $ qemu-system-x86_64 -nodefaults -display none -S -drive if=floppy,id=bogus,unit=7 -device ide-cd,drive=bogus -monitor stdio QEMU 5.0.50 monitor - type 'help' for more information (qemu) info block bogus: [not inserted] Attached to: /machine/peripheral-anon/device[0] Removable device: not locked, tray closed (qemu) info qtree bus: main-system-bus type System [...] bus: ide.1 type IDE dev: ide-cd, id "" ---> drive = "bogus" [...] unit = 0 (0x0) [...] This kind of abuse has always worked. Deprecate it: qemu-system-x86_64: -drive if=floppy,id=bogus,unit=7: warning: bogus if=floppy is deprecated, use if=none Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200622094227.1271650-9-armbru@redhat.com>
2020-05-19blockdev: Split off basic bitmap operations for qemu-imgEric Blake1-284/+0
Upcoming patches want to add some basic bitmap manipulation abilities to qemu-img. But blockdev.o is too heavyweight to link into qemu-img (among other things, it would drag in block jobs and transaction support - qemu-img does offline manipulation, where atomicity is less important because there are no concurrent modifications to compete with), so it's time to split off the bare bones of what we will need into a new file block/monitor/bitmap-qmp-cmds.o. This is sufficient to expose 6 QMP commands for use by qemu-img (add, remove, clear, enable, disable, merge), as well as move the three helper functions touched in the previous patch. Regarding MAINTAINERS, the new file is automatically part of block core, but also makes sense as related to other dirty bitmap files. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200513011648.166876-6-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2020-05-19blockdev: Promote several bitmap functions to non-staticEric Blake1-28/+19
The next patch will split blockdev.c, which will require accessing some previously-static functions from more than one .c file. But part of promoting a function to public is picking a naming scheme that does not reek of exposing too many internals (two of the three functions were named starting with 'do_'). To make future code motion easier, perform the function rename and non-static promotion into its own patch. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200513011648.166876-5-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2020-05-05block: Add blk_new_with_bs() helperEric Blake1-5/+3
There are several callers that need to create a new block backend from an existing BDS; make the task slightly easier with a common helper routine. Suggested-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424190903.522087-2-eblake@redhat.com> [mreitz: Set @ret only in error paths, see https://lists.nongnu.org/archive/html/qemu-block/2020-04/msg01216.html] Signed-off-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200428192648.749066-2-eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2020-04-30block-backend: Add flags to blk_truncate()Kevin Wolf1-1/+1
Now that node level interface bdrv_truncate() supports passing request flags to the block driver, expose this on the BlockBackend level, too. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200424125448.63318-4-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-04-30qapi: Only input visitors can actually failMarkus Armbruster1-14/+2
The previous few commits have made this more obvious, and removed the one exception. Time to clarify the documentation, and drop dead error checking. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424084338.26803-13-armbru@redhat.com>
2020-04-07job: take each job's lock individually in job_txn_applyStefan Reiter1-0/+9
All callers of job_txn_apply hold a single job's lock, but different jobs within a transaction can have different contexts, thus we need to lock each one individually before applying the callback function. Similar to job_completed_txn_abort this also requires releasing the caller's context before and reacquiring it after to avoid recursive locks which might break AIO_WAIT_WHILE in the callback. This is safe, since existing code would already have to take this into account, lest job_completed_txn_abort might have broken. This also brings to light a different issue: When a callback function in job_txn_apply moves it's job to a different AIO context, callers will try to release the wrong lock (now that we re-acquire the lock correctly, previously it would just continue with the old lock, leaving the job unlocked for the rest of the return path). Fix this by not caching the job's context. This is only necessary for qmp_block_job_finalize, qmp_job_finalize and job_exit, since everyone else calls through job_exit. One test needed adapting, since it calls job_finalize directly, so it manually needs to acquire the correct context. Signed-off-by: Stefan Reiter <s.reiter@proxmox.com> Message-Id: <20200407115651.69472-2-s.reiter@proxmox.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-03-11block: Fix cross-AioContext blockdev-snapshotKevin Wolf1-16/+0
external_snapshot_prepare() tries to move the overlay to the AioContext of the backing file (the snapshotted node). However, it's possible that this doesn't work, but the backing file can instead be moved to the overlay's AioContext (e.g. opening the backing chain for a mirror target). bdrv_append() already indirectly uses bdrv_attach_node(), which takes care to move nodes to make sure they use the same AioContext and which tries both directions. So the problem has a simple fix: Just delete the unnecessary extra bdrv_try_set_aio_context() call in external_snapshot_prepare() and instead assert in bdrv_append() that both nodes were indeed moved to the same AioContext. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20200310113831.27293-6-kwolf@redhat.com> Tested-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-03-11block: Relax restrictions for blockdev-snapshotKevin Wolf1-6/+8
blockdev-snapshot returned an error if the overlay was already in use, which it defined as having any BlockBackend parent. This is in fact both too strict (some parents can tolerate the change of visible data caused by attaching a backing file) and too loose (some non-BlockBackend parents may not be happy with it). One important use case that is prevented by the too strict check is live storage migration with blockdev-mirror. Here, the target node is usually opened without a backing file so that the active layer is mirrored while its backing chain can be copied in the background. The backing chain should be attached to the mirror target node when finalising the job, just before switching the users of the source node to the new copy (at which point the mirror job still has a reference to the node). drive-mirror did this automatically, but with blockdev-mirror this is the job of the QMP client, so it needs a way to do this. blockdev-snapshot is the obvious way, so this patch makes it work in this scenario. The new condition is that no parent uses CONSISTENT_READ permissions. This will ensure that the operation will still be blocked when the node is attached to the guest device, so blockdev-snapshot remains safe. (For the sake of completeness, x-blockdev-reopen can be used to achieve the same, however it is a big hammer, performs the graph change completely unchecked and is still experimental. So even with the option of using x-blockdev-reopen, there are reasons why blockdev-snapshot should be able to perform this operation.) Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20200310113831.27293-3-kwolf@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> Tested-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-03-09monitor/hmp: Move hmp_drive_add_node to block-hmp-cmds.cMaxim Levitsky1-35/+8
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Message-Id: <20200308092440.23564-12-mlevitsk@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
2020-03-09monitor/hmp: move hmp_drive_del and hmp_commit to block-hmp-cmds.cMaxim Levitsky1-95/+1
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Message-Id: <20200308092440.23564-5-mlevitsk@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
2020-03-06block: Move system emulator QMP commands to block/qapi-sysemu.cKevin Wolf1-559/+0
These commands make only sense for system emulators and their implementations call functions that don't exist in tools (e.g. to resolve qdev IDs). Move them out so that blockdev.c can be linked to qemu-storage-daemon. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20200224143008.13362-4-kwolf@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-02-20qapi: Allow getting flat output from 'query-named-block-nodes'Peter Krempa1-2/+6
When a management application manages node names there's no reason to recurse into backing images in the output of query-named-block-nodes. Add a parameter to the command which will return just the top level structs. Signed-off-by: Peter Krempa <pkrempa@redhat.com> Message-Id: <4470f8c779abc404dcf65e375db195cd91a80651.1579509782.git.pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [mreitz: Fixed coding style] Signed-off-by: Max Reitz <mreitz@redhat.com>
2020-02-18blockdev: Allow resizing everywhereMax Reitz1-5/+0
Block nodes that do not allow resizing should not share BLK_PERM_RESIZE. It does not matter whether they are the first non-filter in their chain or not. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200218103454.296704-3-mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-02-18blockdev: Allow external snapshots everywhereMax Reitz1-5/+0
There is no good reason why we would allow external snapshots only on the first non-filter node in a chain. Parent BDSs should not care whether their child is replaced by a snapshot. (If they do care, they should announce that via freezing the chain, which is checked in bdrv_append() through bdrv_set_backing_hd().) Before we had bdrv_is_first_non_filter() here (since 212a5a8f095), there was a special function bdrv_check_ext_snapshot() that allowed snapshots by default, but block drivers could override this. Only blkverify did so, however. It is not clear to me why blkverify would do so; maybe just so that the testee block driver would not be replaced. The introducing commit f6186f49e2c does not explain why. Maybe because 08b24cfe376 would have been the correct solution? (Which adds a .supports_backing check.) Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200218103454.296704-2-mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-02-18commit: Expose on-error option in QMPKevin Wolf1-4/+4
Now that the error handling in the common block job is fixed, we can expose the on-error option in QMP instead of hard-coding it as 'report' in qmp_block_commit(). This fulfills the promise that the old comment in that function made, even if a bit later than expected: "This will be part of the QMP command, if/when the BlockdevOnError change for blkmirror makes it in". Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20200214200812.28180-7-kwolf@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-01-30blockdev: adds bdrv_parse_aio to use io_uringAarushi Mehta1-8/+4
Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com> Acked-by: Stefano Garzarella <sgarzare@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20200120141858.587874-8-stefanha@redhat.com Message-Id: <20200120141858.587874-8-stefanha@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2020-01-27blockdev: Return bs to the proper context on snapshot abortSergio Lopez1-0/+21
external_snapshot_abort() calls to bdrv_set_backing_hd(), which returns state->old_bs to the main AioContext, as it's intended to be used then the BDS is going to be released. As that's not the case when aborting an external snapshot, return it to the AioContext it was before the call. This issue can be triggered by issuing a transaction with two actions, a proper blockdev-snapshot-sync and a bogus one, so the second will trigger a transaction abort. This results in a crash with an stack trace like this one: #0 0x00007fa1048b28df in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 #1 0x00007fa10489ccf5 in __GI_abort () at abort.c:79 #2 0x00007fa10489cbc9 in __assert_fail_base (fmt=0x7fa104a03300 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x5572240b44d8 "bdrv_get_aio_context(old_bs) == bdrv_get_aio_context(new_bs)", file=0x557224014d30 "block.c", line=2240, function=<optimized out>) at assert.c:92 #3 0x00007fa1048aae96 in __GI___assert_fail (assertion=assertion@entry=0x5572240b44d8 "bdrv_get_aio_context(old_bs) == bdrv_get_aio_context(new_bs)", file=file@entry=0x557224014d30 "block.c", line=line@entry=2240, function=function@entry=0x5572240b5d60 <__PRETTY_FUNCTION__.31620> "bdrv_replace_child_noperm") at assert.c:101 #4 0x0000557223e631f8 in bdrv_replace_child_noperm (child=0x557225b9c980, new_bs=new_bs@entry=0x557225c42e40) at block.c:2240 #5 0x0000557223e68be7 in bdrv_replace_node (from=0x557226951a60, to=0x557225c42e40, errp=0x5572247d6138 <error_abort>) at block.c:4196 #6 0x0000557223d069c4 in external_snapshot_abort (common=0x557225d7e170) at blockdev.c:1731 #7 0x0000557223d069c4 in external_snapshot_abort (common=0x557225d7e170) at blockdev.c:1717 #8 0x0000557223d09013 in qmp_transaction (dev_list=<optimized out>, has_props=<optimized out>, props=0x557225cc7d70, errp=errp@entry=0x7ffe704c0c98) at blockdev.c:2360 #9 0x0000557223e32085 in qmp_marshal_transaction (args=<optimized out>, ret=<optimized out>, errp=0x7ffe704c0d08) at qapi/qapi-commands-transaction.c:44 #10 0x0000557223ee798c in do_qmp_dispatch (errp=0x7ffe704c0d00, allow_oob=<optimized out>, request=<optimized out>, cmds=0x5572247d3cc0 <qmp_commands>) at qapi/qmp-dispatch.c:132 #11 0x0000557223ee798c in qmp_dispatch (cmds=0x5572247d3cc0 <qmp_commands>, request=<optimized out>, allow_oob=<optimized out>) at qapi/qmp-dispatch.c:175 #12 0x0000557223e06141 in monitor_qmp_dispatch (mon=0x557225c69ff0, req=<optimized out>) at monitor/qmp.c:120 #13 0x0000557223e0678a in monitor_qmp_bh_dispatcher (data=<optimized out>) at monitor/qmp.c:209 #14 0x0000557223f2f366 in aio_bh_call (bh=0x557225b9dc60) at util/async.c:117 #15 0x0000557223f2f366 in aio_bh_poll (ctx=ctx@entry=0x557225b9c840) at util/async.c:117 #16 0x0000557223f32754 in aio_dispatch (ctx=0x557225b9c840) at util/aio-posix.c:459 #17 0x0000557223f2f242 in aio_ctx_dispatch (source=<optimized out>, callback=<optimized out>, user_data=<optimized out>) at util/async.c:260 #18 0x00007fa10913467d in g_main_dispatch (context=0x557225c28e80) at gmain.c:3176 #19 0x00007fa10913467d in g_main_context_dispatch (context=context@entry=0x557225c28e80) at gmain.c:3829 #20 0x0000557223f31808 in glib_pollfds_poll () at util/main-loop.c:219 #21 0x0000557223f31808 in os_host_main_loop_wait (timeout=<optimized out>) at util/main-loop.c:242 #22 0x0000557223f31808 in main_loop_wait (nonblocking=<optimized out>) at util/main-loop.c:518 #23 0x0000557223d13201 in main_loop () at vl.c:1828 #24 0x0000557223bbfb82 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4504 RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1779036 Signed-off-by: Sergio Lopez <slp@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-01-27blockdev: Acquire AioContext on dirty bitmap functionsSergio Lopez1-4/+18
Dirty map addition and removal functions are not acquiring to BDS AioContext, while they may call to code that expects it to be acquired. This may trigger a crash with a stack trace like this one: #0 0x00007f0ef146370f in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 #1 0x00007f0ef144db25 in __GI_abort () at abort.c:79 #2 0x0000565022294dce in error_exit (err=<optimized out>, msg=msg@entry=0x56502243a730 <__func__.16350> "qemu_mutex_unlock_impl") at util/qemu-thread-posix.c:36 #3 0x00005650222950ba in qemu_mutex_unlock_impl (mutex=mutex@entry=0x5650244b0240, file=file@entry=0x565022439adf "util/async.c", line=line@entry=526) at util/qemu-thread-posix.c:108 #4 0x0000565022290029 in aio_context_release (ctx=ctx@entry=0x5650244b01e0) at util/async.c:526 #5 0x000056502221cd08 in bdrv_can_store_new_dirty_bitmap (bs=bs@entry=0x5650244dc820, name=name@entry=0x56502481d360 "bitmap1", granularity=granularity@entry=65536, errp=errp@entry=0x7fff22831718) at block/dirty-bitmap.c:542 #6 0x000056502206ae53 in qmp_block_dirty_bitmap_add (errp=0x7fff22831718, disabled=false, has_disabled=<optimized out>, persistent=<optimized out>, has_persistent=true, granularity=65536, has_granularity=<optimized out>, name=0x56502481d360 "bitmap1", node=<optimized out>) at blockdev.c:2894 #7 0x000056502206ae53 in qmp_block_dirty_bitmap_add (node=<optimized out>, name=0x56502481d360 "bitmap1", has_granularity=<optimized out>, granularity=<optimized out>, has_persistent=true, persistent=<optimized out>, has_disabled=false, disabled=false, errp=0x7fff22831718) at blockdev.c:2856 #8 0x00005650221847a3 in qmp_marshal_block_dirty_bitmap_add (args=<optimized out>, ret=<optimized out>, errp=0x7fff22831798) at qapi/qapi-commands-block-core.c:651 #9 0x0000565022247e6c in do_qmp_dispatch (errp=0x7fff22831790, allow_oob=<optimized out>, request=<optimized out>, cmds=0x565022b32d60 <qmp_commands>) at qapi/qmp-dispatch.c:132 #10 0x0000565022247e6c in qmp_dispatch (cmds=0x565022b32d60 <qmp_commands>, request=<optimized out>, allow_oob=<optimized out>) at qapi/qmp-dispatch.c:175 #11 0x0000565022166061 in monitor_qmp_dispatch (mon=0x56502450faa0, req=<optimized out>) at monitor/qmp.c:145 #12 0x00005650221666fa in monitor_qmp_bh_dispatcher (data=<optimized out>) at monitor/qmp.c:234 #13 0x000056502228f866 in aio_bh_call (bh=0x56502440eae0) at util/async.c:117 #14 0x000056502228f866 in aio_bh_poll (ctx=ctx@entry=0x56502440d7a0) at util/async.c:117 #15 0x0000565022292c54 in aio_dispatch (ctx=0x56502440d7a0) at util/aio-posix.c:459 #16 0x000056502228f742 in aio_ctx_dispatch (source=<optimized out>, callback=<optimized out>, user_data=<optimized out>) at util/async.c:260 #17 0x00007f0ef5ce667d in g_main_dispatch (context=0x56502449aa40) at gmain.c:3176 #18 0x00007f0ef5ce667d in g_main_context_dispatch (context=context@entry=0x56502449aa40) at gmain.c:3829 #19 0x0000565022291d08 in glib_pollfds_poll () at util/main-loop.c:219 #20 0x0000565022291d08 in os_host_main_loop_wait (timeout=<optimized out>) at util/main-loop.c:242 #21 0x0000565022291d08 in main_loop_wait (nonblocking=<optimized out>) at util/main-loop.c:518 #22 0x00005650220743c1 in main_loop () at vl.c:1828 #23 0x0000565021f20a72 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4504 Fix this by acquiring the AioContext at qmp_block_dirty_bitmap_add() and qmp_block_dirty_bitmap_add(). RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1782175 Signed-off-by: Sergio Lopez <slp@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-01-27blockdev: honor bdrv_try_set_aio_context() context requirementsSergio Lopez1-8/+60
bdrv_try_set_aio_context() requires that the old context is held, and the new context is not held. Fix all the occurrences where it's not done this way. Suggested-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Sergio Lopez <slp@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-01-27blockdev: unify qmp_blockdev_backup and blockdev-backup transaction pathsSergio Lopez1-47/+13
Issuing a blockdev-backup from qmp_blockdev_backup takes a slightly different path than when it's issued from a transaction. In the code, this is manifested as some redundancy between do_blockdev_backup() and blockdev_backup_prepare(). This change unifies both paths, merging do_blockdev_backup() and blockdev_backup_prepare(), and changing qmp_blockdev_backup() to create a transaction instead of calling do_backup_common() direcly. As a side-effect, now qmp_blockdev_backup() is executed inside a drained section, as it happens when creating a blockdev-backup transaction. This change is visible from the user's perspective, as the job gets paused and immediately resumed before starting the actual work. Signed-off-by: Sergio Lopez <slp@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-01-27blockdev: unify qmp_drive_backup and drive-backup transaction pathsSergio Lopez1-124/+100
Issuing a drive-backup from qmp_drive_backup takes a slightly different path than when it's issued from a transaction. In the code, this is manifested as some redundancy between do_drive_backup() and drive_backup_prepare(). This change unifies both paths, merging do_drive_backup() and drive_backup_prepare(), and changing qmp_drive_backup() to create a transaction instead of calling do_backup_common() direcly. As a side-effect, now qmp_drive_backup() is executed inside a drained section, as it happens when creating a drive-backup transaction. This change is visible from the user's perspective, as the job gets paused and immediately resumed before starting the actual work. Also fix tests 141, 185 and 219 to cope with the extra JOB_STATUS_CHANGE lines. Signed-off-by: Sergio Lopez <slp@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
generated by cgit v1.1 at 2024-12-27 05:09:43 +0000