Re: [Qemu-devel] [Qemu-block] [PATCH RFC 04/16] block: Propagate BLK_PERM_AIO_CONTEXT_CHANGE down the graph

[Fam Zheng <famz@redhat.com>]

On Mon, 04/10 09:57, Stefan Hajnoczi wrote:
> On Tue, Mar 21, 2017 at 11:16:23AM +0800, Fam Zheng wrote:
> > @@ -1713,21 +1714,22 @@ void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c,
> >          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.
> > +        /* We want consistent read and aio context change from backing files if
> > +         * the parent needs it.
> >           * No other operations are performed on backing files. */
> > -        perm &= BLK_PERM_CONSISTENT_READ;
> > +        perm &= BLK_PERM_CONSISTENT_READ | BLK_PERM_AIO_CONTEXT_CHANGE;
> >  
> > -        /* If the parent can deal with changing data, we're okay with a
> > +        /* If the parent can deal with changing aio context, we're okay too;
> > +         * If the parent can deal with changing data, we're okay with a
> >           * writable and resizable backing file. */
> >          /* TODO Require !(perm & BLK_PERM_CONSISTENT_READ), too? */
> > +        shared &= BLK_PERM_AIO_CONTEXT_CHANGE | BLK_PERM_WRITE;
> >          if (shared & BLK_PERM_WRITE) {
> > -            shared = BLK_PERM_WRITE | BLK_PERM_RESIZE;
> > -        } else {
> > -            shared = 0;
> > +            shared |= BLK_PERM_WRITE | BLK_PERM_RESIZE;
> 
> We already have BLK_PERM_WRITE so we're just adding BLK_PERM_RESIZE.
> The following is clearer:
> 
>   shared |= BLK_PERM_RESIZE;
> 
> >          }
> >  
> >          shared |= BLK_PERM_CONSISTENT_READ | BLK_PERM_GRAPH_MOD |
> > -                  BLK_PERM_WRITE_UNCHANGED;
> > +                  BLK_PERM_WRITE_UNCHANGED | BLK_PERM_AIO_CONTEXT_CHANGE;
> 
> Why was shared &= BLK_PERM_AIO_CONTEXT_CHANGE necessary above if we
> unconditionally OR it here?

It's redundant. Will fix both.

Fam