Re: [Qemu-devel] [PATCH] block: Always call bdrv_child_check_perm first

[Fam Zheng <famz@redhat.com>]

On Tue, 03/14 08:28, Eric Blake wrote:
> On 03/13/2017 09:30 PM, Fam Zheng wrote:
> > bdrv_child_set_perm alone is not very usable because the caller must
> > call bdrv_child_check_perm first. This is already encapsulated
> > conveniently in bdrv_child_try_set_perm, so remove the other prototypes
> > from the header and fix the one wrong caller, block/mirror.c.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  block.c                   | 13 +++++++++----
> >  block/mirror.c            |  6 ++++--
> >  include/block/block_int.h |  4 ----
> >  3 files changed, 13 insertions(+), 10 deletions(-)
> > 
> > diff --git a/block.c b/block.c
> > index cb57370..a77e8a0 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -1393,6 +1393,11 @@ static int bdrv_fill_options(QDict **options, const char *filename,
> >      return 0;
> >  }
> >  
> > +static int bdrv_child_check_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
> > +                                 GSList *ignore_children, Error **errp);
> > +static void bdrv_child_abort_perm_update(BdrvChild *c);
> > +static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);
> > +
> 
> Now that you have static prototypes, is it worth rearranging the file
> (in a followup) to sort the function implementations into topological
> order so that a prototype is not necessary?  [In general, I try to code
> so that static prototypes are only necessary if I am implementing
> mutually-referencing recursive code. But it's not a strict requirement]

Yes, thanks for pointing out, but it does have a recursion here:

bdrv_check_update_perm
    -> bdrv_check_perm
        -> bdrv_child_check_perm
            -> bdrv_check_update_perm

Fam

> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>