qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: qemu-block@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>, "Fam Zheng" <fam@euphon.net>,
	"Vladimir Sementsov-Ogievskiy" <vsementsov@virtuozzo.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Juan Quintela" <quintela@redhat.com>,
	qemu-devel@nongnu.org, "John Snow" <jsnow@redhat.com>,
	"Emanuele Giuseppe Esposito" <eesposit@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Hanna Reitz" <hreitz@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Eric Blake" <eblake@redhat.com>
Subject: [PATCH v5 30/31] crypto: delegate permission functions to JobDriver .pre_run
Date: Wed, 24 Nov 2021 01:44:17 -0500	[thread overview]
Message-ID: <20211124064418.3120601-31-eesposit@redhat.com> (raw)
In-Reply-To: <20211124064418.3120601-1-eesposit@redhat.com>

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 ib 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_amend_options_generic_luks
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.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block/amend.c  | 20 ++++++++++++++++++++
 block/crypto.c | 18 ++++++++++++------
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/block/amend.c b/block/amend.c
index 392df9ef83..fba6add51a 100644
--- a/block/amend.c
+++ b/block/amend.c
@@ -53,10 +53,30 @@ static int coroutine_fn blockdev_amend_run(Job *job, Error **errp)
     return ret;
 }
 
+static int blockdev_amend_refresh_perms(Job *job, Error **errp)
+{
+    BlockdevAmendJob *s = container_of(job, BlockdevAmendJob, common);
+
+    return bdrv_child_refresh_perms(s->bs, s->bs->file, errp);
+}
+
+static int blockdev_amend_pre_run(Job *job, Error **errp)
+{
+    return blockdev_amend_refresh_perms(job, errp);
+}
+
+static void blockdev_amend_clean(Job *job)
+{
+    Error *errp;
+    blockdev_amend_refresh_perms(job, &errp);
+}
+
 static const JobDriver blockdev_amend_job_driver = {
     .instance_size = sizeof(BlockdevAmendJob),
     .job_type      = JOB_TYPE_AMEND,
     .run           = blockdev_amend_run,
+    .pre_run       = blockdev_amend_pre_run,
+    .clean         = blockdev_amend_clean,
 };
 
 void qmp_x_blockdev_amend(const char *job_id,
diff --git a/block/crypto.c b/block/crypto.c
index c8ba4681e2..82f154516c 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -780,6 +780,7 @@ block_crypto_get_specific_info_luks(BlockDriverState *bs, Error **errp)
 static int
 block_crypto_amend_options_generic_luks(BlockDriverState *bs,
                                         QCryptoBlockAmendOptions *amend_options,
+                                        bool under_bql,
                                         bool force,
                                         Error **errp)
 {
@@ -791,9 +792,12 @@ block_crypto_amend_options_generic_luks(BlockDriverState *bs,
 
     /* 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;
+
+    if (under_bql) {
+        ret = bdrv_child_refresh_perms(bs, bs->file, errp);
+        if (ret) {
+            goto cleanup;
+        }
     }
 
     ret = qcrypto_block_amend_options(crypto->block,
@@ -806,7 +810,9 @@ block_crypto_amend_options_generic_luks(BlockDriverState *bs,
 cleanup:
     /* release exclusive read/write permissions to the underlying file*/
     crypto->updating_keys = false;
-    bdrv_child_refresh_perms(bs, bs->file, errp);
+    if (under_bql) {
+        bdrv_child_refresh_perms(bs, bs->file, errp);
+    }
     return ret;
 }
 
@@ -834,7 +840,7 @@ block_crypto_amend_options_luks(BlockDriverState *bs,
         goto cleanup;
     }
     ret = block_crypto_amend_options_generic_luks(bs, amend_options,
-                                                  force, errp);
+                                                  true, force, errp);
 cleanup:
     qapi_free_QCryptoBlockAmendOptions(amend_options);
     return ret;
@@ -853,7 +859,7 @@ coroutine_fn block_crypto_co_amend_luks(BlockDriverState *bs,
         .u.luks = *qapi_BlockdevAmendOptionsLUKS_base(&opts->u.luks),
     };
     return block_crypto_amend_options_generic_luks(bs, &amend_opts,
-                                                   force, errp);
+                                                   false, force, errp);
 }
 
 static void
