summaryrefslogtreecommitdiffstats
path: root/blockjob.c
Commit message (Collapse)AuthorAgeFilesLines
* block: Make bdrv_child_get_parent_aio_context I/OHanna Reitz2022-11-101-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | We want to use bdrv_child_get_parent_aio_context() from bdrv_parent_drained_{begin,end}_single(), both of which are "I/O or GS" functions. Prior to 3ed4f708fe1, all the implementations were I/O code anyway. 3ed4f708fe1 has put block jobs' AioContext field under the job mutex, so to make child_job_get_parent_aio_context() work in an I/O context, we need to take that lock there. Furthermore, blk_root_get_parent_aio_context() is not marked as anything, but is safe to run in an I/O context, so mark it that way now. (blk_get_aio_context() is an I/O code function.) With that done, all implementations explicitly are I/O code, so we can mark bdrv_child_get_parent_aio_context() as I/O code, too, so callers know it is safe to run from both GS and I/O contexts. Signed-off-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20221107151321.211175-2-hreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: remove all unused ->can_set_aio_ctx and ->set_aio_ctx callbacksEmanuele Giuseppe Esposito2022-10-271-35/+0Star
| | | | | | | | | | Together with all _can_set_ and _set_ APIs, as they are not needed anymore. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20221025084952.2139888-9-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* blockjob: implement .change_aio_ctx in child_jobEmanuele Giuseppe Esposito2022-10-271-0/+45
| | | | | | | | | | | | | | | | | | | child_job_change_aio_ctx() is very similar to child_job_can_set_aio_ctx(), but it implements a new transaction so that if all check pass, the new transaction's .commit() will take care of changin the BlockJob AioContext. child_job_set_aio_ctx_commit() is similar to child_job_set_aio_ctx(), but it doesn't need to invoke the recursion, as this is already taken care by child_job_change_aio_ctx(). Note: bdrv_child_try_change_aio_context() is not called by anyone at this point. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20221025084952.2139888-5-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* blockjob: remove unused functionsEmanuele Giuseppe Esposito2022-10-071-14/+2Star
| | | | | | | | | | | These public functions are not used anywhere, thus can be dropped. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20220926093214.506243-21-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block_job_query: remove atomic readEmanuele Giuseppe Esposito2022-10-071-1/+1
| | | | | | | | | | | | | Not sure what the atomic here was supposed to do, since job.busy is protected by the job lock. Since the whole function is called under job_mutex, just remove the atomic. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20220926093214.506243-20-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* blockjob: protect iostatus field in BlockJob structEmanuele Giuseppe Esposito2022-10-071-2/+3
| | | | | | | | | | | | | | | | | | | iostatus is the only field (together with .job) that needs protection using the job mutex. It is set in the main loop (GLOBAL_STATE functions) but read in I/O code (block_job_error_action). In order to protect it, change block_job_iostatus_set_err to block_job_iostatus_set_err_locked(), always called under job lock. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20220926093214.506243-17-eesposit@redhat.com> [kwolf: Fixed up type of iostatus] Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* blockjob: rename notifier callbacks as _lockedEmanuele Giuseppe Esposito2022-10-071-10/+15
| | | | | | | | | | | They all are called with job_lock held, in job_event_*_locked() Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20220926093214.506243-16-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* jobs: protect job.aio_context with BQL and job_mutexEmanuele Giuseppe Esposito2022-10-071-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | In order to make it thread safe, implement a "fake rwlock", where we allow reads under BQL *or* job_mutex held, but writes only under BQL *and* job_mutex. The only write we have is in child_job_set_aio_ctx, which always happens under drain (so the job is paused). For this reason, introduce job_set_aio_context and make sure that the context is set under BQL, job_mutex and drain. Also make sure all other places where the aiocontext is read are protected. The reads in commit.c and mirror.c are actually safe, because always done under BQL. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Message-Id: <20220926093214.506243-14-eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* jobs: group together API calls under the same job lockEmanuele Giuseppe Esposito2022-10-071-13/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that the API offers also _locked() functions, take advantage of it and give also the caller control to take the lock and call _locked functions. This makes sense especially when we have for loops, because it makes no sense to have: for(job = job_next(); ...) where each job_next() takes the lock internally. Instead we want JOB_LOCK_GUARD(); for(job = job_next_locked(); ...) In addition, protect also direct field accesses, by either creating a new critical section or widening the existing ones. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-Id: <20220926093214.506243-12-eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* blockjob: introduce block_job _locked() APIsEmanuele Giuseppe Esposito2022-10-071-10/+42
| | | | | | | | | | | | | | | | | Just as done with job.h, create _locked() functions in blockjob.h These functions will be later useful when caller has already taken the lock. All blockjob _locked functions call job _locked functions. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20220926093214.506243-8-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* job: move and update comments from blockjob.cEmanuele Giuseppe Esposito2022-10-071-20/+0Star
| | | | | | | | | | | | | | | | | This comment applies more on job, it was left in blockjob as in the past the whole job logic was implemented there. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20220926093214.506243-7-eesposit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* assertions for blockjob.h global state APIEmanuele Giuseppe Esposito2022-03-041-0/+11
| | | | | | Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Message-Id: <20220303151616.325444-20-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* GS and IO CODE macros for blockjob_int.hEmanuele Giuseppe Esposito2022-03-041-0/+5
| | | | | | Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Message-Id: <20220303151616.325444-17-eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* blockjob: drop BlockJob.blk fieldVladimir Sementsov-Ogievskiy2021-12-281-19/+12Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's unused now (except for permission handling)[*]. The only reasonable user of it was block-stream job, recently updated to use own blk. And other block jobs prefer to use own source node related objects. So, the arguments of dropping the field are: - block jobs prefer not to use it - block jobs usually has more then one node to operate on, and better to operate symmetrically (for example has both source and target blk's in specific block-job state structure) *: BlockJob.blk is used to keep some permissions. We simply move permissions to block-job child created in block_job_create() together with blk. In mirror, we just should not care anymore about restoring state of blk. Most probably this code could be dropped long ago, after dropping bs->job pointer. Now it finally goes away together with BlockJob.blk itself. iotest 141 output is updated, as "bdrv_has_blk(bs)" check in qmp_blockdev_del() doesn't fail (we don't have blk now). Still, new error message looks even better. In iotest 283 we need to add a job id, otherwise "Invalid job ID" happens now earlier than permission check (as permissions moved from blk to block-job node). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Nikita Lapshin <nikita.lapshin@virtuozzo.com>
* blockjob: implement and use block_job_get_aio_contextVladimir Sementsov-Ogievskiy2021-12-281-0/+5
| | | | | | | | We are going to drop BlockJob.blk. So let's retrieve block job context from underlying job instead of main node. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Nikita Lapshin <nikita.lapshin@virtuozzo.com>
* progressmeter: protect with a mutexEmanuele Giuseppe Esposito2021-06-251-8/+25
| | | | | | | | | | | | | | | | | | | | Progressmeter is protected by the AioContext mutex, which is taken by the block jobs and their caller (like blockdev). We would like to remove the dependency of block layer code on the AioContext mutex, since most drivers and the core I/O code are already not relying on it. Create a new C file to implement the ProgressMeter API, but keep the struct as public, to avoid forcing allocation on the heap. Also add a mutex to be able to provide an accurate snapshot of the progress values to the caller. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20210614081130.22134-5-eesposit@redhat.com> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
* blockjob: let ratelimit handle a speed of 0Paolo Bonzini2021-06-251-10/+3Star
| | | | | | | | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Message-Id: <20210614081130.22134-4-eesposit@redhat.com> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
* ratelimit: protect with a mutexPaolo Bonzini2021-05-041-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | Right now, rate limiting is protected by the AioContext mutex, which is taken for example both by the block jobs and by qmp_block_job_set_speed (via find_block_job). We would like to remove the dependency of block layer code on the AioContext mutex, since most drivers and the core I/O code are already not relying on it. However, there is no existing lock that can easily be taken by both ratelimit_set_speed and ratelimit_calculate_delay, especially because the latter might run in coroutine context (and therefore under a CoMutex) but the former will not. Since concurrent calls to ratelimit_calculate_delay are not possible, one idea could be to use a seqlock to get a snapshot of slice_ns and slice_quota. But for now keep it simple, and just add a mutex to the RateLimit struct; block jobs are generally not performance critical to the point of optimizing the clock cycles spent in synchronization. This also requires the introduction of init/destroy functions, so add them to the two users of ratelimit.h. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* block: drop ctx argument from bdrv_root_attach_childVladimir Sementsov-Ogievskiy2021-04-301-2/+1Star
| | | | | | | | | | | | Passing parent aio context is redundant, as child_class and parent opaque pointer are enough to retrieve it. Drop the argument and use new bdrv_child_get_parent_aio_context() interface. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-7-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: BdrvChildClass: add .get_parent_aio_context handlerVladimir Sementsov-Ogievskiy2021-04-301-0/+8
| | | | | | | | | | | Add new handler to get aio context and implement it in all child classes. Add corresponding public interface to be used soon. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210428151804.439460-6-vsementsov@virtuozzo.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-10/+8Star
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * blockjob: return status from block_job_set_speed()Vladimir Sementsov-Ogievskiy2021-03-081-10/+8Star
| | | | | | | | | | | | | | | | | | | | | | Better to return status together with setting errp. It allows to avoid error propagation in the caller. 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-8-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* | blockjob: report a better error messageStefano Garzarella2021-03-081-3/+7
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | When a block job fails, we report strerror(-job->job.ret) error message, also if the job set an error object. Let's report a better error message using error_get_pretty(job->job.err). If an error object was not set, strerror(-job->ret) is used as fallback, as explained in include/qemu/job.h: typedef struct Job { ... /** * Error object for a failed job. * If job->ret is nonzero and an error object was not set, it will be set * to strerror(-job->ret) during job_completed. */ Error *err; } In block_job_query() there can be a transient where 'job.err' is not set by a scheduled bottom half. In that case we use strerror(-job->ret) as it was before. Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-Id: <20210225103633.76746-1-sgarzare@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* blockjob: Fix crash with IOthread when block commit after snapshotMichael Qiu2021-02-151-2/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, if guest has workloads, IO thread will acquire aio_context lock before do io_submit, it leads to segmentfault when do block commit after snapshot. Just like below: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7f7c7d91f700 (LWP 99907)] 0x00005576d0f65aab in bdrv_mirror_top_pwritev at ../block/mirror.c:1437 1437 ../block/mirror.c: No such file or directory. (gdb) p s->job $17 = (MirrorBlockJob *) 0x0 (gdb) p s->stop $18 = false Call trace of IO thread: 0 0x00005576d0f65aab in bdrv_mirror_top_pwritev at ../block/mirror.c:1437 1 0x00005576d0f7f3ab in bdrv_driver_pwritev at ../block/io.c:1174 2 0x00005576d0f8139d in bdrv_aligned_pwritev at ../block/io.c:1988 3 0x00005576d0f81b65 in bdrv_co_pwritev_part at ../block/io.c:2156 4 0x00005576d0f8e6b7 in blk_do_pwritev_part at ../block/block-backend.c:1260 5 0x00005576d0f8e84d in blk_aio_write_entry at ../block/block-backend.c:1476 ... Switch to qemu main thread: 0 0x00007f903be704ed in __lll_lock_wait at /lib/../lib64/libpthread.so.0 1 0x00007f903be6bde6 in _L_lock_941 at /lib/../lib64/libpthread.so.0 2 0x00007f903be6bcdf in pthread_mutex_lock at /lib/../lib64/libpthread.so.0 3 0x0000564b21456889 in qemu_mutex_lock_impl at ../util/qemu-thread-posix.c:79 4 0x0000564b213af8a5 in block_job_add_bdrv at ../blockjob.c:224 5 0x0000564b213b00ad in block_job_create at ../blockjob.c:440 6 0x0000564b21357c0a in mirror_start_job at ../block/mirror.c:1622 7 0x0000564b2135a9af in commit_active_start at ../block/mirror.c:1867 8 0x0000564b2133d132 in qmp_block_commit at ../blockdev.c:2768 9 0x0000564b2141fef3 in qmp_marshal_block_commit at qapi/qapi-commands-block-core.c:346 10 0x0000564b214503c9 in do_qmp_dispatch_bh at ../qapi/qmp-dispatch.c:110 11 0x0000564b21451996 in aio_bh_poll at ../util/async.c:164 12 0x0000564b2146018e in aio_dispatch at ../util/aio-posix.c:381 13 0x0000564b2145187e in aio_ctx_dispatch at ../util/async.c:306 14 0x00007f9040239049 in g_main_context_dispatch at /lib/../lib64/libglib-2.0.so.0 15 0x0000564b21447368 in main_loop_wait at ../util/main-loop.c:232 16 0x0000564b21447368 in main_loop_wait at ../util/main-loop.c:255 17 0x0000564b21447368 in main_loop_wait at ../util/main-loop.c:531 18 0x0000564b212304e1 in qemu_main_loop at ../softmmu/runstate.c:721 19 0x0000564b20f7975e in main at ../softmmu/main.c:50 In IO thread when do bdrv_mirror_top_pwritev, the job is NULL, and stop field is false, this means the MirrorBDSOpaque "s" object has not been initialized yet, and this object is initialized by block_job_create(), but the initialize process is stuck in acquiring the lock. In this situation, IO thread come to bdrv_mirror_top_pwritev(),which means that mirror-top node is already inserted into block graph, but its bs->opaque->job is not initialized. The root cause is that qemu main thread do release/acquire when hold the lock, at the same time, IO thread get the lock after release stage, and the crash occured. Actually, in this situation, job->job.aio_context will not equal to qemu_get_aio_context(), and will be the same as bs->aio_context, thus, no need to release the lock, becasue bdrv_root_attach_child() will not change the context. This patch fix this issue. Fixes: 132ada80 "block: Adjust AioContexts when attaching nodes" Signed-off-by: Michael Qiu <qiudayu@huayun.com> Message-Id: <20210203024059.52683-1-08005325@163.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* blockjob: add set_speed to BlockJobDriverVladimir Sementsov-Ogievskiy2021-01-261-0/+6
| | | | | | | | | | We are going to use async block-copy call in backup, so we'll need to passthrough setting backup speed to block-copy call. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20210116214705.822267-9-vsementsov@virtuozzo.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
* qemu/atomic.h: rename atomic_ to qatomic_Stefan Hajnoczi2020-09-231-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>
* block: Add BdrvChildRole to BdrvChildMax Reitz2020-05-181-2/+3
| | | | | | | | | | For now, it is always set to 0. Later patches in this series will ensure that all callers pass an appropriate combination of flags. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200513110544.176672-6-mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: Rename BdrvChildRole to BdrvChildClassMax Reitz2020-05-181-1/+1
| | | | | | | | | | | | | | This structure nearly only contains parent callbacks for child state changes. It cannot really reflect a child's role, because different roles may overlap (as we will see when real roles are introduced), and because parents can have custom callbacks even when the child fulfills a standard role. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Message-Id: <20200513110544.176672-4-mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: Add blk_new_with_bs() helperEric Blake2020-05-051-5/+2Star
| | | | | | | | | | | | | | | 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>
* job: refactor progress to separate objectVladimir Sementsov-Ogievskiy2020-03-111-8/+8
| | | | | | | | | | | | We need it in separate to pass to the block-copy object in the next commit. Cc: qemu-stable@nongnu.org Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200311103004.7649-2-vsementsov@virtuozzo.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
* blockjob: Fix error message for negative speedKevin Wolf2019-12-181-1/+2
| | | | | | | | | | | | | | | The error message for a negative speed uses QERR_INVALID_PARAMETER, which implies that the 'speed' option doesn't even exist: {"error": {"class": "GenericError", "desc": "Invalid parameter 'speed'"}} Make it use QERR_INVALID_PARAMETER_VALUE instead: {"error": {"class": "GenericError", "desc": "Parameter 'speed' expects a non-negative value"}} Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
* blockjob: update nodes head while removing all bdrvSergio Lopez2019-09-161-4/+13
| | | | | | | | | | | | | | | | | | | block_job_remove_all_bdrv() iterates through job->nodes, calling bdrv_root_unref_child() for each entry. The call to the latter may reach child_job_[can_]set_aio_ctx(), which will also attempt to traverse job->nodes, potentially finding entries that where freed on previous iterations. To avoid this situation, update job->nodes head on each iteration to ensure that already freed entries are no longer linked to the list. RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1746631 Signed-off-by: Sergio Lopez <slp@redhat.com> Cc: qemu-stable@nongnu.org Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20190911100316.32282-1-mreitz@redhat.com Reviewed-by: Sergio Lopez <slp@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
* job: drop job_drainVladimir Sementsov-Ogievskiy2019-09-101-13/+0Star
| | | | | | | | | | | | 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>
* Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into stagingPeter Maydell2019-08-161-0/+3
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Block layer patches: - file-posix: Fix O_DIRECT alignment detection - Fixes for concurrent block jobs - block-backend: Queue requests while drained (fix IDE vs. job crashes) - qemu-img convert: Deprecate using -n and -o together - iotests: Migration tests with filter nodes - iotests: More media change tests # gpg: Signature made Fri 16 Aug 2019 10:29:18 BST # gpg: using RSA key 7F09B272C88F2FD6 # gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full] # Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6 * remotes/kevin/tags/for-upstream: file-posix: Handle undetectable alignment qemu-img convert: Deprecate using -n and -o together block-backend: Queue requests while drained mirror: Keep mirror_top_bs drained after dropping permissions block: Remove blk_pread_unthrottled() iotests: Add test for concurrent stream/commit tests: Test mid-drain bdrv_replace_child_noperm() tests: Test polling in bdrv_drop_intermediate() block: Reduce (un)drains when replacing a child block: Keep subtree drained in drop_intermediate block: Simplify bdrv_filter_default_perms() iotests: Test migration with all kinds of filter nodes iotests: Move migration helpers to iotests.py iotests/118: Add -blockdev based tests iotests/118: Create test classes dynamically iotests/118: Test media change for scsi-cd Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * block-backend: Queue requests while drainedKevin Wolf2019-08-161-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This fixes devices like IDE that can still start new requests from I/O handlers in the CPU thread while the block backend is drained. The basic assumption is that in a drain section, no new requests should be allowed through a BlockBackend (blk_drained_begin/end don't exist, we get drain sections only on the node level). However, there are two special cases where requests should not be queued: 1. Block jobs: We already make sure that block jobs are paused in a drain section, so they won't start new requests. However, if the drain_begin is called on the job's BlockBackend first, it can happen that we deadlock because the job stays busy until it reaches a pause point - which it can't if its requests aren't processed any more. The proper solution here would be to make all requests through the job's filter node instead of using a BlockBackend. For now, just disabling request queuing on the job BlockBackend is simpler. 2. In test cases where making requests through bdrv_* would be cumbersome because we'd need a BdrvChild. As we already got the functionality to disable request queuing from 1., use it in tests, too, for convenience. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
* | Include qemu/main-loop.h lessMarkus Armbruster2019-08-161-0/+1
|/ | | | | | | | | | | | | | | | | | | | In my "build everything" tree, changing qemu/main-loop.h triggers a recompile of some 5600 out of 6600 objects (not counting tests and objects that don't depend on qemu/osdep.h). It includes block/aio.h, which in turn includes qemu/event_notifier.h, qemu/notify.h, qemu/processor.h, qemu/qsp.h, qemu/queue.h, qemu/thread-posix.h, qemu/thread.h, qemu/timer.h, and a few more. Include qemu/main-loop.h only where it's needed. Touching it now recompiles only some 1700 objects. For block/aio.h and qemu/event_notifier.h, these numbers drop from 5600 to 2800. For the others, they shrink only slightly. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20190812052359.30071-21-armbru@redhat.com> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
* block: Do not poll in bdrv_do_drained_end()Max Reitz2019-07-191-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We should never poll anywhere in bdrv_do_drained_end() (including its recursive callees like bdrv_drain_invoke()), because it does not cope well with graph changes. In fact, it has been written based on the postulation that no graph changes will happen in it. Instead, the callers that want to poll must poll, i.e. all currently globally available wrappers: bdrv_drained_end(), bdrv_subtree_drained_end(), bdrv_unapply_subtree_drain(), and bdrv_drain_all_end(). Graph changes there do not matter. They can poll simply by passing a pointer to a drained_end_counter and wait until it reaches 0. This patch also adds a non-polling global wrapper for bdrv_do_drained_end() that takes a drained_end_counter pointer. We need such a variant because now no function called anywhere from bdrv_do_drained_end() must poll. This includes BdrvChildRole.drained_end(), which already must not poll according to its interface documentation, but bdrv_child_cb_drained_end() just violates that by invoking bdrv_drained_end() (which does poll). Therefore, BdrvChildRole.drained_end() must take a *drained_end_counter parameter, which bdrv_child_cb_drained_end() can pass on to the new bdrv_drained_end_no_poll() function. Note that we now have a pattern of all drained_end-related functions either polling or receiving a *drained_end_counter to let the caller poll based on that. A problem with a single poll loop is that when the drained section in bdrv_set_aio_context_ignore() ends, some nodes in the subgraph may be in the old contexts, while others are in the new context already. To let the collective poll in bdrv_drained_end() work correctly, we must not hold a lock to the old context, so that the old context can make progress in case it is different from the current context. (In the process, remove the comment saying that the current context is always the old context, because it is wrong.) In all other places, all nodes in a subtree must be in the same context, so we can just poll that. The exception of course is bdrv_drain_all_end(), but that always runs in the main context, so we can just poll NULL (like bdrv_drain_all_begin() does). Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: drop bs->jobVladimir Sementsov-Ogievskiy2019-06-181-8/+0Star
| | | | | | | | | | | | | | | | | | | | | Drop remaining users of bs->job: 1. assertions actually duplicated by assert(!bs->refcnt) 2. trace-point seems not enough reason to change stream_start to return BlockJob pointer 3. Restricting creation of two jobs based on same bs is bad idea, as 3.1 Some jobs creates filters to be their main node, so, this check don't actually prevent creating second job on same real node (which will create another filter node) (but I hope it is restricted by other mechanisms) 3.2 Even without bs->job we have two systems of permissions: op-blockers and BLK_PERM 3.3 We may want to run several jobs on one node one day And finally, drop bs->job pointer itself. Hurrah! Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* blockdev: blockdev_mark_auto_del: drop usage of bs->jobVladimir Sementsov-Ogievskiy2019-06-181-0/+14
| | | | | | | | | | We are going to remove bs->job pointer. Drop it's usage in blockdev_mark_auto_del: instead of looking at bs->job let's check all jobs for references to bs. Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* Include qemu-common.h exactly where neededMarkus Armbruster2019-06-121-1/+0Star
| | | | | | | | | | | | | | | | No header includes qemu-common.h after this commit, as prescribed by qemu-common.h's file comment. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20190523143508.25387-5-armbru@redhat.com> [Rebased with conflicts resolved automatically, except for include/hw/arm/xlnx-zynqmp.h hw/arm/nrf51_soc.c hw/arm/msf2-soc.c block/qcow2-refcount.c block/qcow2-cluster.c block/qcow2-cache.c target/arm/cpu.h target/lm32/cpu.h target/m68k/cpu.h target/mips/cpu.h target/moxie/cpu.h target/nios2/cpu.h target/openrisc/cpu.h target/riscv/cpu.h target/tilegx/cpu.h target/tricore/cpu.h target/unicore32/cpu.h target/xtensa/cpu.h; bsd-user/main.c and net/tap-bsd.c fixed up]
* block: Adjust AioContexts when attaching nodesKevin Wolf2019-06-041-2/+8
| | | | | | | | | | | | | So far, we only made sure that updating the AioContext of a node affected the whole subtree. However, if a node is newly attached to a new parent, we also need to make sure that both the subtree of the node and the parent are in the same AioContext. This tries to move the new child node to the parent AioContext and returns an error if this isn't possible. BlockBackends now actually apply their AioContext to their root node. Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: Add BlockBackend.ctxKevin Wolf2019-06-041-1/+1
| | | | | | | | | | | | This adds a new parameter to blk_new() which requires its callers to declare from which AioContext this BlockBackend is going to be used (or the locks of which AioContext need to be taken anyway). The given context is only stored and kept up to date when changing AioContexts. Actually applying the stored AioContext to the root node is saved for another commit. Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: Make bdrv_root_attach_child() unref child_bs on failureAlberto Garcia2019-05-281-1/+1
| | | | | | | | | | | | | | | | | | | | | | A consequence of the previous patch is that bdrv_attach_child() transfers the reference to child_bs from the caller to parent_bs, which will drop it on bdrv_close() or when someone calls bdrv_unref_child(). But this only happens when bdrv_attach_child() succeeds. If it fails then the caller is responsible for dropping the reference to child_bs. This patch makes bdrv_attach_child() take the reference also when there is an error, freeing the caller for having to do it. A similar situation happens with bdrv_root_attach_child(), so the changes on this patch affect both functions. Signed-off-by: Alberto Garcia <berto@igalia.com> Message-id: 20dfb3d9ccec559cdd1a9690146abad5d204a186.1557754872.git.berto@igalia.com [mreitz: Removed now superfluous BdrvChild * variable in bdrv_open_child()] Signed-off-by: Max Reitz <mreitz@redhat.com>
* blockjob: Remove AioContext notifiersKevin Wolf2019-05-201-41/+2Star
| | | | | | | | | | | | | | | The notifiers made sure that the job is quiesced and that the job->aio_context field is updated. The first part is unnecessary today since bdrv_set_aio_context_ignore() drains the block node, and this means drainig the block job, too. The second part can be done in the .set_aio_ctx callback of the block job BdrvChildRole. The notifiers were problematic because they poll the AioContext while the graph is in an inconsistent state with some nodes already in the new context, but others still in the old context. So removing the notifiers not only simplifies the code, but actually makes the code safer. Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* blockjob: Propagate AioContext change to all job nodesKevin Wolf2019-05-201-0/+34
| | | | | | | | | | Block jobs require that all of the nodes the job is using are in the same AioContext. Therefore all BdrvChild objects of the job propagate .(can_)set_aio_context to all other job nodes, so that the switch is checked and performed consistently even if both nodes are in different subtrees. Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* blockjob: fix user pause in block_job_error_actionVladimir Sementsov-Ogievskiy2019-03-191-3/+5
| | | | | | | | | | | | | | | Job (especially mirror) may call block_job_error_action several times before actual pause if it has several in-flight requests. block_job_error_action will call job_pause more than once in this case, which lead to following block-job-resume qmp command can't actually resume the job. Fix it by do not increase pause level in block_job_error_action if user_paused already set. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: Use a single global AioWaitKevin Wolf2018-09-251-12/+1Star
| | | | | | | | | | | | | | | | | | | | | | | | | | When draining a block node, we recurse to its parent and for subtree drains also to its children. A single AIO_WAIT_WHILE() is then used to wait for bdrv_drain_poll() to become true, which depends on all of the nodes we recursed to. However, if the respective child or parent becomes quiescent and calls bdrv_wakeup(), only the AioWait of the child/parent is checked, while AIO_WAIT_WHILE() depends on the AioWait of the original node. Fix this by using a single AioWait for all callers of AIO_WAIT_WHILE(). This may mean that the draining thread gets a few more unnecessary wakeups because an unrelated operation got completed, but we already wake it up when something _could_ have changed rather than only if it has certainly changed. Apart from that, drain is a slow path anyway. In theory it would be possible to use wakeups more selectively and still correctly, but the gains are likely not worth the additional complexity. In fact, this patch is a nice simplification for some places in the code. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
* blockjob: Lie better in child_job_drained_poll()Kevin Wolf2018-09-251-1/+1
| | | | | | | | | | | | | | | | | | | | | | | Block jobs claim in .drained_poll() that they are in a quiescent state as soon as job->deferred_to_main_loop is true. This is obviously wrong, they still have a completion BH to run. We only get away with this because commit 91af091f923 added an unconditional aio_poll(false) to the drain functions, but this is bypassing the regular drain mechanisms. However, just removing this and telling that the job is still active doesn't work either: The completion callbacks themselves call drain functions (directly, or indirectly with bdrv_reopen), so they would deadlock then. As a better lie, tell that the job is active as long as the BH is pending, but falsely call it quiescent from the point in the BH when the completion callback is called. At this point, nested drain calls won't deadlock because they ignore the job, and outer drains will wait for the job to really reach a quiescent state because the callback is already running. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
* blockjob: Wake up BDS when job becomes idleKevin Wolf2018-09-251-0/+18
| | | | | | | | | | | | | | | | | In the context of draining a BDS, the .drained_poll callback of block jobs is called. If this returns true (i.e. there is still some activity pending), the drain operation may call aio_poll() with blocking=true to wait for completion. As soon as the pending activity is completed and the job finally arrives in a quiescent state (i.e. its coroutine either yields with busy=false or terminates), the block job must notify the aio_poll() loop to wake up, otherwise we get a deadlock if both are running in different threads. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
* qapi: Drop qapi_event_send_FOO()'s Error ** argumentPeter Xu2018-08-281-8/+5Star
| | | | | | | | | | | | | | | | | | | The generated qapi_event_send_FOO() take an Error ** argument. They can't actually fail, because all they do with the argument is passing it to functions that can't fail: the QObject output visitor, and the @qmp_emit callback, which is either monitor_qapi_event_queue() or event_test_emit(). Drop the argument, and pass &error_abort to the QObject output visitor and @qmp_emit instead. Suggested-by: Eric Blake <eblake@redhat.com> Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com> Message-Id: <20180815133747.25032-4-peterx@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Commit message rewritten, update to qapi-code-gen.txt corrected] Signed-off-by: Markus Armbruster <armbru@redhat.com>
generated by cgit v1.2.3-55-g7522 (git 2.39.0) at 2024-06-07 17:00:12 +0200