Re: [Qemu-devel] [Qemu-block] [PATCH v5 1/4] Add new block driver interface to add/delete a BDS's child

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3832 bytes --]

On 22.09.2015 09:44, Wen Congyang wrote:
> In some cases, we want to take a quorum child offline, and take
> another child online.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  block.c                   | 50 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/block/block.h     |  5 +++++
>  include/block/block_int.h |  5 +++++
>  3 files changed, 60 insertions(+)
> 
> diff --git a/block.c b/block.c
> index e815d73..1b25e43 100644
> --- a/block.c
> +++ b/block.c
> @@ -4265,3 +4265,53 @@ BlockAcctStats *bdrv_get_stats(BlockDriverState *bs)
>  {
>      return &bs->stats;
>  }
> +
> +/*
> + * Hot add/remove a BDS's child. So the user can take a child offline when
> + * it is broken and take a new child online
> + */
> +void bdrv_add_child(BlockDriverState *parent_bs, BlockDriverState *child_bs,
> +                    Error **errp)
> +{
> +
> +    if (!parent_bs->drv || !parent_bs->drv->bdrv_add_child) {
> +        error_setg(errp, "The BDS %s doesn't support adding a child",
> +                   bdrv_get_device_or_node_name(parent_bs));
> +        return;
> +    }
> +
> +    if (!QLIST_EMPTY(&child_bs->parents)) {
> +        error_setg(errp, "The BDS %s already has parent",
> +                   child_bs->node_name);
> +        return;
> +    }
> +
> +    parent_bs->drv->bdrv_add_child(parent_bs, child_bs, errp);
> +}
> +
> +void bdrv_del_child(BlockDriverState *parent_bs, BlockDriverState *child_bs,
> +                    Error **errp)
> +{
> +    BdrvChild *child;
> +
> +    if (!parent_bs->drv || !parent_bs->drv->bdrv_del_child) {
> +        error_setg(errp, "The BDS %s doesn't support removing a child",
> +                   bdrv_get_device_or_node_name(parent_bs));
> +        return;
> +    }
> +
> +    QLIST_FOREACH(child, &parent_bs->children, next) {
> +        if (child->bs == child_bs) {
> +            break;
> +        }
> +    }
> +
> +    if (!child) {
> +        error_setg(errp, "BDS %s is not a child of %s",
> +                   bdrv_get_device_or_node_name(child_bs),
> +                   bdrv_get_device_or_node_name(parent_bs));
> +        return;
> +    }
> +
> +    parent_bs->drv->bdrv_del_child(parent_bs, child_bs, errp);

How about we make this (BlockDriver.bdrv_del_child()) take a BdrvChild
instead of a BDS? We could even make bdrv_del_child() as a whole take a
BdrvChild parameter, but I don't suppose that would help much.

Max

> +}
> diff --git a/include/block/block.h b/include/block/block.h
> index ef67353..665c56f 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -616,4 +616,9 @@ void bdrv_flush_io_queue(BlockDriverState *bs);
>  
>  BlockAcctStats *bdrv_get_stats(BlockDriverState *bs);
>  
> +void bdrv_add_child(BlockDriverState *parent, BlockDriverState *child,
> +                    Error **errp);
> +void bdrv_del_child(BlockDriverState *parent, BlockDriverState *child,
> +                    Error **errp);
> +
>  #endif
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 2f2c47b..64cbc55 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -288,6 +288,11 @@ struct BlockDriver {
>       */
>      int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
>  
> +    void (*bdrv_add_child)(BlockDriverState *parent, BlockDriverState *child,
> +                           Error **errp);
> +    void (*bdrv_del_child)(BlockDriverState *parent, BlockDriverState *child,
> +                           Error **errp);
> +
>      QLIST_ENTRY(BlockDriver) list;
>  };
>  
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]