-- 
2.27.0



  parent reply	other threads:[~2021-11-24  7:26 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-24  6:43 [PATCH v5 00/31] block layer: split block APIs in global state and I/O Emanuele Giuseppe Esposito
2021-11-24  6:43 ` [PATCH v5 01/31] main-loop.h: introduce qemu_in_main_thread() Emanuele Giuseppe Esposito
2021-11-24  6:43 ` [PATCH v5 02/31] include/block/block: split header into I/O and global state API Emanuele Giuseppe Esposito
2021-11-24  6:43 ` [PATCH v5 03/31] assertions for block " Emanuele Giuseppe Esposito
2021-12-16 15:17   ` Hanna Reitz
2021-11-24  6:43 ` [PATCH v5 04/31] include/sysemu/block-backend: split header into I/O and global state (GS) API Emanuele Giuseppe Esposito
2021-12-10 14:21   ` Hanna Reitz
2021-11-24  6:43 ` [PATCH v5 05/31] block-backend: special comments for blk_set/get_perm due to fuse Emanuele Giuseppe Esposito
2021-12-10 14:38   ` Hanna Reitz
2021-12-15  8:57     ` Emanuele Giuseppe Esposito
2021-12-15 10:13       ` Emanuele Giuseppe Esposito
2021-12-16 14:00         ` Hanna Reitz
2021-11-24  6:43 ` [PATCH v5 06/31] block/block-backend.c: assertions for block-backend Emanuele Giuseppe Esposito
2021-12-10 15:37   ` Hanna Reitz
2021-11-24  6:43 ` [PATCH v5 07/31] include/block/block_int: split header into I/O and global state API Emanuele Giuseppe Esposito
2021-11-24  6:43 ` [PATCH v5 08/31] assertions for block_int " Emanuele Giuseppe Esposito
2021-11-24  6:43 ` [PATCH v5 09/31] block: introduce assert_bdrv_graph_writable Emanuele Giuseppe Esposito
2021-12-10 17:43   ` Hanna Reitz
2021-12-14 19:48     ` Emanuele Giuseppe Esposito
2021-12-16 14:01       ` Hanna Reitz
2021-11-24  6:43 ` [PATCH v5 10/31] block.c: modify .attach and .detach callbacks of child_of_bds Emanuele Giuseppe Esposito
2021-12-16 14:57   ` Hanna Reitz
2021-12-16 16:05     ` Emanuele Giuseppe Esposito
2021-12-16 16:12       ` Hanna Reitz
2021-11-24  6:43 ` [PATCH v5 11/31] include/block/blockjob_int.h: split header into I/O and GS API Emanuele Giuseppe Esposito
2021-11-24  6:43 ` [PATCH v5 12/31] assertions for blockjob_int.h Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 13/31] block.c: add assertions to static functions Emanuele Giuseppe Esposito
2021-12-16 16:08   ` Hanna Reitz
2021-12-16 16:39     ` Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 14/31] include/block/blockjob.h: global state API Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 15/31] assertions for blockjob.h " Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 16/31] include/sysemu/blockdev.h: " Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 17/31] assertions for blockdev.h " Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 18/31] include/block/snapshot: global state API + assertions Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 19/31] block/copy-before-write.h: " Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 20/31] block/coroutines: I/O API Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 21/31] block_int-common.h: split function pointers in BlockDriver Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 22/31] block_int-common.h: assertion in the callers of BlockDriver function pointers Emanuele Giuseppe Esposito
2021-12-16 18:43   ` Hanna Reitz
2021-12-17 15:53     ` Emanuele Giuseppe Esposito
2021-12-17 16:00       ` Hanna Reitz
2021-11-24  6:44 ` [PATCH v5 23/31] block_int-common.h: split function pointers in BdrvChildClass Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 24/31] block_int-common.h: assertions in the callers of BdrvChildClass function pointers Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 25/31] block-backend-common.h: split function pointers in BlockDevOps Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 26/31] job.h: split function pointers in JobDriver Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 27/31] job.h: assertions in the callers of JobDriver funcion pointers Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 28/31] block.c: assert BQL lock held in bdrv_co_invalidate_cache Emanuele Giuseppe Esposito
2021-12-17 11:04   ` Hanna Reitz
2021-12-17 16:38     ` Emanuele Giuseppe Esposito
2021-12-20 12:20       ` Emanuele Giuseppe Esposito
2021-12-23 17:11         ` Hanna Reitz
2022-01-19 15:57           ` Hanna Reitz
2022-01-19 17:44             ` Hanna Reitz
2022-01-19 18:34         ` Kevin Wolf
2022-01-20 13:22           ` Paolo Bonzini
2022-01-20 13:48             ` Kevin Wolf
2022-01-21 10:22               ` Emanuele Giuseppe Esposito
2021-11-24  6:44 ` [PATCH v5 29/31] jobs: introduce pre_run function in JobDriver Emanuele Giuseppe Esposito
2021-11-24  6:44 ` Emanuele Giuseppe Esposito [this message]
2021-12-17 12:29   ` [PATCH v5 30/31] crypto: delegate permission functions to JobDriver .pre_run Hanna Reitz
2021-12-17 14:32     ` Hanna Reitz
2021-12-20 15:47     ` Emanuele Giuseppe Esposito
2021-12-23 17:15       ` Hanna Reitz
2021-11-24  6:44 ` [PATCH v5 31/31] block.c: assertions to the block layer permissions API Emanuele Giuseppe Esposito
2021-11-29 13:32 ` [PATCH v5 00/31] block layer: split block APIs in global state and I/O Stefan Hajnoczi
2021-11-30  8:39   ` Emanuele Giuseppe Esposito

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211124064418.3120601-31-eesposit@redhat.com \
    --to=eesposit@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=fam@euphon.net \
    --cc=hreitz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=richard.henderson@linaro.org \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).