block_crypto_amend_options_generic_luks uses the block layer
permission API, therefore it should be called with the BQL held.
However, the same function is being called by two BlockDriver
callbacks: bdrv_amend_options (under BQL) and bdrv_co_amend (I/O).
The latter is I/O because it is invoked by block/amend.c's
blockdev_amend_run(), a .run callback of the amend JobDriver
Therefore we want to 1) change block_crypto driver
to use the permission API only when the BQL is held, and
2) use the .pre_run JobDriver callback to check for
permissions before switching to the job aiocontext. This has also
the benefit of applying the same permission operation to all
amend implementations, not only luks.
Remove the permission check in block_crypto_amend_options_generic_luks()
and:
- Add helper functions block_crypto_amend_options_{prepare/cleanup}
that take care of checking permissions in
block_crypto_amend_options_luks(), so when it is under BQL, and
- Use job->pre_run() and job->clean() to do the same thing when
we are in an iothread, by performing these checks before the
job runs in its aiocontext. So far job->pre_run() is only defined
but not called in job_start(), now it is the moment to use it.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block/crypto.c | 57 ++++++++++++++++++++++++++++++++------------------
job.c | 13 ++++++++++++
2 files changed, 50 insertions(+), 20 deletions(-)
diff --git a/block/crypto.c b/block/crypto.c
index f5e0c7b7c0..bdb4ba5664 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -791,6 +791,28 @@ block_crypto_amend_cleanup(BlockDriverState *bs)
crypto->updating_keys = false;
}
+static int
+block_crypto_amend_options_prepare(BlockDriverState *bs,
+ Error **errp)
+{
+ BlockCrypto *crypto = bs->opaque;
+
+ /* apply for exclusive read/write permissions to the underlying file*/
+ crypto->updating_keys = true;
+ return bdrv_child_refresh_perms(bs, bs->file, errp);
+}
+
+static int
+block_crypto_amend_options_cleanup(BlockDriverState *bs,
+ Error **errp)
+{
+ BlockCrypto *crypto = bs->opaque;
+
+ /* release exclusive read/write permissions to the underlying file*/
+ crypto->updating_keys = false;
+ return bdrv_child_refresh_perms(bs, bs->file, errp);
+}
+
static int
block_crypto_amend_options_generic_luks(BlockDriverState *bs,
QCryptoBlockAmendOptions
*amend_options,
@@ -798,30 +820,17 @@ block_crypto_amend_options_generic_luks(BlockDriverState
*bs,
Error **errp)
{
BlockCrypto *crypto = bs->opaque;
- int ret;
assert(crypto);
assert(crypto->block);
- /* apply for exclusive read/write permissions to the underlying file*/
- crypto->updating_keys = true;
- ret = bdrv_child_refresh_perms(bs, bs->file, errp);
- if (ret) {
- goto cleanup;
- }
-
- ret = qcrypto_block_amend_options(crypto->block,
- block_crypto_read_func,
- block_crypto_write_func,
- bs,
- amend_options,
- force,
- errp);
-cleanup:
- /* release exclusive read/write permissions to the underlying file*/
- crypto->updating_keys = false;
- bdrv_child_refresh_perms(bs, bs->file, errp);
- return ret;
+ return qcrypto_block_amend_options(crypto->block,
+ block_crypto_read_func,
+ block_crypto_write_func,
+ bs,
+ amend_options,
+ force,
+ errp);
}
static int
@@ -847,8 +856,16 @@ block_crypto_amend_options_luks(BlockDriverState *bs,
if (!amend_options) {
goto cleanup;
}
+
+ ret = block_crypto_amend_options_prepare(bs, errp);
+ if (ret) {
+ goto perm_cleanup;
+ }
ret = block_crypto_amend_options_generic_luks(bs, amend_options,
force, errp);
+
+perm_cleanup:
+ block_crypto_amend_options_cleanup(bs, errp);
cleanup:
qapi_free_QCryptoBlockAmendOptions(amend_options);
return ret;
diff --git a/job.c b/job.c
index 39bf511949..cf0dc9325a 100644
--- a/job.c
+++ b/job.c
@@ -967,11 +967,24 @@ static void coroutine_fn job_co_entry(void *opaque)
aio_bh_schedule_oneshot(qemu_get_aio_context(), job_exit, job);
}
+static int job_pre_run(Job *job)
+{
+ assert(qemu_in_main_thread());
+ if (job->driver->pre_run) {
+ return job->driver->pre_run(job, &job->err);
+ }
+
+ return 0;
+}
+
void job_start(Job *job)
{
assert(job && !job_started(job) && job->paused &&
job->driver && job->driver->run);
job->co = qemu_coroutine_create(job_co_entry, job);
+ if (job_pre_run(job)) {
+ return;