[1/3] block: Use bdrv_lookup_bs() instead of bdrv_find_node()

This is a precursor to making bdrv_find_node() static, and internal
to block.c

To find a BlockDriverState interface, it can be done via blk_by_name(),
bdrv_find_node(), and bdrv_lookup_bs().  The latter can take the place
of the other two, in the instances where we are only concerned with
the BlockDriverState.

There is no benefit in calling bdrv_find_node() directly.  This patch
replaces all calls to bdrv_find_node() outside of block.c with
bdrv_lookup_bs().

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/block-backend.c   | 2 +-
 block/mirror.c          | 2 +-
 block/write-threshold.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

On 14.10.2015 15:16, Jeff Cody wrote:
> This is a precursor to making bdrv_find_node() static, and internal
> to block.c
> 
> To find a BlockDriverState interface, it can be done via blk_by_name(),
> bdrv_find_node(), and bdrv_lookup_bs().  The latter can take the place
> of the other two, in the instances where we are only concerned with
> the BlockDriverState.
> 
> There is no benefit in calling bdrv_find_node() directly.  This patch
> replaces all calls to bdrv_find_node() outside of block.c with
> bdrv_lookup_bs().
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/block-backend.c   | 2 +-
>  block/mirror.c          | 2 +-
>  block/write-threshold.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

On 14.10.2015 19:29, Max Reitz wrote:
> On 14.10.2015 15:16, Jeff Cody wrote:
>> This is a precursor to making bdrv_find_node() static, and internal
>> to block.c
>>
>> To find a BlockDriverState interface, it can be done via blk_by_name(),
>> bdrv_find_node(), and bdrv_lookup_bs().  The latter can take the place
>> of the other two, in the instances where we are only concerned with
>> the BlockDriverState.
>>
>> There is no benefit in calling bdrv_find_node() directly.  This patch
>> replaces all calls to bdrv_find_node() outside of block.c with
>> bdrv_lookup_bs().
>>
>> Signed-off-by: Jeff Cody <jcody@redhat.com>
>> ---
>>  block/block-backend.c   | 2 +-
>>  block/mirror.c          | 2 +-
>>  block/write-threshold.c | 2 +-
>>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>

Oh, wait, on patch 2 gcc tells me I should take that back. If this
series is based on Berto's series, that means it's also based on my
BlockBackend series, and that one adds another instance of bdrv_find_node().

Since I'll have to send a v7 anyway, I'll make it a bdrv_lookup_bs()
there, so my R-b stands.

Max

On Wed, Oct 14, 2015 at 07:34:43PM +0200, Max Reitz wrote:
> On 14.10.2015 19:29, Max Reitz wrote:
> > On 14.10.2015 15:16, Jeff Cody wrote:
> >> This is a precursor to making bdrv_find_node() static, and internal
> >> to block.c
> >>
> >> To find a BlockDriverState interface, it can be done via blk_by_name(),
> >> bdrv_find_node(), and bdrv_lookup_bs().  The latter can take the place
> >> of the other two, in the instances where we are only concerned with
> >> the BlockDriverState.
> >>
> >> There is no benefit in calling bdrv_find_node() directly.  This patch
> >> replaces all calls to bdrv_find_node() outside of block.c with
> >> bdrv_lookup_bs().
> >>
> >> Signed-off-by: Jeff Cody <jcody@redhat.com>
> >> ---
> >>  block/block-backend.c   | 2 +-
> >>  block/mirror.c          | 2 +-
> >>  block/write-threshold.c | 2 +-
> >>  3 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > Reviewed-by: Max Reitz <mreitz@redhat.com>
> 
> Oh, wait, on patch 2 gcc tells me I should take that back. If this
> series is based on Berto's series, that means it's also based on my
> BlockBackend series, and that one adds another instance of bdrv_find_node().
> 
> Since I'll have to send a v7 anyway, I'll make it a bdrv_lookup_bs()
> there, so my R-b stands.
> 
> Max
> 

Thanks.  I actually managed to apply Berto's without yours, but I just
went back and rectified that, and applied yours.

Jeff

On Wed 14 Oct 2015 03:16:00 PM CEST, Jeff Cody <jcody@redhat.com> wrote:
> This is a precursor to making bdrv_find_node() static, and internal
> to block.c
>
> To find a BlockDriverState interface, it can be done via blk_by_name(),
> bdrv_find_node(), and bdrv_lookup_bs().  The latter can take the place
> of the other two, in the instances where we are only concerned with
> the BlockDriverState.
>
> There is no benefit in calling bdrv_find_node() directly.  This patch
> replaces all calls to bdrv_find_node() outside of block.c with
> bdrv_lookup_bs().
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Jeff Cody <jcody@redhat.com> writes:

> This is a precursor to making bdrv_find_node() static, and internal
> to block.c
>
> To find a BlockDriverState interface, it can be done via blk_by_name(),
> bdrv_find_node(), and bdrv_lookup_bs().  The latter can take the place
> of the other two, in the instances where we are only concerned with
> the BlockDriverState.
>
> There is no benefit in calling bdrv_find_node() directly.  This patch
> replaces all calls to bdrv_find_node() outside of block.c with
> bdrv_lookup_bs().
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/block-backend.c   | 2 +-
>  block/mirror.c          | 2 +-
>  block/write-threshold.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 2256551..7026a3f 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -67,7 +67,7 @@ BlockBackend *blk_new(const char *name, Error **errp)
>          error_setg(errp, "Device with id '%s' already exists", name);
>          return NULL;
>      }
> -    if (bdrv_find_node(name)) {
> +    if (bdrv_lookup_bs(NULL, name, NULL)) {
>          error_setg(errp,
>                     "Device name '%s' conflicts with an existing node name",
>                     name);

Here, you ignore bdrv_lookup_bs() errors because we actually succeed on
bdrv_lookup_bs() error.  Good.

> diff --git a/block/mirror.c b/block/mirror.c
> index 7e43511..cb3c765 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -644,7 +644,7 @@ static void mirror_complete(BlockJob *job, Error **errp)
>      if (s->replaces) {
>          AioContext *replace_aio_context;
>  
> -        s->to_replace = bdrv_find_node(s->replaces);
> +        s->to_replace = bdrv_lookup_bs(NULL, s->replaces, NULL);
>          if (!s->to_replace) {
>              error_setg(errp, "Node name '%s' not found", s->replaces);
>              return;

Here, you ignore its errors because the caller sets a better one.
However, the callers is better only because bdrv_lookup_bs()'s sucks:
"Cannot find device= or node_name=FOO".  Follow-up patch to improve that
error and use it here?

> diff --git a/block/write-threshold.c b/block/write-threshold.c
> index a53c1f5..908fa7f 100644
> --- a/block/write-threshold.c
> +++ b/block/write-threshold.c
> @@ -110,7 +110,7 @@ void qmp_block_set_write_threshold(const char *node_name,
>      BlockDriverState *bs;
>      AioContext *aio_context;
>  
> -    bs = bdrv_find_node(node_name);
> +    bs = bdrv_lookup_bs(NULL, node_name, NULL);
>      if (!bs) {
>          error_setg(errp, "Device '%s' not found", node_name);
>          return;

Likewise.