[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [RFC PATCH 15/41] block: Add permissions to blk_new()
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [RFC PATCH 15/41] block: Add permissions to blk_new() |
Date: |
Mon, 20 Feb 2017 11:29:15 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
On 13.02.2017 18:22, Kevin Wolf wrote:
> We want every user to be specific about the permissions it needs, so
> we'll pass the initial permissions as parameters to blk_new(). A user
> only needs to call blk_set_perm() if it wants to change the permissions
> after the fact.
>
> The permissions are stored in the BlockBackend and applied whenever a
> BlockDriverState should be attached in blk_insert_bs().
>
> This does not include actually choosing the right set of permissions
> yet. Instead, the usual FIXME comment is added to each place and will be
> addressed in individual patches.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> block/backup.c | 3 ++-
> block/block-backend.c | 12 ++++++------
> block/commit.c | 12 ++++++++----
> block/mirror.c | 3 ++-
> blockdev.c | 2 +-
> blockjob.c | 3 ++-
> hmp.c | 3 ++-
> hw/block/fdc.c | 3 ++-
> hw/core/qdev-properties-system.c | 3 ++-
> hw/ide/qdev.c | 3 ++-
> hw/scsi/scsi-disk.c | 3 ++-
> include/sysemu/block-backend.h | 2 +-
> migration/block.c | 3 ++-
> nbd/server.c | 3 ++-
> tests/test-blockjob.c | 3 ++-
> tests/test-throttle.c | 7 ++++---
> 16 files changed, 42 insertions(+), 26 deletions(-)
[...]
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 8f0348d..a219f0b 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -123,14 +123,14 @@ static const BdrvChildRole child_root = {
> * Store an error through @errp on failure, unless it's null.
> * Return the new BlockBackend on success, null on failure.
> */
> -BlockBackend *blk_new(void)
> +BlockBackend *blk_new(uint64_t perm, uint64_t shared_perm)
I think it would be a good idea to document what these parameters do, at
least by pointing to the definition of the permission flags.
Also, this makes me think that the permission flags should not be in
block_int.h but in block.h. This function is available outside of the
core block layer.
> {
> BlockBackend *blk;
>
> blk = g_new0(BlockBackend, 1);
> blk->refcnt = 1;
> - blk->perm = 0;
> - blk->shared_perm = BLK_PERM_ALL;
> + blk->perm = perm;
> + blk->shared_perm = shared_perm;
> blk_set_enable_write_cache(blk, true);
>
> qemu_co_queue_init(&blk->public.throttled_reqs[0]);
> @@ -161,7 +161,7 @@ BlockBackend *blk_new_open(const char *filename, const
> char *reference,
> BlockBackend *blk;
> BlockDriverState *bs;
>
> - blk = blk_new();
> + blk = blk_new(0, BLK_PERM_ALL);
> bs = bdrv_open(filename, reference, options, flags, errp);
> if (!bs) {
> blk_unref(blk);
> @@ -505,9 +505,9 @@ void blk_remove_bs(BlockBackend *blk)
> void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
> {
> bdrv_ref(bs);
> - /* FIXME Use real permissions */
> blk->root = bdrv_root_attach_child(bs, "root", &child_root,
> - 0, BLK_PERM_ALL, blk, &error_abort);
> + blk->perm, blk->shared_perm, blk,
> + &error_abort);
And the error_abort does not qualify as a FIXME?
>
> notifier_list_notify(&blk->insert_bs_notifiers, blk);
> if (blk->public.throttle_state) {
[...]
> diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
> index 068c9e4..1dd1cfa 100644
> --- a/tests/test-blockjob.c
> +++ b/tests/test-blockjob.c
> @@ -53,7 +53,8 @@ static BlockJob *do_test_id(BlockBackend *blk, const char
> *id,
> * BlockDriverState inserted. */
> static BlockBackend *create_blk(const char *name)
> {
> - BlockBackend *blk = blk_new();
> + /* FIXME Use real permissions */
> + BlockBackend *blk = blk_new(0, BLK_PERM_ALL);
> BlockDriverState *bs;
>
> bs = bdrv_open("null-co://", NULL, NULL, 0, &error_abort);
Pretty much independent of this patch, but: blk_new_open() would
probably be the better choice then to call blk_new() + bdrv_open() +
blk_insert_bs().
Max
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-block] [RFC PATCH 10/41] block: Request child permissions in format drivers, (continued)
- [Qemu-block] [RFC PATCH 11/41] vvfat: Implement .bdrv_child_perm(), Kevin Wolf, 2017/02/13
- [Qemu-block] [RFC PATCH 12/41] block: Require .bdrv_child_perm() with child nodes, Kevin Wolf, 2017/02/13
- [Qemu-block] [RFC PATCH 13/41] block: Request real permissions in bdrv_attach_child(), Kevin Wolf, 2017/02/13
- [Qemu-block] [RFC PATCH 14/41] block: Add permissions to BlockBackend, Kevin Wolf, 2017/02/13
- [Qemu-block] [RFC PATCH 15/41] block: Add permissions to blk_new(), Kevin Wolf, 2017/02/13
- Re: [Qemu-block] [RFC PATCH 15/41] block: Add permissions to blk_new(),
Max Reitz <=
- [Qemu-block] [RFC PATCH 17/41] block: Request real permissions in blk_new_open(), Kevin Wolf, 2017/02/13
- [Qemu-block] [RFC PATCH 16/41] block: Add error parameter to blk_insert_bs(), Kevin Wolf, 2017/02/13
- [Qemu-block] [RFC PATCH 18/41] block: Allow error return in BlockDevOps.change_media_cb(), Kevin Wolf, 2017/02/13
- [Qemu-block] [RFC PATCH 19/41] hw/block: Request permissions, Kevin Wolf, 2017/02/13