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

[Wen Congyang]

On 09/02/2015 11:37 PM, Eric Blake wrote:
> On 09/02/2015 02:51 AM, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang <address@hidden>
>> ---
>>  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).

Hmm, I think the API's name cause a misunderstanding.

These APIs are used to check if the BB is used by some block device. It is very
useful if we allow references for backing files. If the backing file referes an
existing BDS that with BB, the block device can't use this BB at the same time.

Thanks
Wen Congyang

> 
>> + * 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.
>