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
> +++ 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
> +#
> +# 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
> +# @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.
> +#
> +# 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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
