Re: [Qemu-devel] [RFC] qapi: New command query-mtree

[Marc Marí]

El Wed, 20 Aug 2014 13:09:20 -0600
Eric Blake <address@hidden> escribió:
> On 08/20/2014 11:46 AM, Marc Marí wrote:
> > Add command query-mtree to get the memory tree of the guest.
> > 
> > As we were looking for a flexible solution on accessing the guest
> > memory from qtests, Stefan came with the idea to implement this new
> > qmp command.
> > 
> > This way, the result can be parsed, and the RAM direction
> > extracted, so only a generic qtest malloc is necessary and not one
> > per machine, as it is implemented at the moment (malloc-pc uses
> > fw_cfg).
> > 
> > The actual output is this: http://pastebin.com/nHAH9Jie
> > Which corresponds to this info mtree: http://pastebin.com/B5vw8DDf
> 
> Alas, these pastebins will expire, even when we still read qemu.git
> many years from now.  If it weren't for the fact that 116 lines of
> mtree is exploding into 1033 lines of prettified JSON, I'd suggest
> putting it here in the commit message.  Or if you can come up with a
> simpler testcase, or summarize a sample section here, even that would
> be nicer than pointing to what will eventually be a stale URL.

In the final commit message I was thinking in dropping the "why" and the
examples. I put those just for the RFC.

> 
> > +##
> > +{ 'type': 'MemTree', 'data': {'spaces': ['AddrSpace'],
> > +                                'aliases': ['AddrSpace']} }
> 
> Whitespace doesn't matter, but consistent alignment would look better.
> 
> > +
> > +##
> > +# @query-mtree:
> > +#
> > +# Return the memory distribution of the guest
> > +#
> > +# Returns: a list of @AddrSpace
> > +#
> > +# Since: 2.x
> 
> s/2.x/2.2/
> 
> > +##
> > +{ 'command': 'query-mtree', 'returns': 'MemTree' }
> 
> I was expecting ['MemTree'] (an array of structs), but you gave
> 'MemTreee' (a struct of parallel arrays).  Am I guaranteed that
> returns.spaces and returns.aliases are arrays of the same length?  If
> not, what's the difference between 'spaces' and 'aliases' (that is,
> why do I need two arrays)?  Should the designation of 'space' vs.
> 'alias' be part of the 'AddrSpace' struct, rather than having to be
> learned indirectly by which of the two arrays it appeared in?
> 

I put it this way to do it as similar as possible as "info mtree" where
there are two sections, "alias" and "the other". But this could be just
specified inside AddrSpace, and return a ['AddrSpace'] or change its
name to ['MemTree']

> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index 4be4765..22f91b0 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -3755,3 +3755,22 @@ Example:
> >  <- { "return": {} }
> >  
> >  EQMP
> > +    {
> > +        .name       = "query-mtree",
> > +        .args_type  = "",
> > +        .mhandler.cmd_new = qmp_marshal_input_query_mtree,
> > +    },
> > +SQMP
> > +query-mtree
> > +---------
> > +
> > +Memory tree.
> > +
> > +The returned value is a json-array of the memory distribution of
> > the system.
> 
> Docs don't match your qapi schema.  Let's make sure we get the schema
> correct, and then make the docs match. 

I changed the schema after writing the docs, and I didn't update the
docs.

> > +Each address space is represented by a json-object, which has a
> > name, and a +json-array of all memory regions that contain.
> 
> that contain what?  Did you mean "that it contains"?

Yes.

Thanks for your comments
Marc