mirror of
https://gitlab.com/qemu-project/qemu
synced 2024-09-06 18:05:37 +00:00
block: drop BLK_PERM_GRAPH_MOD
First, this permission never protected a node from being changed, as generic child-replacing functions don't check it. Second, it's a strange thing: it presents a permission of parent node to change its child. But generally, children are replaced by different mechanisms, like jobs or qmp commands, not by nodes. Graph-mod permission is hard to understand. All other permissions describe operations which done by parent node on its child: read, write, resize. Graph modification operations are something completely different. The only place where BLK_PERM_GRAPH_MOD is used as "perm" (not shared perm) is mirror_start_job, for s->target. Still modern code should use bdrv_freeze_backing_chain() to protect from graph modification, if we don't do it somewhere it may be considered as a bug. So, it's a bit risky to drop GRAPH_MOD, and analyzing of possible loss of protection is hard. But one day we should do it, let's do it now. One more bit of information is that locking the corresponding byte in file-posix doesn't make sense at all. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20210902093754.2352-1-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
parent
96054c76ff
commit
64631f3681
7
block.c
7
block.c
|
@ -2485,7 +2485,6 @@ char *bdrv_perm_names(uint64_t perm)
|
|||
{ BLK_PERM_WRITE, "write" },
|
||||
{ BLK_PERM_WRITE_UNCHANGED, "write unchanged" },
|
||||
{ BLK_PERM_RESIZE, "resize" },
|
||||
{ BLK_PERM_GRAPH_MOD, "change children" },
|
||||
{ 0, NULL }
|
||||
};
|
||||
|
||||
|
@ -2601,8 +2600,7 @@ static void bdrv_default_perms_for_cow(BlockDriverState *bs, BdrvChild *c,
|
|||
shared = 0;
|
||||
}
|
||||
|
||||
shared |= BLK_PERM_CONSISTENT_READ | BLK_PERM_GRAPH_MOD |
|
||||
BLK_PERM_WRITE_UNCHANGED;
|
||||
shared |= BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
|
||||
|
||||
if (bs->open_flags & BDRV_O_INACTIVE) {
|
||||
shared |= BLK_PERM_WRITE | BLK_PERM_RESIZE;
|
||||
|
@ -2720,7 +2718,6 @@ uint64_t bdrv_qapi_perm_to_blk_perm(BlockPermission qapi_perm)
|
|||
[BLOCK_PERMISSION_WRITE] = BLK_PERM_WRITE,
|
||||
[BLOCK_PERMISSION_WRITE_UNCHANGED] = BLK_PERM_WRITE_UNCHANGED,
|
||||
[BLOCK_PERMISSION_RESIZE] = BLK_PERM_RESIZE,
|
||||
[BLOCK_PERMISSION_GRAPH_MOD] = BLK_PERM_GRAPH_MOD,
|
||||
};
|
||||
|
||||
QEMU_BUILD_BUG_ON(ARRAY_SIZE(permissions) != BLOCK_PERMISSION__MAX);
|
||||
|
@ -5546,8 +5543,6 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
|
|||
update_inherits_from = bdrv_inherits_from_recursive(base, explicit_top);
|
||||
|
||||
/* success - we can delete the intermediate states, and link top->base */
|
||||
/* TODO Check graph modification op blockers (BLK_PERM_GRAPH_MOD) once
|
||||
* we've figured out how they should work. */
|
||||
if (!backing_file_str) {
|
||||
bdrv_refresh_filename(base);
|
||||
backing_file_str = base->filename;
|
||||
|
|
|
@ -370,7 +370,6 @@ void commit_start(const char *job_id, BlockDriverState *bs,
|
|||
s->base = blk_new(s->common.job.aio_context,
|
||||
base_perms,
|
||||
BLK_PERM_CONSISTENT_READ
|
||||
| BLK_PERM_GRAPH_MOD
|
||||
| BLK_PERM_WRITE_UNCHANGED);
|
||||
ret = blk_insert_bs(s->base, base, errp);
|
||||
if (ret < 0) {
|
||||
|
|
|
@ -1139,10 +1139,7 @@ static void mirror_complete(Job *job, Error **errp)
|
|||
replace_aio_context = bdrv_get_aio_context(s->to_replace);
|
||||
aio_context_acquire(replace_aio_context);
|
||||
|
||||
/* TODO Translate this into permission system. Current definition of
|
||||
* GRAPH_MOD would require to request it for the parents; they might
|
||||
* not even be BlockDriverStates, however, so a BdrvChild can't address
|
||||
* them. May need redefinition of GRAPH_MOD. */
|
||||
/* TODO Translate this into child freeze system. */
|
||||
error_setg(&s->replace_blocker,
|
||||
"block device is in use by block-job-complete");
|
||||
bdrv_op_block_all(s->to_replace, s->replace_blocker);
|
||||
|
@ -1666,7 +1663,7 @@ static BlockJob *mirror_start_job(
|
|||
s = block_job_create(job_id, driver, NULL, mirror_top_bs,
|
||||
BLK_PERM_CONSISTENT_READ,
|
||||
BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
|
||||
BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD, speed,
|
||||
BLK_PERM_WRITE, speed,
|
||||
creation_flags, cb, opaque, errp);
|
||||
if (!s) {
|
||||
goto fail;
|
||||
|
@ -1710,9 +1707,7 @@ static BlockJob *mirror_start_job(
|
|||
target_perms |= BLK_PERM_RESIZE;
|
||||
}
|
||||
|
||||
target_shared_perms |= BLK_PERM_CONSISTENT_READ
|
||||
| BLK_PERM_WRITE
|
||||
| BLK_PERM_GRAPH_MOD;
|
||||
target_shared_perms |= BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE;
|
||||
} else if (bdrv_chain_contains(bs, bdrv_skip_filters(target))) {
|
||||
/*
|
||||
* We may want to allow this in the future, but it would
|
||||
|
@ -1723,10 +1718,6 @@ static BlockJob *mirror_start_job(
|
|||
goto fail;
|
||||
}
|
||||
|
||||
if (backing_mode != MIRROR_LEAVE_BACKING_CHAIN) {
|
||||
target_perms |= BLK_PERM_GRAPH_MOD;
|
||||
}
|
||||
|
||||
s->target = blk_new(s->common.job.aio_context,
|
||||
target_perms, target_shared_perms);
|
||||
ret = blk_insert_bs(s->target, target, errp);
|
||||
|
|
|
@ -171,8 +171,7 @@ bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,
|
|||
perm |= BLK_PERM_WRITE;
|
||||
}
|
||||
|
||||
shared_perm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
|
||||
BLK_PERM_GRAPH_MOD;
|
||||
shared_perm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
|
||||
if (resizable) {
|
||||
shared_perm |= BLK_PERM_RESIZE;
|
||||
}
|
||||
|
|
|
@ -269,12 +269,13 @@ enum {
|
|||
BLK_PERM_RESIZE = 0x08,
|
||||
|
||||
/**
|
||||
* This permission is required to change the node that this BdrvChild
|
||||
* points to.
|
||||
* There was a now-removed bit BLK_PERM_GRAPH_MOD, with value of 0x10. QEMU
|
||||
* 6.1 and earlier may still lock the corresponding byte in block/file-posix
|
||||
* locking. So, implementing some new permission should be very careful to
|
||||
* not interfere with this old unused thing.
|
||||
*/
|
||||
BLK_PERM_GRAPH_MOD = 0x10,
|
||||
|
||||
BLK_PERM_ALL = 0x1f,
|
||||
BLK_PERM_ALL = 0x0f,
|
||||
|
||||
DEFAULT_PERM_PASSTHROUGH = BLK_PERM_CONSISTENT_READ
|
||||
| BLK_PERM_WRITE
|
||||
|
|
|
@ -1878,14 +1878,11 @@
|
|||
#
|
||||
# @resize: This permission is required to change the size of a block node.
|
||||
#
|
||||
# @graph-mod: This permission is required to change the node that this
|
||||
# BdrvChild points to.
|
||||
#
|
||||
# Since: 4.0
|
||||
##
|
||||
{ 'enum': 'BlockPermission',
|
||||
'data': [ 'consistent-read', 'write', 'write-unchanged', 'resize',
|
||||
'graph-mod' ] }
|
||||
'data': [ 'consistent-read', 'write', 'write-unchanged', 'resize' ] }
|
||||
|
||||
##
|
||||
# @XDbgBlockGraphEdge:
|
||||
#
|
||||
|
|
|
@ -35,7 +35,6 @@ def perm(arr):
|
|||
s = 'w' if 'write' in arr else '_'
|
||||
s += 'r' if 'consistent-read' in arr else '_'
|
||||
s += 'u' if 'write-unchanged' in arr else '_'
|
||||
s += 'g' if 'graph-mod' in arr else '_'
|
||||
s += 's' if 'resize' in arr else '_'
|
||||
return s
|
||||
|
||||
|
|
|
@ -204,7 +204,6 @@ Testing: -blockdev file,node-name=base,filename=TEST_DIR/t.IMGFMT.base -blockdev
|
|||
"name": "file",
|
||||
"parent": 5,
|
||||
"shared-perm": [
|
||||
"graph-mod",
|
||||
"write-unchanged",
|
||||
"consistent-read"
|
||||
],
|
||||
|
@ -219,7 +218,6 @@ Testing: -blockdev file,node-name=base,filename=TEST_DIR/t.IMGFMT.base -blockdev
|
|||
"name": "backing",
|
||||
"parent": 5,
|
||||
"shared-perm": [
|
||||
"graph-mod",
|
||||
"resize",
|
||||
"write-unchanged",
|
||||
"write",
|
||||
|
@ -233,7 +231,6 @@ Testing: -blockdev file,node-name=base,filename=TEST_DIR/t.IMGFMT.base -blockdev
|
|||
"name": "file",
|
||||
"parent": 3,
|
||||
"shared-perm": [
|
||||
"graph-mod",
|
||||
"write-unchanged",
|
||||
"consistent-read"
|
||||
],
|
||||
|
@ -246,7 +243,6 @@ Testing: -blockdev file,node-name=base,filename=TEST_DIR/t.IMGFMT.base -blockdev
|
|||
"name": "backing",
|
||||
"parent": 3,
|
||||
"shared-perm": [
|
||||
"graph-mod",
|
||||
"resize",
|
||||
"write-unchanged",
|
||||
"write",
|
||||
|
|
Loading…
Reference in a new issue