From: Stefan Hajnoczi <stefanha@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>,
qemu-block@nongnu.org, afrosi@redhat.com,
"Max Reitz" <mreitz@redhat.com>,
"Stefan Hajnoczi" <stefanha@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: [PATCH] raw-format: drop WRITE and RESIZE child perms when possible
Date: Mon, 26 Jul 2021 13:28:39 +0100 [thread overview]
Message-ID: <20210726122839.822900-1-stefanha@redhat.com> (raw)
The following command-line fails due to a permissions conflict:
$ qemu-storage-daemon \
--blockdev driver=nvme,node-name=nvme0,device=0000:08:00.0,namespace=1 \
--blockdev driver=raw,node-name=l1-1,file=nvme0,offset=0,size=1073741824 \
--blockdev driver=raw,node-name=l1-2,file=nvme0,offset=1073741824,size=1073741824 \
--nbd-server addr.type=unix,addr.path=/tmp/nbd.sock,max-connections=2 \
--export type=nbd,id=nbd-l1-1,node-name=l1-1,name=l1-1,writable=on \
--export type=nbd,id=nbd-l1-2,node-name=l1-2,name=l1-2,writable=on
qemu-storage-daemon: --export type=nbd,id=nbd-l1-1,node-name=l1-1,name=l1-1,writable=on: Permission conflict on node 'nvme0': permissions 'resize' are both required by node 'l1-1' (uses node 'nvme0' as 'file' child) and unshared by node 'l1-2' (uses node 'nvme0' as 'file' child).
The problem is that block/raw-format.c relies on bdrv_default_perms() to
set permissions on the nvme node. The default permissions add RESIZE in
anticipation of a format driver like qcow2 that needs to grow the image
file. This fails because RESIZE is unshared, so we cannot get the RESIZE
permission.
Max Reitz pointed out that block/crypto.c already handles this case by
implementing a custom ->bdrv_child_perm() function that adjusts the
result of bdrv_default_perms().
This patch takes the same approach in block/raw-format.c so that RESIZE
is only required if it's actually necessary (e.g. the parent is qcow2).
Cc: Max Reitz <mreitz@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
This is not a bug fix, so I didn't mark it for QEMU 6.1. It's new
behavior that hasn't been supported before. I want to split an NVMe
drive using the raw format's offset=/size= feature.
---
block/raw-format.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/block/raw-format.c b/block/raw-format.c
index 7717578ed6..c26f493688 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -580,6 +580,25 @@ static void raw_cancel_in_flight(BlockDriverState *bs)
bdrv_cancel_in_flight(bs->file->bs);
}
+static void raw_child_perm(BlockDriverState *bs, BdrvChild *c,
+ BdrvChildRole role,
+ BlockReopenQueue *reopen_queue,
+ uint64_t parent_perm, uint64_t parent_shared,
+ uint64_t *nperm, uint64_t *nshared)
+{
+ bdrv_default_perms(bs, c, role, reopen_queue, parent_perm,
+ parent_shared, nperm, nshared);
+
+ /*
+ * bdrv_default_perms() may add WRITE and/or RESIZE (see comment in
+ * bdrv_default_perms_for_storage() for an explanation) but we only need
+ * them if they are in parent_perm. Drop WRITE and RESIZE whenever possible
+ * to avoid permission conflicts.
+ */
+ *nperm &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
+ *nperm |= parent_perm & (BLK_PERM_WRITE | BLK_PERM_RESIZE);
+}
+
BlockDriver bdrv_raw = {
.format_name = "raw",
.instance_size = sizeof(BDRVRawState),
@@ -588,7 +607,7 @@ BlockDriver bdrv_raw = {
.bdrv_reopen_commit = &raw_reopen_commit,
.bdrv_reopen_abort = &raw_reopen_abort,
.bdrv_open = &raw_open,
- .bdrv_child_perm = bdrv_default_perms,
+ .bdrv_child_perm = raw_child_perm,
.bdrv_co_create_opts = &raw_co_create_opts,
.bdrv_co_preadv = &raw_co_preadv,
.bdrv_co_pwritev = &raw_co_pwritev,
--
2.31.1
next reply other threads:[~2021-07-26 12:29 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-26 12:28 Stefan Hajnoczi [this message]
2021-07-26 14:41 ` [PATCH] raw-format: drop WRITE and RESIZE child perms when possible Vladimir Sementsov-Ogievskiy
2021-07-26 15:42 ` Kevin Wolf
2021-07-26 15:59 ` Stefan Hajnoczi
2021-08-19 13:37 ` Hanna Reitz
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=20210726122839.822900-1-stefanha@redhat.com \
--to=stefanha@redhat.com \
--cc=afrosi@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=philmd@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/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).