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

[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.

Paolo