Re: [Qemu-devel] [PATCH 2/3] QMP: Introduce commands documentation

[Luiz Capitulino]

On Mon, 31 May 2010 21:22:22 +0100
Stefan Hajnoczi <address@hidden> wrote:

> On Mon, May 31, 2010 at 6:43 PM, Luiz Capitulino <address@hidden> wrote:
> Hi Luiz,
> 
> I'm interested in QMP, have left some feedback.  As I get up to speed
> with QMP my questions and suggestions will hopefully be useful.
> Apologies in advance if any of my thoughts here are old news and have
> been discussed elsewhere :).

 No problem.

> I didn't see documentation on the errors that a command can encounter.
>  Error documentation would be useful for commands where the client
> needs to react by trying an alternative command.  If errors are opaque
> or undocumented, then the client can't perform error recovery, they
> can only log the error or prompt the user for help.

 The immediate need is to have something which is minimally useful,
we're going to describe errors later.

 Actually, ultimate goal is to have self-description support, which should
describe everything (commands, arguments, errors, async messages, etc). But
I'm wondering whether we're going to have this on time for 0.13.

> > @@ -113,6 +184,35 @@ Password: ********
> >
> > address@hidden table
> >  ETEXI
> > +SQMP
> > +change
> > +------
> > +
> > +Change a removable medium or VNC configuration.
> > +
> > +Arguments:
> > +
> > +- "device": device name (json-string)
> > +- "target": filename or item (json-string)
> > +- "arg": additional argument (json-string, optional)
> > +
> > +Examples:
> > +
> > +1. Change a removable medium
> > +
> > +-> { "execute": "change",
> > +             "arguments": { "device": "ide1-cd0",
> > +                            "target": 
> > "/srv/images/Fedora-12-x86_64-DVD.iso" } }
> > +<- { "return": {} }
> > +
> > +2. Change VNC password
> > +
> > +-> { "execute": "change",
> > +             "arguments": { "device": "vnc", "target": "password",
> > +                            "arg": "foobar1" } }
> > +<- { "return": {} }
> > +
> > +EQMP
> >
> >     {
> >         .name       = "screendump",
> 
> For removable media is there a way to use, for instance, sheepdog, or
> is the target strictly a file name?

 Good question, hey, aren't you a block layer guy? :)

 Anyway, this command has the same behavior of HMP's change command.

> Can the VNC password be removed when arg is the empty string or arg is
> not present?

 Yes, when arg is the empty string the password seems to be removed, which
is an ill behavior.

 Anyway, the plan is to drop this command from QMP and introduce
change_blockdev & change_password.

> > @@ -532,6 +747,24 @@ STEXI
> > address@hidden cpu
> >  Set the default CPU.
> >  ETEXI
> > +SQMP
> > +cpu
> > +---
> > +
> > +Set the default CPU.
> > +
> > +Arguments:
> > +
> > +- "index": the CPU's index (json-int)
> > +
> > +Example:
> > +
> > +-> { "execute": "cpu", "arguments": { "index": 0 } }
> > +<- { "return": {} }
> > +
> > +Note: CPUs' indexes are obtained with the 'query-cpus' command.
> > +
> > +EQMP
> >
> >     {
> >         .name       = "mouse_move",
> 
> I believe this is the per-monitor default cpu for memory dumping and
> other per-cpu commands.  The concept of the default cpu makes sense
> for an interactive monitor, humans don't want to type the cpu index
> for each cpu-specific command.  For QMP it feels cleaner to have a cpu
> argument for commands that are per-cpu.  As a newcomer I'm not sure of
> the philosophy for QMP commands, 1:1 with monitor commands or as
> appropriate for a JSON interface?

 The latter and you're right about the cpu command, Avi has also
pointed this out.

> If the QMP cpu command is supported in a QEMU release, can it be
> phased out later?

 No, we have to fix all this stuff before 0.13 is out, otherwise we'll
have to support them forever. Actually, we'll probably have some kind of
deprecation process, even then we'll have to support bad commands for
long time.