Re: [Qemu-devel] [PATCH 1/2] QMP: Introduce commands doc

[Luiz Capitulino <lcapitulino@redhat.com>]

On Wed, 12 May 2010 18:48:38 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> > +query-block
> > +-----------
> > +
> > +Show the block devices.
> > +
> > +Each block device information is stored in a json-object and the returned value
> > +is a json-array of all devices.
> > +
> > +Each json-object contain the following:
> > +
> > +- "device": device name (json-string)
> > +- "type": device type (json-string)
> 
> Possible values?  "hd", "cdrom", "floppy".  Code also has "unknown", but
> when it uses that, it's badly broken.

 Yes, but you didn't mean we shouldn't use 'unknown', right?

> > +- "removable": true if the device is removable, false otherwise (json-bool)
> > +- "locked": true if the device is locked, false otherwise (json-bool)
> > +- "inserted": only present if the device is inserted, it is a json-object
> > +   containing the following:
> > +         - "file": device file name (json-string)
> > +         - "ro": true if read-only, false otherwise (json-bool)
> > +         - "drv": driver format name (json-string)
> 
> Possible values?

 I got the following list by grepping the code. Kevin, can you confirm it's
correct?

 "blkdebug", "bochs", "cloop", "cow", "dmg", "file", "file", "ftp", "ftps",
 "host_cdrom", "host_cdrom", "host_device", "host_device", "host_floppy",
 "http", "https", "nbd", "parallels", "qcow", "qcow2", "raw", "tftp", "vdi",
 "vmdk", "vpc", "vvfat"

> > +query-cpus
> > +----------
> > +
> > +Show CPU information.
> > +
> > +Return a json-array. Each CPU is represented by a json-object, which contains:
> > +
> > +- "cpu": CPU index (json-int)
> 
> It's actually upper case (see example below).  I hate that.

 Hm, this one leaked.. But it's quite minor anyway.

> > +- "current": true if this is the current CPU, false otherwise (json-bool)
> > +- "halted": true if the cpu is halted, false otherwise (json-bool)
> > +- Current program counter. The key's name depends on the architecture:
> > +     "pc": i386/x86_64 (json-int)
> > +     "nip": PPC (json-int)
> > +     "pc" and "npc": sparc (json-int)
> > +     "PC": mips (json-int)
> 
> Key name depending on arch: isn't that an extraordinarily bad idea?

 Honestly, I don't think it's that bad: it's a form of optional key,
but if you think it's so bad I can add a "arch" key with the arch's name.

 Don't think anyone is using this, anyway.

> > +query-migrate
> > +-------------
> > +
> > +Migration status.
> > +
> > +Return a json-object. If migration is active there will be another json-object
> > +with RAM migration status and if block migration is active another one with
> > +block migration status.
> > +
> > +The main json-object contains the following:
> > +
> > +- "status": migration status (json-string)
> 
> Possible values?  "active", "completed", "failed", "cancelled".  Note
> there's no value for "never attempted"; see below.
> 
> > +- "ram": only present if "status" is "active", it is a json-object with the
> > +  following RAM information (in bytes):
> > +         - "transferred": amount transferred (json-int)
> > +         - "remaining": amount remaining (json-int)
> > +         - "total": total (json-int)
> > +- "disk": only present if "status" is "active" and it is a block migration,
> > +  it is a json-object with the following disk information (in bytes):
> > +         - "transferred": amount transferred (json-int)
> > +         - "remaining": amount remaining (json-int)
> > +         - "total": total (json-int)
> 
> Before the first migration, we actually reply with
> 
> {"return": {}}
> 
> which contradicts the doc.

 Good catch, what would be the best behavior here?

> > +Example:
> > +
> > +{
> > +   "return":[
> > +      {
> > +         "bus":0,
> > +         "devices":[
> > +            {
> > +               "bus":0,
> > +               "qdev_id":"",
> > +               "slot":0,
> > +               "class_info":{
> > +                  "class":1536,
> > +                  "desc":"Host bridge"
> > +               },
> > +               "id":{
> > +                  "device":32902,
> > +                  "vendor":4663
> > +               },
> > +               "function":0,
> > +               "regions":[
> > +
> > +               ]
> 
> Suggest to simply write
> 
>                   "regions":[]

 I could, and I agree it's better, but I'm using a formatting tool, so
editing by hand would be a PITA.

> > +Note: This example has been shortened as the real response is too long.
> 
> Actually, I get a shorter response for my minimal guest: just slots
> 0..3.  Suggest to omit slot 4 and this note.

 What's the command-line for it?

> > +query-vnc
> > +---------
> > +
> > +Show VNC server information.
> > +
> > +Return a json-object with server information. Connected clients are returned
> > +as a json-array of json-objects.
> > +
> > +The main json-object contains the following:
> > +
> > +- "enabled": true or false (json-bool)
> > +- "host": server's IP address (json-string)
> 
> Wouldn't hurt to mention it can be a wildcard address.  The example
> below shows the IPv4 wildcard address "0.0.0.0".
> 
> > +- "family": address family (json-string, "ipv4" or "ipv6")
> 
> inet_strfamily() can also return "unix" and "unknown".
> 
> By the way, we use PF_INET6, PF_INET and PF_UNIX there.  To be
> pedantically correct, we should use AF_INET6, AF_INET and AF_LOCAL.
> 
> > +- "service": server's port number (json-string)
> 
> Why isn't this json-int?

 Because getnameinfo() returns a string and I didn't bother using it,
do you?