On Wed, 27 Nov 2013 08:25:22 -0700
Eric Blake <ebl...@redhat.com> wrote:
> On 11/20/2013 07:38 PM, Igor Mammedov wrote:
> > Provides framework for splitting host RAM allocation/
> > policies into a separate backend that could be used
> > by devices. It would allow to separate host specific
> > options from device model like it's done for netdev & co.
>
> Just an interface review:
>
> > +++ b/hmp-commands.hx
> > @@ -1719,3 +1719,16 @@ ETEXI
> > STEXI
> > @end table
> > ETEXI
> > +
> > + {
> > + .name = "memdev-add",
>
> Typically, we've been naming HMP commands with underscore: memdev_add
there is several DASH command in HMP and I thought we stopped to
do UNDERSCORE and that DASH is preffered way now.
>
> > +++ b/qapi-schema.json
> > @@ -4212,3 +4212,21 @@
> > # Since: 1.7
> > ##
> > { 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
> > +
> > +##
> > +# @memdev_add:
>
> Whereas QMP commands have a dash: memdev-add
I'll fix it.
>
> > +#
> > +# Add a host memory backend.
> > +#
> > +# @id: the name of the new memory backend
> > +# @size: amount of memory backend should allocate
>
> Maybe add "in bytes" somewhere in the phrase to make it clear
Sure
>
> > +# @type: backend type. [default: compat-ram-host-memory]
>
> That's the default, but what other types are valid? I'd much rather
> have an enum of valid types than an open-coded string.
there is only one so far, I plan to add additional hugepage backend later.
>
> > +#
> > +# Since: 1.8
> > +#
> > +# Returns: Nothing on success
> > +##
> > +{ 'command': 'memdev-add',
>
> Ah, you did name the QMP command with dash, so it's the docs above that
> are wrong.
>
> > + 'data': {'id': 'str', 'size': 'size', '*type': 'str'},
> > + 'gen': 'no'
> > +}
>
> > +Arguments:
> > +
> > +- "id": the device's ID, must be unique (json-string)
> > +- "size": amount of memory backend should allocate (json-int)
> > +- "type": backend type (json-string, optional), default:
> > compat-ram-host-memory
> > +
> > +Example:
> >
> > +-> { "execute": "memdev-add",
> > + "arguments": { "id": "memdev1",
> > + "size": "1G",
>
> I don't think this is valid QMP; you want to be passing sizes in bytes
> as an integer, not as a string that requires further parsing.
I'll amend it.
Thanks!
