Re: [Qemu-devel] [PATCH 2/3] block: Add 'blockdev-del' QMP command

[Max Reitz]

On 13.10.2015 15:48, Alberto Garcia wrote:
> This is the companion to 'blockdev-add'. It allows deleting a
> BlockBackend with its associated BlockDriverState tree, or a
> BlockDriverState that is not attached to any backend.
> 
> In either case, the command fails if the reference count is greater
> than 1 or the BlockDriverState has any parents.
> 
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
>  blockdev.c           | 42 ++++++++++++++++++++++++++++++++++++++++++
>  qapi/block-core.json | 21 +++++++++++++++++++++
>  qmp-commands.hx      | 36 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 99 insertions(+)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 2360c1f..c448a0b 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3402,6 +3402,48 @@ fail:
>      qmp_output_visitor_cleanup(ov);
>  }
>  
> +void qmp_blockdev_del(const char *device, Error **errp)
> +{
> +    AioContext *aio_context;
> +    BlockBackend *blk;
> +    BlockDriverState *bs;
> +
> +    blk = blk_by_name(device);
> +    if (blk) {
> +        bs = blk_bs(blk);
> +        aio_context = blk_get_aio_context(blk);
> +    } else {
> +        bs = bdrv_find_node(device);
> +        if (!bs) {
> +            error_setg(errp, "Cannot find block device %s", device);
> +            return;
> +        }
> +        blk = bs->blk;

After seeing testBlockBackendUsingNodeName() in patch 3, I don't know
about this. We often have this notion of "If you need a BDS, you can
specify both the BB and the node name", but here it's the other way
around: One "needs" a BB (at least that's what the command will work on)
and one gets it by specifying a node name. That seems a bit strange to me.

Max

> +        aio_context = bdrv_get_aio_context(bs);
> +    }
> +
> +    aio_context_acquire(aio_context);
> +
> +    if (bs && bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, errp)) {
> +        goto out;
> +    }
> +
> +    if (blk_get_refcnt(blk) > 1 ||
> +        (bs && (bs->refcnt > 1 || !QLIST_EMPTY(&bs->parents)))) {
> +        error_setg(errp, "Block device %s is being used", device);
> +        goto out;
> +    }
> +
> +    if (blk) {
> +        blk_unref(blk);
> +    } else {
> +        bdrv_unref(bs);
> +    }
> +
> +out:
> +    aio_context_release(aio_context);
> +}
> +
>  BlockJobInfoList *qmp_query_block_jobs(Error **errp)
>  {
>      BlockJobInfoList *head = NULL, **p_next = &head;

signature.asc
Description: OpenPGP digital signature