Re: [Qemu-devel] [PATCH RFC 2/2] qmp: New command qom-new

[Anthony Liguori]

On 05/24/2012 08:48 AM, Igor Mammedov wrote:
> On 05/24/2012 03:04 PM, Anthony Liguori wrote:
> > 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<container>"}, {"name":
> > > "i440fx", "type": "child<i440FX-pcihost>"}, {"name": "unattached",
> > > "type": "child<container>"}, {"name": "peripheral", "type":
> > > "child<container>"}, {"name": "peripheral-anon", "type": "child<container>"}]}
> > >
> > > 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<address@hidden>
> > > ---
> > > 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.
>
> Will we have a special cases for every incompatible device types that is going
> to be hot-plugged via device_add monitor command?
>
> For CPUs my thoughts were moving in opposite direction, like:
> - make possible to create and initialize CPU as a regular QOM object

I think we should make CPUs a TYPE_DEVICE.  That hasn't been possible yet but 
with the bus conversion series, it will be.

Regards,

Anthony Liguori