On 15.02.2017 18:29, Kevin Wolf wrote: > Am 15.02.2017 um 18:11 hat Max Reitz geschrieben: >> On 13.02.2017 18:22, Kevin Wolf wrote: >>> Almost all format drivers have the same characteristics as far as >>> permissions are concerned: They have one or more children for storing >>> their own data and, more importantly, metadata (can be written to and >>> grow even without external write requests, must be protected against >>> other writers and present consistent data) and optionally a backing file >>> (this is just data, so like for a filter, it only depends on what the >>> parent nodes need). >>> >>> This provides a default implementation that can be shared by most of >>> our format drivers. >>> >>> Signed-off-by: Kevin Wolf <kw...@redhat.com> >>> --- >>> block.c | 37 +++++++++++++++++++++++++++++++++++++ >>> include/block/block_int.h | 8 ++++++++ >>> 2 files changed, 45 insertions(+) >>> >>> diff --git a/block.c b/block.c >>> index 290768d..8e99bb5 100644 >>> --- a/block.c >>> +++ b/block.c >>> @@ -1459,6 +1459,43 @@ void bdrv_filter_default_perms(BlockDriverState *bs, >>> BdrvChild *c, >>> (c->shared_perm & DEFAULT_PERM_UNCHANGED); >>> } >>> >>> +void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c, >>> + const BdrvChildRole *role, >>> + uint64_t perm, uint64_t shared, >>> + uint64_t *nperm, uint64_t *nshared) >>> +{ >>> + bool backing = (role == &child_backing); >>> + assert(role == &child_backing || role == &child_file); >>> + >>> + if (!backing) { >>> + /* Apart from the modifications below, the same permissions are >>> + * forwarded and left alone as for filters */ >>> + bdrv_filter_default_perms(bs, c, role, perm, shared, &perm, >>> &shared); >>> + >>> + /* Format drivers may touch metadata even if the guest doesn't >>> write */ >>> + if (!bdrv_is_read_only(bs)) { >>> + perm |= BLK_PERM_WRITE | BLK_PERM_RESIZE; >>> + } >>> + >>> + /* 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); >>> + } else { >>> + /* We want consistent read from backing files if the parent needs >>> it. >>> + * No other operations are performed on backing files. */ >>> + perm &= BLK_PERM_CONSISTENT_READ; >>> + >>> + /* If the parent can deal with changing data, we're okay with a >>> + * writable backing file. */ >> >> Are we OK with a resizable backing file, too? I'm not sure, actually. >> Maybe we should just forbid it and hope nobody asks for it. > > That's pretty much the same thought that I had, so unless I'm mistaken, > this is exactly what the patch implements.
Yes, it is. I'm just dumping random thoughts. :-) > I can't think of a reason why resizing a backing file would hurt as long > as the parent allows writes to the backing file (though we might have to > audit that no driver caches the backing file size), but then I can't > think of a reason either why anyone would want to do this. > > If we ever find out that there is a good reason, we can still change it. > > ... > > Hm, scratch that. I guess a commit block job with a smaller backing file > is enough... Good thing that I neglected to get the RESIZE permission > for s->base there. :-) TIL commit resizes the base in that case. Max
signature.asc
Description: OpenPGP digital signature