Am 20.04.2017 um 09:52 hat Fam Zheng geschrieben:
> Signed-off-by: Fam Zheng <f...@redhat.com>
> ---
> block.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/block.c b/block.c
> index 1fbbb8d..f5182d8 100644
> --- a/block.c
> +++ b/block.c
> @@ -1722,9 +1722,15 @@ void bdrv_format_default_perms(BlockDriverState *bs,
> BdrvChild *c,
> }
>
> /* bs->file always needs to be consistent because of the metadata. We
> - * can never allow other users to resize or write to it. */
> - perm |= BLK_PERM_CONSISTENT_READ;
> - shared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
> + * cannot allow other users to resize or write to it unless the
> caller
> + * explicitly expects unsafe readings. */
> + if (!(bdrv_get_flags(bs) & BDRV_O_UNSAFE_READ)) {
We have already spent considerable time to get rid of flags and instead
convert them into options passed in the QDict, so that they become
configurable with things like blockdev-add. Adding new flags potentially
moves in the opposite direction, so we have to be careful there.
I would be okay with patch 1, because in this case it's basically just a
shortcut for callers of blk_new_open(), which is fine. As soon as we
start querying the flag later and even rely on it being inherited, like
in this patch, I think it becomes a problem.
So if we need the flag in all nodes, can we make it an option that is
parsed in bdrv_open_common() into a bool bs->unsafe_read and inherited
explicitly in bdrv_inherited_options() and bdrv_backing_options()?
> + perm |= BLK_PERM_CONSISTENT_READ;
> + shared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
> + } else {
> + perm &= ~BLK_PERM_CONSISTENT_READ;
> + shared |= BLK_PERM_WRITE | BLK_PERM_RESIZE;
> + }
I'm not completely sure why we would be interested in CONSISTENT_READ
anyway, isn't allowing shared writes what we really need? (Which you
already do here in addition to dropping CONSISTENT_READ, without it
being mentioned in the commit message.)
Also, another thought: Being only at the start of the series, I'm not
sure what this will be used for, but can we make sure that unsafe_read
is only set if the image is opened read-only? If this is for the
libguestfs use case, this restriction should be fine.
Kevin
