Re: [Qemu-devel] [PATCH 01/16] introduce a new API to enable/disable attach device model

[Eric Blake <eblake@redhat.com>]

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

On 09/02/2015 02:51 AM, Wen Congyang wrote:
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  block/block-backend.c          | 24 ++++++++++++++++++++++++
>  include/sysemu/block-backend.h |  2 ++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/block/block-backend.c b/block/block-backend.c
> index aee8a12..72d8b2c 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -344,6 +344,30 @@ void *blk_get_attached_dev(BlockBackend *blk)
>  }
>  
>  /*
> + * Disable to attach a device mode to @blk.

s/to attach/attaching/
s/mode/model/

But I'm not even sure this patch makes sense.  I'd rather see Max's work
go in that allows for a BB without a BDS as representing a drive without
media, because then it is clear - either a BB has an associated BDS (and
cannot attach another one), or it does not (and attaching is fine).

> + * Return 0 on success, -EBUSY when a device model is attached already.
> + */
> +int blk_disable_attach_dev(BlockBackend *blk)
> +{
> +    if (blk->dev) {
> +        return blk->dev == (void *)-1 ? 0 : -EBUSY;
> +    }
> +
> +    blk->dev = (void *)-1;
> +    return 0;
> +}
> +
> +/*
> + * Enable to attach a device mode to @blk.
> + */
> +void blk_enable_attach_dev(BlockBackend *blk)
> +{
> +    if (blk->dev == (void *)-1) {

At the very least, if we allow a special sentinel to represent a BB
without a BDS (other than NULL, the way Max's series does it), it should
at least be wrapped by a macro, rather than using '(void *)-1' at
multiple call sites.

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


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