Re: [Qemu-devel] [PATCH v3 09/12] block: add ability for block-stream to use node-name

[Eric Blake]

On 05/30/2014 11:35 AM, Jeff Cody wrote:
> This adds the ability for block-stream to use node-name arguments
> for base, to specify the backing image to stream from.
> 
> Both 'base' and 'base-node-name' are optional, but mutually exclusive.
> Either can be specified, but not both together.
> 
> The argument for "device" is now optional as well, so long as
> base-node-name is specified.  From the node-name, we can determine
> the full device chain.
> 
> Signed-off-by: Jeff Cody <address@hidden>
> ---
>  blockdev.c       | 63 
> ++++++++++++++++++++++++++++++++++++++++++++++----------
>  hmp.c            |  4 ++--
>  qapi-schema.json | 17 +++++++++++----
>  qmp-commands.hx  |  2 +-
>  4 files changed, 68 insertions(+), 18 deletions(-)
> 

> +    if (!has_base_node_name && !has_device) {
> +        error_setg(errp, "'device' required if 'base-node-name' not 
> specified");
> +        return;
> +    }
> +
> +    if (has_device) {
> +        bs = bdrv_find(device);
> +        if (!bs) {
> +            error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> +            return;
> +        }
> +    }
> +

Based on these two earlier clauses...

> +    if (has_base_node_name) {
> +        base_bs = bdrv_lookup_bs(NULL, base_node_name, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return;
> +        }
> +        /* the overlay->backing_file may not always be equal to
> +         * base_bs->filename, due to how relative paths are calculated */
> +        base_name = base_bs->overlay ? base_bs->overlay->backing_file : NULL;
> +        bs = bs ?: bdrv_find_active(base_bs);
> +    }
> +
>      if (!bs) {

...the only way this condition can be met is if the has_base_node_name
assignment to bs failed (that is, the bdrv_find_active(base_bs) failed).

> -        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> +        error_setg(errp, "Could not find active layer");
>          return;
>      }

But I'm not sure if it is worth hoisting the error check into the
earlier clause (I'm fine leaving it here).

Reviewed-by: Eric Blake <address@hidden>

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

signature.asc
Description: OpenPGP digital signature