[22/47] block: make device optional in BlockInfo

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 <pbonzini@redhat.com>
---
 qapi-schema.json |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

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 <pbonzini@redhat.com>
> ---
>  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.

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.

Kevin

----- Messaggio originale -----
> Da: "Kevin Wolf" <kwolf@redhat.com>
> A: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: qemu-devel@nongnu.org, eblake@redhat.com, jcody@redhat.com, stefanha@linux.vnet.ibm.com
> 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 <pbonzini@redhat.com>
> > ---
> >  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.

Paolo

Am 11.09.2012 15:49, schrieb Paolo Bonzini:
> 
> 
> ----- Messaggio originale -----
>> Da: "Kevin Wolf" <kwolf@redhat.com>
>> A: "Paolo Bonzini" <pbonzini@redhat.com>
>> Cc: qemu-devel@nongnu.org, eblake@redhat.com, jcody@redhat.com, stefanha@linux.vnet.ibm.com
>> 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 <pbonzini@redhat.com>
>>> ---
>>>  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

> > 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?

No, not yet.  I was going to follow up this week.  Almost all the changes
are in the patches you already reviewed in July/August (and the changes
match your review).  So you can go on, just skip these three:

   add hierarchical bitmap data type and test cases (Laszlo reviewed
       this one offlist and I have some changes from him; better docs too)
   block: add target info to QMP query-blockjobs command
   mirror: support querying target file

Paolo