[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] block: Use bdrv_lookup_bs() instead of bdrv
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] block: Use bdrv_lookup_bs() instead of bdrv_find_node() |
Date: |
Fri, 16 Oct 2015 09:52:51 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Jeff Cody <address@hidden> 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 <address@hidden>
> ---
> 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.
- Re: [Qemu-devel] [PATCH 2/3] block: make bdrv_find_node() static, (continued)