Re: [Qemu-devel] [RFC PATCH 20/41] hw/block: Introduce share-rw qdev property

[Kevin Wolf]

Am 20.02.2017 um 13:28 hat Max Reitz geschrieben:
> On 13.02.2017 18:22, Kevin Wolf wrote:
> > By default, don't allow another writer for block devices that are
> > attached to a guest device. For the cases where this setup is intended
> > (e.g. using a cluster filesystem on the disk), the new option can be
> > used to allow it.
> > 
> > This change affects only devices using DEFINE_BLOCK_PROPERTIES().
> > Devices directly using DEFINE_PROP_DRIVE() still accept writers
> > unconditionally.
> > 
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> >  hw/block/block.c           | 14 ++++++------
> >  include/hw/block/block.h   |  4 +++-
> >  tests/qemu-iotests/172.out | 53 
> > ++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 64 insertions(+), 7 deletions(-)
> > 
> > diff --git a/hw/block/block.c b/hw/block/block.c
> > index c3d3901..3c218eb 100644
> > --- a/hw/block/block.c
> > +++ b/hw/block/block.c
> > @@ -56,7 +56,7 @@ void blkconf_apply_backend_options(BlockConf *conf, bool 
> > readonly,
> >  {
> >      BlockBackend *blk = conf->blk;
> >      BlockdevOnError rerror, werror;
> > -    uint64_t perm;
> > +    uint64_t perm, shared_perm;
> >      bool wce;
> >      int ret;
> >  
> > @@ -65,11 +65,13 @@ void blkconf_apply_backend_options(BlockConf *conf, 
> > bool readonly,
> >          perm |= BLK_PERM_WRITE;
> >      }
> >  
> > -    /* TODO Remove BLK_PERM_WRITE unless explicitly configured so */
> > -    ret = blk_set_perm(blk, perm,
> > -                       BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED 
> > |
> > -                       BLK_PERM_GRAPH_MOD | BLK_PERM_RESIZE | 
> > BLK_PERM_WRITE,
> > -                       errp);
> > +    shared_perm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
> > +                  BLK_PERM_GRAPH_MOD | BLK_PERM_RESIZE;
> 
> I'm not so sure BLK_PERM_RESIZE belongs here.

Where does it belong in your opinion?

An option that I considered was adding BLK_PERM_RESIZE in
blk_set_dev_ops() if op.resize_cb != NULL, but it felt a bit too
implicit to me.

Or I guess we can just add another bool parameter?

Kevin

pgp07mRsxcPBm.pgp
Description: PGP signature