From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41300) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SXXiJ-0007Ic-TX for qemu-devel@nongnu.org; Thu, 24 May 2012 09:04:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SXXiE-0001xP-6W for qemu-devel@nongnu.org; Thu, 24 May 2012 09:04:23 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:64356) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SXXiE-0001x8-00 for qemu-devel@nongnu.org; Thu, 24 May 2012 09:04:18 -0400 Received: by obbwd20 with SMTP id wd20so15505815obb.4 for ; Thu, 24 May 2012 06:04:16 -0700 (PDT) Message-ID: <4FBE31CD.1080101@codemonkey.ws> Date: Thu, 24 May 2012 08:04:13 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1337859784-24097-1-git-send-email-armbru@redhat.com> <1337859784-24097-3-git-send-email-armbru@redhat.com> In-Reply-To: <1337859784-24097-3-git-send-email-armbru@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC 2/2] qmp: New command qom-new List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, afaerber@suse.de On 05/24/2012 06:43 AM, Markus Armbruster wrote: > To create objects via QMP. > > Test case: > > $ upstream-qemu --enable-kvm -S -m 384 -vnc :0 -monitor stdio -chardev socket,id=qmp,path=test-qmp,server=on,wait=off -mon mode=control,chardev=qmp > > Conversation on the qmp socket: > {"QMP": {"version": {"qemu": {"micro": 93, "minor": 0, "major": 1}, "package": ""}, "capabilities": []}} > { "execute": "qmp_capabilities" } > {"return": {}} > {"execute":"qom-new","arguments":{"parent":"/xxx", "prop-name":"test-qmp-new", "type-name":"xxx"}} > {"error": {"class": "DeviceNotFound", "desc": "Device '/xxx' not found", "data": {"device": "/xxx"}}} > {"execute":"qom-new","arguments":{"parent":"/machine", "prop-name":"test-qmp-new", "type-name":"xxx"}} > {"error": {"class": "InvalidParameterValue", "desc": "Parameter 'type-name' expects a type name", "data": {"name": "type-name", "expected": "a type name"}}} > {"execute":"qom-new","arguments":{"parent":"/machine", "prop-name":"test-qmp-new", "type-name":"container"}} > {"return": "/machine/test-qmp-new"} > {"execute":"qom-list","arguments":{"path":"/machine"}} > {"return": [{"name": "test-qmp-new", "type": "child"}, {"name": "i440fx", "type": "child"}, {"name": "unattached", "type": "child"}, {"name": "peripheral", "type": "child"}, {"name": "peripheral-anon", "type": "child"}]} > > Note: qdev objects (subtype of TYPE_DEVICE) created with qom-new lack > additional magic performed by qdev_try_create(), and almost certainly > won't work. > > Signed-off-by: Markus Armbruster > --- > qapi-schema.json | 22 ++++++++++++++++++++++ > qmp-commands.hx | 5 +++++ > qmp.c | 27 +++++++++++++++++++++++++++ > 3 files changed, 54 insertions(+), 0 deletions(-) > > diff --git a/qapi-schema.json b/qapi-schema.json > index 2ca7195..ab9e68b 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -1699,6 +1699,28 @@ > 'returns': [ 'ObjectTypeInfo' ] } > > ## > +# qom-new: > +# > +# Create a new object > +# > +# @parent: the parent's path within the object model. See @qom-get > +# for a description of paths. > +# > +# @prop-name: the name of the property to add to the parent. > +# > +# @type: the new object's type name > +# > +# Returns: The new object's canonical absolute path > +# > +# Since: 1.2 > +# > +# Notes: This command is experimental and may change syntax in future releases. > +## > +{ 'command': 'qom-new', > + 'data': { 'parent': 'str', 'prop-name': 'str', 'type-name': 'str' }, > + 'returns': 'str' } > + > +## > # @migrate > # > # Migrates the current running guest to another Virtual Machine. > diff --git a/qmp-commands.hx b/qmp-commands.hx > index db980fa..53adda2 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -2142,3 +2142,8 @@ EQMP > .args_type = "implements:s?,abstract:b?", > .mhandler.cmd_new = qmp_marshal_input_qom_list_types, > }, > + { > + .name = "qom-new", > + .args_type = "parent:s,prop-name:s,type-name:s", > + .mhandler.cmd_new = qmp_marshal_input_qom_new, > + }, > diff --git a/qmp.c b/qmp.c > index fee9fb2..cad5610 100644 > --- a/qmp.c > +++ b/qmp.c > @@ -417,3 +417,30 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements, > > return ret; > } > + > +char *qmp_qom_new(const char *parent, const char *prop_name, > + const char *type_name, Error **errp) > +{ > + Object *p, *obj; > + Type type; > + > + // TODO anything fancy with containger_get() needed? I'm not sure how I feel about this. I never intended for a user to be able to create objects that were arbitrary children of other objects. In some ways, I think this is almost too powerful of an interface to expose to users. I like things like device_add() better that only creates objects of TYPE_DEVICE that are always in /peripherial. For block, we'd have a similar interface that always created objects of TYPE_BLOCK_DRIVER and put them in /block. > + p = object_resolve_path(parent, NULL); > + if (!p) { > + error_set(errp, QERR_DEVICE_NOT_FOUND, parent); > + return NULL; > + } > + > + type = type_get_by_name(type_name); > + if (!type) { > + error_set(errp, QERR_INVALID_PARAMETER_VALUE, > + "type-name", "a type name"); > + return NULL; > + } > + obj = object_new_with_type(type); > + > + // TODO bombs if p is an interface object; can this happen? All interface types have .abstract set and there's an assert() to validate that an object isn't abstract when creating. So it should be object_new_with_type() that's asserting, not object_property_add_child. We'll probably want to enforce that qom-new isn't given an abstract type by introducing a type_is_abstract() or something like that. Regards, Anthony Liguori > + object_property_add_child(p, prop_name, obj, NULL); > + > + return object_get_canonical_path(obj); > +}