Now copy-before-write filter has weak permission model: when it has no
parents, it share write permission on source. Otherwise we just can't
blockdev-add it, when existing user of source has write permission.
The situation is bad, it means that copy-before-write filter doesn't
guarantee that all write goes through it. And a lot better is unshare
write always. But how to insert the filter in this case?
The solution is to do blockdev-add and blockdev-replace in one
transaction, and more, update permissions only after both command.
For now, let's create a possibility to not update permission on file
child of copy-before-write filter at time of open.
New interfaces are:
- bds_tree_init() with flags argument, so that caller may pass
additional flags, for example the new BDRV_O_NOPERM.
- bdrv_open_file_child_common() with boolean refresh_perms arguments.
Drivers may use this function with refresh_perms = true, if they want
to satisfy BDRV_O_NOPERM. No one such driver for now.
Signed-off-by: Vladimir Sementsov-Ogievskiy <v.sementsov...@mail.ru>
---
block.c | 84 ++++++++++++++++++++++++----------
block/monitor/block-hmp-cmds.c | 2 +-
blockdev.c | 13 +++---
include/block/block.h | 14 ++++++
include/block/block_int.h | 5 +-
5 files changed, 85 insertions(+), 33 deletions(-)
diff --git a/block.c b/block.c
index 7c22b31259..a3bc28cf32 100644
--- a/block.c
+++ b/block.c
@@ -3065,12 +3065,13 @@ out:
* If @parent_bs and @child_bs are in different AioContexts, the caller must
* hold the AioContext lock for @child_bs, but not for @parent_bs.
*/
-BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
- BlockDriverState *child_bs,
- const char *child_name,
- const BdrvChildClass *child_class,
- BdrvChildRole child_role,
- Error **errp)
+static BdrvChild *bdrv_do_attach_child(BlockDriverState *parent_bs,
+ BlockDriverState *child_bs,
+ const char *child_name,
+ const BdrvChildClass *child_class,
+ BdrvChildRole child_role,
+ bool refresh_perms,
+ Error **errp)
{
int ret;
BdrvChild *child;
@@ -3082,9 +3083,11 @@ BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
goto out;
}
- ret = bdrv_refresh_perms(parent_bs, tran, errp);
- if (ret < 0) {
- goto out;
+ if (refresh_perms) {
+ ret = bdrv_refresh_perms(parent_bs, tran, errp);
+ if (ret < 0) {
+ goto out;
+ }
}
out:
@@ -3095,6 +3098,17 @@ out:
return ret < 0 ? NULL : child;
}
+BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
+ BlockDriverState *child_bs,
+ const char *child_name,
+ const BdrvChildClass *child_class,
+ BdrvChildRole child_role,
+ Error **errp)
+{
+ return bdrv_do_attach_child(parent_bs, child_bs, child_name, child_class,
+ child_role, true, errp);
+}
+
/* Caller is responsible to refresh permissions in @refresh_list */
static void bdrv_root_unref_child_tran(BdrvChild *child, GSList **refresh_list,
Transaction *tran)
@@ -3556,12 +3570,13 @@ done:
*
* The BlockdevRef will be removed from the options QDict.
*/
-BdrvChild *bdrv_open_child(const char *filename,
- QDict *options, const char *bdref_key,
- BlockDriverState *parent,
- const BdrvChildClass *child_class,
- BdrvChildRole child_role,
- bool allow_none, Error **errp)
+BdrvChild *bdrv_open_child_common(const char *filename,
+ QDict *options, const char *bdref_key,
+ BlockDriverState *parent,
+ const BdrvChildClass *child_class,
+ BdrvChildRole child_role,
+ bool allow_none, bool refresh_perms,
+ Error **errp)
{
BlockDriverState *bs;
@@ -3571,16 +3586,29 @@ BdrvChild *bdrv_open_child(const char *filename,
return NULL;
}
- return bdrv_attach_child(parent, bs, bdref_key, child_class, child_role,
- errp);
+ return bdrv_do_attach_child(parent, bs, bdref_key, child_class, child_role,
+ refresh_perms, errp);
+}
+
+BdrvChild *bdrv_open_child(const char *filename,
+ QDict *options, const char *bdref_key,
+ BlockDriverState *parent,
+ const BdrvChildClass *child_class,
+ BdrvChildRole child_role,
+ bool allow_none, Error **errp)
+{
+ return bdrv_open_child_common(filename, options, bdref_key, parent,
+ child_class, child_role, allow_none, true,
+ errp);
}
/*
* Wrapper on bdrv_open_child() for most popular case: open primary child of
bs.
*/
-int bdrv_open_file_child(const char *filename,
- QDict *options, const char *bdref_key,
- BlockDriverState *parent, Error **errp)
+int bdrv_open_file_child_common(const char *filename,
+ QDict *options, const char *bdref_key,
+ BlockDriverState *parent, bool refresh_perms,
+ Error **errp)
{
BdrvChildRole role;
@@ -3589,8 +3617,9 @@ int bdrv_open_file_child(const char *filename,
role = parent->drv->is_filter ?
(BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY) : BDRV_CHILD_IMAGE;
- if (!bdrv_open_child(filename, options, bdref_key, parent,
- &child_of_bds, role, false, errp))
+ if (!bdrv_open_child_common(filename, options, bdref_key, parent,
+ &child_of_bds, role, false, refresh_perms,
+ errp))
{
return -EINVAL;
}
@@ -3598,6 +3627,15 @@ int bdrv_open_file_child(const char *filename,
return 0;
}
+int bdrv_open_file_child(const char *filename,
+ QDict *options, const char *bdref_key,
+ BlockDriverState *parent,
+ Error **errp)
+{
+ return bdrv_open_file_child_common(filename, options, bdref_key, parent,
+ true, errp);
+}
+
/*
* TODO Future callers may need to specify parent/child_class in order for
* option inheritance to work. Existing callers use it for the root node.
@@ -6647,7 +6685,7 @@ void bdrv_unref_tran(BlockDriverState *bs, GSList
**refresh_list,
tran_add(tran, &bdrv_unref_drv, bs);
- if (bs->drv && (!bs->drv->bdrv_close || bs->drv->indepenent_close) &&
+ if (bs->drv && (!bs->drv->bdrv_close || bs->drv->independent_close) &&
refresh_list && bs->refcnt == 0)
{
QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index bfb3c043a0..9145ccfc46 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -76,7 +76,7 @@ static void hmp_drive_add_node(Monitor *mon, const char
*optstr)
goto out;
}
- BlockDriverState *bs = bds_tree_init(qdict, &local_err);
+ BlockDriverState *bs = bds_tree_init(qdict, 0, &local_err);
if (!bs) {
error_report_err(local_err);
goto out;
diff --git a/blockdev.c b/blockdev.c
index 517be48399..3569b0e6ee 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -624,11 +624,10 @@ err_no_opts:
}
/* Takes the ownership of bs_opts */
-BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp)
+BlockDriverState *bds_tree_init(QDict *bs_opts, BdrvRequestFlags flags,
+ Error **errp)
{
- int bdrv_flags = 0;
-
- /* bdrv_open() defaults to the values in bdrv_flags (for compatibility
+ /* bdrv_open() defaults to the values in flags (for compatibility
* with other callers) rather than what we want as the real defaults.
* Apply the defaults here instead. */
qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_DIRECT, "off");
@@ -636,10 +635,10 @@ BlockDriverState *bds_tree_init(QDict *bs_opts, Error
**errp)
qdict_set_default_str(bs_opts, BDRV_OPT_READ_ONLY, "off");
if (runstate_check(RUN_STATE_INMIGRATE)) {
- bdrv_flags |= BDRV_O_INACTIVE;
+ flags |= BDRV_O_INACTIVE;
}
- return bdrv_open(NULL, NULL, bs_opts, bdrv_flags, errp);
+ return bdrv_open(NULL, NULL, bs_opts, flags, errp);
}
void blockdev_close_all_bdrv_states(void)
@@ -3449,7 +3448,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error
**errp)
goto fail;
}
- bs = bds_tree_init(qdict, errp);
+ bs = bds_tree_init(qdict, 0, errp);
if (!bs) {
goto fail;
}
diff --git a/include/block/block.h b/include/block/block.h
index 92fe31bd13..017bf9b7c0 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -123,6 +123,9 @@ typedef struct HDGeometry {
#define BDRV_O_AUTO_RDONLY 0x20000 /* degrade to read-only if opening
read-write fails */
#define BDRV_O_IO_URING 0x40000 /* use io_uring instead of the thread pool
*/
+#define BDRV_O_NOPERM 0x80000 /* Don't update permissions if possible,
+ open() caller will do that. */
+
#define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_NO_FLUSH)
@@ -416,6 +419,17 @@ BdrvChild *bdrv_open_child(const char *filename,
const BdrvChildClass *child_class,
BdrvChildRole child_role,
bool allow_none, Error **errp);
+BdrvChild *bdrv_open_child_common(const char *filename,
+ QDict *options, const char *bdref_key,
+ BlockDriverState *parent,
+ const BdrvChildClass *child_class,
+ BdrvChildRole child_role,
+ bool allow_none, bool refresh_perms,
+ Error **errp);
+int bdrv_open_file_child_common(const char *filename,
+ QDict *options, const char *bdref_key,
+ BlockDriverState *parent, bool refresh_perms,
+ Error **errp);
int bdrv_open_file_child(const char *filename,
QDict *options, const char *bdref_key,
BlockDriverState *parent, Error **errp);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index e2bb936451..f6deb89f23 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -178,7 +178,7 @@ struct BlockDriver {
* and is safe to be called in commit phase of block-graph modifying
* transaction.
*/
- bool indepenent_close;
+ bool independent_close;
/* For handling image reopen for split or non-split files */
int (*bdrv_reopen_prepare)(BDRVReopenState *reopen_state,
@@ -1436,7 +1436,8 @@ int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src,
int64_t src_offset,
int refresh_total_sectors(BlockDriverState *bs, int64_t hint);
void bdrv_set_monitor_owned(BlockDriverState *bs);
-BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp);
+BlockDriverState *bds_tree_init(QDict *bs_opts, BdrvRequestFlags flags,
+ Error **errp);
/**
* Simple implementation of bdrv_co_create_opts for protocol drivers
--
2.35.1
