Re: [Qemu-devel] [PATCH 7/7] monitor: QMP/HMP support for retrieving VNVRAM details

[Corey Bryant]

On 05/23/2013 01:59 PM, Eric Blake wrote:
> On 05/23/2013 11:44 AM, Corey Bryant wrote:
> > Signed-off-by: Corey Bryant <address@hidden>
> > ---
>
> Might help to list a sample HMP or QMP usage in the commit message.
>
> > +++ b/qapi-schema.json
> > @@ -3619,3 +3619,50 @@
> >               '*cpuid-input-ecx': 'int',
> >               'cpuid-register': 'X86CPURegister32',
> >               'features': 'int' } }
> > +
> > +# @VNVRAMEntryInfo:
> > +#
> > +# Information about an entry in the VNVRAM.
> > +#
> > +# @name: name of the entry
> > +#
> > +# @cur-size: current size of the entry's blob in bytes
> > +#
> > +# @max-size: max size of the entry's blob in bytes
> > +#
> > +# Since: 1.6
> > +#
> > +##
> > +{ 'type': 'VNVRAMEntryInfo',
> > +  'data': {'name': 'str', 'cur-size': 'int', 'max-size': 'int', } }
>
> No trailing commas in JSON.  :(

I'll fix that.

> > +
> > +##
> > +# @VNVRAMInfo:
> > +#
> > +# Information about the VNVRAM device.
> > +#
> > +# @drive-id: ID of the VNVRAM (and associated drive)
> > +#
> > +# @virtual-disk-size: Virtual size of the associated disk drive in bytes
> > +#
> > +# @vnvram-size: Size of the VNVRAM in bytes
> > +#
> > +# @entries: Array of @VNVRAMEntryInfo
> > +#
> > +# Since: 1.6
> > +#
> > +##
> > +{ 'type': 'VNVRAMInfo',
> > +  'data': {'drive-id': 'str', 'virtual-disk-size': 'int',
> > +           'vnvram-size': 'int', 'entries' : ['VNVRAMEntryInfo']} }
> > +
> > +##
> > +# @query-vnvram:
> > +#
> > +# Return information about the VNVRAM devices.
> > +#
> > +# Returns: @VNVRAMInfo on success
> > +#
> > +# Since: 1.6
> > +##
> > +{ 'command': 'query-vnvram', 'returns': ['VNVRAMInfo'] }
>
> Other than that, this looks fine from an interface point of view.  I
> haven't closely reviewed code, though.
>
> > +
> > +Example:
> > +
> > +-> { "execute": "query-vnvram" }
> > +<- {"return": [
> > +      { "vnvram-size": 2050, "virtual-disk-size": 2000896,
> > +        "drive-id": "drive-ide0-0-0",
> > +        "entries": [
> > +         { "name": "this-entry", "cur-size": 2048, "max-size": 21504 },
> > +         { "name": "that-entry", "cur-size": 1024, "max-size": 21504 },
> > +         { "name": "other-entry", "cur-size": 4096, "max-size": 41472 } ]
> > +      } ]
> > +   }
>
> Looks reasonable.

Thanks for the review!

--
Regards,
Corey Bryant