aboutsummaryrefslogtreecommitdiff
path: root/block/stream.c
AgeCommit message (Collapse)Author
2024-01-26stream: Allow users to request only format driver names in backing file formatPeter Krempa
Introduce a new flag 'backing-mask-protocol' for the block-stream QMP command which instructs the internals to use 'raw' instead of the protocol driver in case when a image is used without a dummy 'raw' wrapper. The flag is designed such that it can be always asserted by management tools even when there isn't any update to backing files. The flag will be used by libvirt so that the backing images still reference the proper format even when libvirt will stop using the dummy raw driver (raw driver with no other config). Libvirt needs this so that the images stay compatible with older libvirt versions which didn't expect that a protocol driver name can appear in the backing file format field. Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-ID: <bbee9a0a59748a8893289bf8249f568f0d587e62.1701796348.git.pkrempa@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-12-21graph-lock: remove AioContext lockingStefan Hajnoczi
Stop acquiring/releasing the AioContext lock in bdrv_graph_wrlock()/bdrv_graph_unlock() since the lock no longer has any effect. The distinction between bdrv_graph_wrunlock() and bdrv_graph_wrunlock_ctx() becomes meaningless and they can be collapsed into one function. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231205182011.1976568-6-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-11-21stream: Fix AioContext locking during bdrv_graph_wrlock()Kevin Wolf
In stream_prepare(), we need to temporarily drop the AioContext lock that job_prepare_locked() took for us while calling the graph write lock functions which can poll. All block nodes related to this block job are in the same AioContext, so we can pass any of them to bdrv_graph_wrlock()/ bdrv_graph_wrunlock(). Unfortunately, the one that we picked is base, which can be NULL - and in this case the AioContext lock is not released and deadlocks can occur. Fix this by passing s->target_bs, which is never NULL. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231115172012.112727-4-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-11-21block: Fix deadlocks in bdrv_graph_wrunlock()Kevin Wolf
bdrv_graph_wrunlock() calls aio_poll(), which may run callbacks that have a nested event loop. Nested event loops can depend on other iothreads making progress, so in order to allow them to make progress it must not hold the AioContext lock of another thread while calling aio_poll(). This introduces a @bs parameter to bdrv_graph_wrunlock() whose AioContext is temporarily dropped (which matches bdrv_graph_wrlock()), and a bdrv_graph_wrunlock_ctx() that can be used if the BlockDriverState doesn't necessarily exist any more when unlocking. This also requires a change to bdrv_schedule_unref(), which was relying on the incorrectly taken lock. It needs to take the lock itself now. While this is a separate bug, it can't be fixed a separate patch because otherwise the intermediate state would either deadlock or try to release a lock that we don't even hold. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231115172012.112727-3-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> [kwolf: Fixed up bdrv_schedule_unref()] Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-11-07block: Mark bdrv_set_backing_hd_drained() GRAPH_WRLOCKKevin Wolf
Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_set_backing_hd_drained(). Basically everthing in the function needs the lock and its callers may already want to hold the graph lock and so wouldn't be able to call functions that take it internally. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231027155333.420094-14-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-11-07block: Mark bdrv_cow_child() and callers GRAPH_RDLOCKKevin Wolf
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_cow_child() need to hold a reader lock for the graph because it accesses bs->backing. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231027155333.420094-13-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-11-07block: Mark bdrv_(un)freeze_backing_chain() and callers GRAPH_RDLOCKKevin Wolf
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_(un)freeze_backing_chain() need to hold a reader lock for the graph because it calls bdrv_filter_or_cow_child(), which accesses bs->file/backing. Use the opportunity to make bdrv_is_backing_chain_frozen() static, it has no external callers. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231027155333.420094-10-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-11-07block: Mark bdrv_skip_filters() and callers GRAPH_RDLOCKKevin Wolf
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_skip_filters() need to hold a reader lock for the graph because it calls bdrv_filter_child(), which accesses bs->file/backing. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231027155333.420094-9-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-11-07block: Mark bdrv_filter_or_cow_bs() and callers GRAPH_RDLOCKKevin Wolf
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_filter_or_cow_bs() need to hold a reader lock for the graph because it calls bdrv_filter_or_cow_child(), which accesses bs->file/backing. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231027155333.420094-7-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-11-07block: Mark block_job_add_bdrv() GRAPH_WRLOCKKevin Wolf
Instead of taking the writer lock internally, require callers to already hold it when calling block_job_add_bdrv(). These callers will typically already hold the graph lock once the locking work is completed, which means that they can't call functions that take it internally. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231027155333.420094-6-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-11-07block: Mark bdrv_filter_bs() and callers GRAPH_RDLOCKKevin Wolf
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_filter_bs() need to hold a reader lock for the graph because it calls bdrv_filter_child(), which accesses bs->file/backing. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231027155333.420094-4-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-10-12block: convert more bdrv_is_allocated* and bdrv_block_status* calls to ↵Paolo Bonzini
coroutine versions Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-ID: <20230904100306.156197-5-pbonzini@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-09-29block: Clean up local variable shadowingMarkus Armbruster
Local variables shadowing other local variables or parameters make the code needlessly hard to understand. Tracked down with -Wshadow=local. Clean up: delete inner declarations when they are actually redundant, else rename variables. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Ilya Dryomov <idryomov@gmail.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230921121312.1301864-7-armbru@redhat.com>
2023-09-20block: Mark bdrv_attach_child_common() GRAPH_WRLOCKKevin Wolf
Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_attach_child_common(). These callers will typically already hold the graph lock once the locking work is completed, which means that they can't call functions that take it internally. Note that the transaction callbacks still take the lock internally, so tran_finalize() must be called without the lock held. This is because bdrv_append() also calls bdrv_replace_node_noperm(), which currently requires the transaction callbacks to be called unlocked. In the next step, both of them can be switched to locked tran_finalize() calls together. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20230911094620.45040-11-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-05-19blockjob: Adhere to rate limit even when reentered earlyKevin Wolf
When jobs are sleeping, for example to enforce a given rate limit, they can be reentered early, in particular in order to get paused, to update the rate limit or to get cancelled. Before this patch, they behave in this case as if they had fully completed their rate limiting delay. This means that requests are sped up beyond their limit, violating the constraints that the user gave us. Change the block jobs to sleep in a loop until the necessary delay is completed, while still allowing cancelling them immediately as well pausing (handled by the pause point in job_sleep_ns()) and updating the rate limit. This change is also motivated by iotests cases being prone to fail because drain operations pause and unpause them so often that block jobs complete earlier than they are supposed to. In particular, the next commit would fail iotests 030 without this change. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230510203601.418015-8-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-02-24Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into stagingPeter Maydell
Block layer patches - Lock the graph, part 2 (BlockDriver callbacks) - virtio-scsi: fix SCSIDevice hot unplug with IOThread - rbd: Add support for layered encryption # -----BEGIN PGP SIGNATURE----- # # iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmP3tUURHGt3b2xmQHJl # ZGhhdC5jb20ACgkQfwmycsiPL9ZQkA/9HFBrcsfSyzU5sHXcpqrcVPsvFwwzhsXN # V6zMvBXQVEMYo6oDBSyNrniOJSYjiFLm1c+bMAaAFbo8dvVqqlkecBuZgQkFjnCy # vXyaYeWnBSG5A91Vs30qzLObBsrX7P1Gh+bvtRvBPThC1zd8lrxMbVzlsxnTfDFo # DsPkgiXL0SZ6YLBN5s61GBCfjvF8i0/8TPAvvwhHEo15sBgcBSTFYSftzEe9TXmH # NHAuHnRshrd9DNnf20tVPuHCanSTsIpbx5cLYBoy81vSbjqJG4agULZLltKP3fiM # kadpqmhJwjq+KhioLmcIjevPnUuqOMEzubaxZUm9o8jjsFPa8Isv4sIaAxyUP6e6 # aze1Xh9vUXn/JEf2/hApUY+2rz5dREL/TqpFwyzZjdqJb8PVCuy1JA1m2zLkvRPd # Bl9pS7kabhcZOHrITnJS7Lvyy4IWeiw78trtaer0nCbKbPdQB62eswSXKYh5g+Ke # kVJbkRSNi6lnljK5egIR3VxxM5kbGZsY4aGuyZk3Lc5yeAuPOil9swHlSO+5LFxP # lRZOyumHbfKU6J7JbGFErrqR2fZiqKUN/6i0HZAIcjpZq1QxXlmHBbmrkXao+j5Y # 0WcHdduH65dHT8fnBMgDZCXUfV7iBufspkCmY1v50YNJRPNmDzb4Os/Jh9qLHHMQ # M1ae+58T0Fo= # =gOli # -----END PGP SIGNATURE----- # gpg: Signature made Thu 23 Feb 2023 18:49:41 GMT # gpg: using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6 # gpg: issuer "kwolf@redhat.com" # gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full] # Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6 * tag 'for-upstream' of https://repo.or.cz/qemu/kevin: (29 commits) block/rbd: Add support for layered encryption block/rbd: Add luks-any encryption opening option block/rbd: Remove redundant stack variable passphrase_len virtio-scsi: reset SCSI devices from main loop thread dma-helpers: prevent dma_blk_cb() vs dma_aio_cancel() race scsi: protect req->aiocb with AioContext lock block: Mark bdrv_co_refresh_total_sectors() and callers GRAPH_RDLOCK block: Mark bdrv_*_dirty_bitmap() and callers GRAPH_RDLOCK block: Mark bdrv_co_delete_file() and callers GRAPH_RDLOCK block: Mark bdrv_(un)register_buf() GRAPH_RDLOCK block: Mark bdrv_co_eject/lock_medium() and callers GRAPH_RDLOCK block: Mark bdrv_co_is_inserted() and callers GRAPH_RDLOCK block: Mark bdrv_co_io_(un)plug() and callers GRAPH_RDLOCK block: Mark bdrv_co_create() and callers GRAPH_RDLOCK block: Mark preadv_snapshot/snapshot_block_status GRAPH_RDLOCK block: Mark bdrv_co_copy_range() GRAPH_RDLOCK block: Mark bdrv_co_do_pwrite_zeroes() GRAPH_RDLOCK block: Mark bdrv_co_pwrite_sync() and callers GRAPH_RDLOCK block: Mark public read/write functions GRAPH_RDLOCK block: Mark read/write in block/io.c GRAPH_RDLOCK ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2023-02-23block: Mark bdrv_co_refresh_total_sectors() and callers GRAPH_RDLOCKKevin Wolf
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_co_refresh_total_sectors() need to hold a reader lock for the graph. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230203152202.49054-24-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-02-23block: Mark bdrv_co_block_status() and callers GRAPH_RDLOCKKevin Wolf
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_co_block_status() need to hold a reader lock for the graph. For some places, we know that they will hold the lock, but we don't have the GRAPH_RDLOCK annotations yet. In this case, add assume_graph_lock() with a FIXME comment. These places will be removed once everything is properly annotated. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230203152202.49054-5-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-02-23error: Drop superfluous #include "qapi/qmp/qerror.h"Markus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20230207075115.1525-2-armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Juan Quintela <quintela@redhat.com> Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
2022-12-15stream: Replace subtree drain with a single node drainKevin Wolf
The subtree drain was introduced in commit b1e1af394d9 as a way to avoid graph changes between finding the base node and changing the block graph as necessary on completion of the image streaming job. The block graph could change between these two points because bdrv_set_backing_hd() first drains the parent node, which involved polling and can do anything. Subtree draining was an imperfect way to make this less likely (because with it, fewer callbacks are called during this window). Everyone agreed that it's not really the right solution, and it was only committed as a stopgap solution. This replaces the subtree drain with a solution that simply drains the parent node before we try to find the base node, and then call a version of bdrv_set_backing_hd() that doesn't drain, but just asserts that the parent node is already drained. This way, any graph changes caused by draining happen before we start looking at the graph and things stay consistent between finding the base node and changing the graph. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20221118174110.55183-10-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-03-29block/stream: Drain subtree around graph changeHanna Reitz
When the stream block job cuts out the nodes between top and base in stream_prepare(), it does not drain the subtree manually; it fetches the base node, and tries to insert it as the top node's backing node with bdrv_set_backing_hd(). bdrv_set_backing_hd() however will drain, and so the actual base node might change (because the base node is actually not part of the stream job) before the old base node passed to bdrv_set_backing_hd() is installed. This has two implications: First, the stream job does not keep a strong reference to the base node. Therefore, if it is deleted in bdrv_set_backing_hd()'s drain (e.g. because some other block job is drained to finish), we will get a use-after-free. We should keep a strong reference to that node. Second, even with such a strong reference, the problem remains that the base node might change before bdrv_set_backing_hd() actually runs and as a result the wrong base node is installed. Both effects can be seen in 030's TestParallelOps.test_overlapping_5() case, which has five nodes, and simultaneously streams from the middle node to the top node, and commits the middle node down to the base node. As it is, this will sometimes crash, namely when we encounter the above-described use-after-free. Taking a strong reference to the base node, we no longer get a crash, but the resuling block graph is less than ideal: The expected result is obviously that all middle nodes are cut out and the base node is the immediate backing child of the top node. However, if stream_prepare() takes a strong reference to its base node (the middle node), and then the commit job finishes in bdrv_set_backing_hd(), supposedly dropping that middle node, the stream job will just reinstall it again. Therefore, we need to keep the whole subtree drained in stream_prepare(), so that the graph modification it performs is effectively atomic, i.e. that the base node it fetches is still the base node when bdrv_set_backing_hd() sets it as the top node's backing node. Verify this by asserting in said 030's test case that the base node is always the top node's immediate backing child when both jobs are done. Signed-off-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20220324140907.17192-1-hreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Acked-by: Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>
2022-03-04assertions for block_int global state APIEmanuele Giuseppe Esposito
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Message-Id: <20220303151616.325444-13-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-12-28block/stream: add own blkVladimir Sementsov-Ogievskiy
block-stream is the only block-job, that reasonably use BlockJob.blk. We are going to drop BlockJob.blk soon. So, let block-stream have own blk. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Nikita Lapshin <nikita.lapshin@virtuozzo.com>
2021-11-16stream: Traverse graph after modificationHanna Reitz
bdrv_cor_filter_drop() modifies the block graph. That means that other parties can also modify the block graph before it returns. Therefore, we cannot assume that the result of a graph traversal we did before remains valid afterwards. We should thus fetch `base` and `unfiltered_base` afterwards instead of before. Signed-off-by: Hanna Reitz <hreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20211111120829.81329-2-hreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20211115145409.176785-2-kwolf@redhat.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
2021-03-19stream: Don't crash when node permission is deniedKevin Wolf
The image streaming block job restricts shared permissions of the nodes it accesses. This can obviously fail when other users already got these permissions. &error_abort is therefore wrong and can crash. Handle these errors gracefully and just fail starting the block job. Reported-by: Nini Gu <ngu@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210309173451.45152-1-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-01-26block: apply COR-filter to block-stream jobsAndrey Shinkevich
This patch completes the series with the COR-filter applied to block-stream operations. Adding the filter makes it possible in future implement discarding copied regions in backing files during the block-stream job, to reduce the disk overuse (we need control on permissions). Also, the filter now is smart enough to do copy-on-read with specified base, so we have benefit on guest reads even when doing block-stream of the part of the backing chain. Several iotests are slightly modified due to filter insertion. Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20201216061703.70908-14-vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2021-01-26block/stream: add s->target_bsVladimir Sementsov-Ogievskiy
Add a direct link to target bs for convenience and to simplify following commit which will insert COR filter above target bs. This is a part of original commit written by Andrey. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20201216061703.70908-13-vsementsov@virtuozzo.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2021-01-26qapi: block-stream: add "bottom" argumentVladimir Sementsov-Ogievskiy
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 Shinkevich
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 Shinkevich
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-10-30block: Return depth level during bdrv_is_allocated_aboveEric Blake
When checking for allocation across a chain, it's already easy to count the depth within the chain at which the allocation is found. Instead of throwing that information away, return it to the caller. Existing callers only cared about allocated/non-allocated, but having a depth available will be used by NBD in the next patch. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20201027050556.269064-9-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> [eblake: rebase to master] Signed-off-by: Eric Blake <eblake@redhat.com>
2020-09-07stream: Deal with filtersMax Reitz
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 Blake
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-03-09block/stream: Remove redundant statement in stream_run()Chen Qun
Clang static code analyzer show warning: block/stream.c:186:9: warning: Value stored to 'ret' is never read ret = 0; ^ ~ Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> Reviewed-by: John Snow <jsnow@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20200302130715.29440-3-kuhn.chenqun@huawei.com> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
2019-09-10job: drop job_drainVladimir Sementsov-Ogievskiy
In job_finish_sync job_enter should be enough for a job to make some progress and draining is a wrong tool for it. So use job_enter directly here and drop job_drain with all related staff not used more. Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Tested-by: John Snow <jsnow@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2019-08-15block/stream: use BDRV_REQ_PREFETCHVladimir Sementsov-Ogievskiy
This helps to avoid extra io, allocations and memory copying. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20190725100550.33801-3-vsementsov@virtuozzo.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> [eblake: fix comment grammar] Signed-off-by: Eric Blake <eblake@redhat.com>
2019-07-15block/stream: Swap backing file change orderMax Reitz
bdrv_change_backing_file() can result in yields. Therefore, @base may no longer be the the backing_bs() of s->bottom afterwards. Just swap the order of the two calls to fix this. Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20190703172813.6868-4-mreitz@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2019-07-15block/stream: Fix error pathMax Reitz
As of commit c624b015bf14fe01f1e6452a36e63b3ea1ae4998, the stream job only freezes the chain until the overlay of the base node. The error path must consider this. Fixes: c624b015bf14fe01f1e6452a36e63b3ea1ae4998 Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20190703172813.6868-3-mreitz@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2019-07-02block/stream: introduce a bottom nodeAndrey Shinkevich
The bottom node is the intermediate block device that has the base as its backing image. It is used instead of the base node while a block stream job is running to avoid dependency on the base that may change due to the parallel jobs. The change may take place due to a filter node as well that is inserted between the base and the intermediate bottom node. It occurs when the base node is the top one for another commit or stream job. After the introduction of the bottom node, don't freeze its backing child, that's the base, anymore. Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Message-id: 1559152576-281803-4-git-send-email-andrey.shinkevich@virtuozzo.com Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2019-07-02block/stream: refactor stream_run: drop gotoAndrey Shinkevich
The goto is unnecessary in the stream_run() since the common exit code was removed in the commit eb23654dbe43b549ea2a9ebff9d8e: "jobs: utilize job_exit shim". Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 1559152576-281803-3-git-send-email-andrey.shinkevich@virtuozzo.com Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2019-07-02block: include base when checking image chain for block allocationAndrey Shinkevich
This patch is used in the 'block/stream: introduce a bottom node' that is following. Instead of the base node, the caller may pass the node that has the base as its backing image to the function bdrv_is_allocated_above() with a new parameter include_base = true and get rid of the dependency on the base that may change during commit/stream parallel jobs. Now, if the specified base is not found in the backing image chain, the QEMU will abort. Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Message-id: 1559152576-281803-2-git-send-email-andrey.shinkevich@virtuozzo.com [mreitz: Squashed in the following as a rebase on conflicting patches:] Message-id: e3cf99ae-62e9-8b6e-5a06-d3c8b9363b85@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2019-04-30block/stream: use buffer-based ioVladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2019-04-02block: freeze the backing chain earlier in stream_start()Alberto Garcia
Commit 6585493369819a48d34a86d57ec6b97cb5cd9bc0 added code to freeze the backing chain from 'top' to 'base' for the duration of the block-stream job. The problem is that the freezing happens too late in stream_start(): during the bdrv_reopen_set_read_only() call earlier in that function another job can jump in and remove the base image. If that happens we have an invalid chain and QEMU crashes. This patch puts the bdrv_freeze_backing_chain() call at the beginning of the function. Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2019-03-12block: Freeze the backing chain for the duration of the stream jobAlberto Garcia
Signed-off-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2019-02-22block/stream: use QEMU_IOVEC_INIT_BUFVladimir Sementsov-Ogievskiy
Use new QEMU_IOVEC_INIT_BUF() instead of qemu_iovec_init_external( ... , 1), which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-7-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-7-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2018-12-14block: Use bdrv_reopen_set_read_only() in stream_start/complete()Alberto Garcia
This patch replaces the bdrv_reopen() calls that set and remove the BDRV_O_RDWR flag with the new bdrv_reopen_set_read_only() function. Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-09-25block/stream: refactor stream to use job callbacksJohn Snow
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 20180906130225.5118-8-jsnow@redhat.com Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2018-09-25block/stream: add block job creation flagsJohn Snow
Add support for taking and passing forward job creation flags. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 20180906130225.5118-4-jsnow@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2018-08-31jobs: utilize job_exit shimJohn Snow
Utilize the job_exit shim by not calling job_defer_to_main_loop, and where applicable, converting the deferred callback into the job_exit callback. This converts backup, stream, create, and the unit tests all at once. Most of these jobs do not see any changes to the order in which they clean up their resources, except the test-blockjob-txn test, which now puts down its bs before job_completed is called. This is safe for the same reason the reordering in the mirror job is safe, because job_completed no longer runs under two locks, making the unref safe even if it causes a flush. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 20180830015734.19765-7-jsnow@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2018-08-31jobs: canonize Error objectJohn Snow
Jobs presently use both an Error object in the case of the create job, and char strings in the case of generic errors elsewhere. Unify the two paths as just j->err, and remove the extra argument from job_completed. The integer error code for job_completed is kept for now, to be removed shortly in a separate patch. Signed-off-by: John Snow <jsnow@redhat.com> Message-id: 20180830015734.19765-3-jsnow@redhat.com [mreitz: Dropped a superfluous g_strdup()] Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>