[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/5] block: add helper function to determine if
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH 2/5] block: add helper function to determine if a BDS is in a chain |
Date: |
Thu, 15 May 2014 10:31:53 -0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Thu, May 15, 2014 at 08:16:27AM -0600, Eric Blake wrote:
> On 05/14/2014 09:20 PM, Jeff Cody wrote:
> > This is a small helper function, to determine if 'base' is in the
> > chain of BlockDriverState 'top'. It returns true if it is in the chain,
> > and false otherwise.
> >
> > If either argument is NULL, it will also return false.
> >
> > Signed-off-by: Jeff Cody <address@hidden>
> > ---
> > block.c | 9 +++++++++
> > include/block/block.h | 1 +
> > 2 files changed, 10 insertions(+)
> >
>
> >
> > +bool bdrv_is_in_chain(BlockDriverState *top, BlockDriverState *base)
>
> No doc comments inline, and not everyone has the commit message handy.
> Which means someone trying to learn what this command does has to read
> the function. Maybe copy the commit message into code comments as well.
>
OK
> Bikeshedding: If I'm reading code, and see bdrv_is_in_chain(a, b), my
> first inclination would be to read that as "return true if node a is in
> chain b". But if I were to see bdrv_chain_contains(a, b), I would parse
> that as "return true if chain a contains node b". I think you either
> want to swap argument order, or rename the function.
I like the bdrv_chain_contains() as the function name, it is clearer.
I'll use that.
>
> The function itself looks useful, though, once we agree on the naming.
>
- Re: [Qemu-devel] [PATCH 1/5] block: Auto-generate node_names for each BDS entry, (continued)
Re: [Qemu-devel] [PATCH 1/5] block: Auto-generate node_names for each BDS entry, Eric Blake, 2014/05/15
Re: [Qemu-devel] [PATCH 1/5] block: Auto-generate node_names for each BDS entry, Eric Blake, 2014/05/15
[Qemu-devel] [PATCH 2/5] block: add helper function to determine if a BDS is in a chain, Jeff Cody, 2014/05/14
[Qemu-devel] [PATCH 3/5] block: make 'top' argument to block-commit optional, Jeff Cody, 2014/05/14
[Qemu-devel] [PATCH 4/5] block: Accept node-name arguments for block-commit, Jeff Cody, 2014/05/14
[Qemu-devel] [PATCH 5/5] block: extend block-commit to accept a string for the backing file, Jeff Cody, 2014/05/14