From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44002) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VlhgG-0001ba-BD for qemu-devel@nongnu.org; Wed, 27 Nov 2013 11:09:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VlhgA-0007s1-BG for qemu-devel@nongnu.org; Wed, 27 Nov 2013 11:09:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:6677) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VlhgA-0007rq-2c for qemu-devel@nongnu.org; Wed, 27 Nov 2013 11:09:30 -0500 Date: Wed, 27 Nov 2013 17:09:13 +0100 From: Igor Mammedov Message-ID: <20131127170913.52fa0596@nial.usersys.redhat.com> In-Reply-To: <52960EE2.9010201@redhat.com> References: <1385001528-12003-1-git-send-email-imammedo@redhat.com> <1385001528-12003-8-git-send-email-imammedo@redhat.com> <52960EE2.9010201@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 07/27] add memdev backend infrastructure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: peter.maydell@linaro.org, stefanha@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, hutao@cn.fujitsu.com, stefanb@linux.vnet.ibm.com, mjt@tls.msk.ru, mdroth@linux.vnet.ibm.com, armbru@redhat.com, vasilis.liaskovitis@profitbricks.com, quintela@redhat.com, kraxel@redhat.com, aliguori@amazon.com, pbonzini@redhat.com, marcel.a@redhat.com, lcapitulino@redhat.com, chegu_vinod@hp.com, afaerber@suse.de On Wed, 27 Nov 2013 08:25:22 -0700 Eric Blake 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!