Message ID | 20191108123455.39445-2-mreitz@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | iotests: Test failing mirror complete | expand |
08.11.2019 15:34, Max Reitz wrote: > We need some way to correlate QAPI BlockPermission values with > BLK_PERM_* flags. We could: > > (1) have the same order in the QAPI definition as the the BLK_PERM_* > flags are in LSb-first order. However, then there is no guarantee > that they actually match (e.g. when someone modifies the QAPI schema > without thinking of the BLK_PERM_* definitions). > We could add static assertions, but these would break what’s good > about this solution, namely its simplicity. > > (2) define the BLK_PERM_* flags based on the BlockPermission values. > But this way whenever someone were to modify the QAPI order > (perfectly sensible in theory), the BLK_PERM_* values would change. > Because these values are used for file locking, this might break > file locking between different qemu versions. > > Therefore, go the slightly more cumbersome way: Add a function to > translate from the QAPI constants to the BLK_PERM_* flags. > > Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block.c | 18 ++++++++++++++++++ > include/block/block.h | 1 + > 2 files changed, 19 insertions(+) > > diff --git a/block.c b/block.c > index 4cffc2bc35..066433f3e2 100644 > --- a/block.c > +++ b/block.c > @@ -2227,6 +2227,24 @@ void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c, > *nshared = shared; > } > > +uint64_t bdrv_qapi_perm_to_blk_perm(BlockPermission qapi_perm) > +{ > + static const uint64_t permissions[] = { > + [BLOCK_PERMISSION_CONSISTENT_READ] = BLK_PERM_CONSISTENT_READ, > + [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); > + QEMU_BUILD_BUG_ON(1UL << ARRAY_SIZE(permissions) != BLK_PERM_ALL + 1); safe enough) > + > + assert(qapi_perm < BLOCK_PERMISSION__MAX); > + > + return permissions[qapi_perm]; > +} > + > static void bdrv_replace_child_noperm(BdrvChild *child, > BlockDriverState *new_bs) > { > diff --git a/include/block/block.h b/include/block/block.h > index 1df9848e74..e9dcfef7fa 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -280,6 +280,7 @@ enum { > }; > > char *bdrv_perm_names(uint64_t perm); > +uint64_t bdrv_qapi_perm_to_blk_perm(BlockPermission qapi_perm); > > /* disk I/O throttling */ > void bdrv_init(void); >
diff --git a/block.c b/block.c index 4cffc2bc35..066433f3e2 100644 --- a/block.c +++ b/block.c @@ -2227,6 +2227,24 @@ void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c, *nshared = shared; } +uint64_t bdrv_qapi_perm_to_blk_perm(BlockPermission qapi_perm) +{ + static const uint64_t permissions[] = { + [BLOCK_PERMISSION_CONSISTENT_READ] = BLK_PERM_CONSISTENT_READ, + [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); + QEMU_BUILD_BUG_ON(1UL << ARRAY_SIZE(permissions) != BLK_PERM_ALL + 1); + + assert(qapi_perm < BLOCK_PERMISSION__MAX); + + return permissions[qapi_perm]; +} + static void bdrv_replace_child_noperm(BdrvChild *child, BlockDriverState *new_bs) { diff --git a/include/block/block.h b/include/block/block.h index 1df9848e74..e9dcfef7fa 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -280,6 +280,7 @@ enum { }; char *bdrv_perm_names(uint64_t perm); +uint64_t bdrv_qapi_perm_to_blk_perm(BlockPermission qapi_perm); /* disk I/O throttling */ void bdrv_init(void);
We need some way to correlate QAPI BlockPermission values with BLK_PERM_* flags. We could: (1) have the same order in the QAPI definition as the the BLK_PERM_* flags are in LSb-first order. However, then there is no guarantee that they actually match (e.g. when someone modifies the QAPI schema without thinking of the BLK_PERM_* definitions). We could add static assertions, but these would break what’s good about this solution, namely its simplicity. (2) define the BLK_PERM_* flags based on the BlockPermission values. But this way whenever someone were to modify the QAPI order (perfectly sensible in theory), the BLK_PERM_* values would change. Because these values are used for file locking, this might break file locking between different qemu versions. Therefore, go the slightly more cumbersome way: Add a function to translate from the QAPI constants to the BLK_PERM_* flags. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block.c | 18 ++++++++++++++++++ include/block/block.h | 1 + 2 files changed, 19 insertions(+)