Re: [Qemu-block] [PATCH 6/6] block: Fix permissions after bdrv_reopen()

[Kevin Wolf]

Am 18.09.2017 um 09:37 hat Fam Zheng geschrieben:
> On Fri, 09/15 12:10, Kevin Wolf wrote:
> > If we switch between read-only and read-write, the permissions that
> > image format drivers need on bs->file change, too. Make sure to update
> > the permissions during bdrv_reopen().
> > 
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> >  include/block/block.h |  1 +
> >  block.c               | 64 
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 65 insertions(+)
> > 
> > diff --git a/include/block/block.h b/include/block/block.h
> > index 082eb2cd9c..3c3af462e4 100644
> > --- a/include/block/block.h
> > +++ b/include/block/block.h
> > @@ -166,6 +166,7 @@ typedef QSIMPLEQ_HEAD(BlockReopenQueue, 
> > BlockReopenQueueEntry) BlockReopenQueue;
> >  typedef struct BDRVReopenState {
> >      BlockDriverState *bs;
> >      int flags;
> > +    uint64_t perm, shared_perm;
> >      QDict *options;
> >      QDict *explicit_options;
> >      void *opaque;
> > diff --git a/block.c b/block.c
> > index 204cbb46c7..5c65fac672 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -2781,6 +2781,10 @@ static BlockReopenQueue 
> > *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
> >      bs_entry->state.explicit_options = explicit_options;
> >      bs_entry->state.flags = flags;
> >  
> > +    /* This needs to be overwritten in bdrv_reopen_prepare() */
> > +    bs_entry->state.perm = UINT64_MAX;
> 
> Probably doesn't matter because as the comment says it will be overwritten 
> soon,
> but is BLK_PERM_ALL more appropriate?

I had BLK_PERM_ALL at first, but after debugging some assertion failures
in gdb, I came to the conclusion that UINT64_MAX is easier to identify as
uninitialised than BLK_PERM_ALL, which could be a valid result of the
permission calculation.

Kevin