Re: [Qemu-devel] [PATCH 22/47] block: make device optional in BlockInfo

[Kevin Wolf]

Am 11.09.2012 15:49, schrieb Paolo Bonzini:
> 
> 
> ----- Messaggio originale -----
>> Da: "Kevin Wolf" <address@hidden>
>> A: "Paolo Bonzini" <address@hidden>
>> Cc: address@hidden, address@hidden, address@hidden, address@hidden
>> Inviato: Martedì, 11 settembre 2012 15:38:33
>> Oggetto: Re: [PATCH 22/47] block: make device optional in BlockInfo
>>
>> Am 24.07.2012 13:04, schrieb Paolo Bonzini:
>>> Targets of a mirroring operation will not have a device.  Once we
>>> have
>>> -blockdev or equivalent, "detached" block devices and non-anonymous
>>> backing files also will not have a device.
>>>
>>> Signed-off-by: Paolo Bonzini <address@hidden>
>>> ---
>>>  qapi-schema.json |    5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>> index fca1806..b00d8c6 100644
>>> --- a/qapi-schema.json
>>> +++ b/qapi-schema.json
>>> @@ -443,7 +443,8 @@
>>>  # Block device information.  This structure describes a virtual
>>>  device and
>>>  # the backing device associated with it.
>>>  #
>>> -# @device: The device name associated with the virtual device.
>>> +# @device: #optional The device name associated with the virtual
>>> device.
>>> +#          Always included in the output of query-block.
>>>  #
>>>  # @type: This field is returned only for compatibility reasons, it
>>>  should
>>>  #        not be used (always returns 'unknown')
>>> @@ -465,7 +466,7 @@
>>>  # Since:  0.14.0
>>>  ##
>>>  { 'type': 'BlockInfo',
>>> -  'data': {'device': 'str', 'type': 'str', 'removable': 'bool',
>>> +  'data': {'*device': 'str', 'type': 'str', 'removable': 'bool',
>>>             'locked': 'bool', '*inserted': 'BlockDeviceInfo',
>>>             '*tray_open': 'bool', '*io-status':
>>>             'BlockDeviceIoStatus'} }
>>
>> Is this really a compatible change? That 'device' is basically the
>> unique key by which block device are identified doesn't exactly make
>> feel more comfortable about the change.
> 
> As long as query-block ensures that the field is present---yes.
> 
>> Of course, not making it optional means that basically we need to go
>> the way of referencing the block device in query-block-jobs immediately
>> instead of thinking about it later. You know that I preferred this
>> from the start, and this change is just another detail that makes me think
>> it's the right thing to do.
> 
> Indeed; this patch is not anymore in the current version of the series,
> after your comments from July/August.

Wait, am I reviewing the wrong version of the series? :-/

Did you post a newer one?

Kevin