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

[Avi Kivity]

On 05/05/2010 10:11 PM, Luiz Capitulino wrote:
> One of the most important missing feature in QMP today is its
> supported commands documentation.
>
> The plan is to make it part of self-description support, however
> self-description is a big task we have been postponing for a
> long time now and still don't know when it's going to be done.
>
> In order not to compromise QMP adoption and make users' life easier,
> this commit adds a simple text documentation which fully describes
> all QMP supported commands.
>
> This is not ideal for a number of reasons (harder to maintain,
> text-only, etc) but does improve the current situation.
>
>    

Excellent.

> +change
> +------
> +
> +Change a removable medium or VNC configuration.
>    

This is sad.  Would anyone write a C function with a similar description?

Do we have this in 0.12?  If not we can change (...) it.  If we do, it's 
a more difficult deprecation process.

> +cpu
> +---
> +
> +Set the default CPU.
> +
> +Arguments:
> +
> +- "index": the CPU's index (json-int)
> +
> +Example:
> +
> +{ "execute": "cpu", "arguments": { "index": 0 } }
> +
> +Note: CPUs' indexes are obtained with the 'query-cpus' command.
>    

The default cpu is local to the monitor?

Not sure, but it may have been better to omit it and instead require an 
explicit cpu argument in all commands that rely on it.  As it is, 
multithreaded management programs will need to lock the monitor when 
reading cpu registers, etc.

Or, we can add an optional cpu argument to all commands that depend on it.

> +
> +memsave
> +-------
> +
> +Save to disk virtual memory dump starting at 'val' of size 'size'.
> +
> +Arguments:
> +
> +- "val": the starting address (json-int)
> +- "size": the memory size (json-int)
> +- "filename": file path (json-string)
> +
> +Example:
> +
> +{ "execute": "memsave",
> +             "arguments": { "val": 10,
> +                            "size": 100,
> +                            "filename": "/tmp/virtual-mem-dump" } }
>    

Suggest a note that this depends on the current cpu.

> +
> +migrate
> +-------
> +
> +Migrate to URI.
> +
> +Arguments:
> +
> +- "blk": block migration, full disk copy (json-bool, optional)
> +- "inc": incremental disk copy (json-bool, optional)
>    

Don't these need to be per-disk?

> +- "uri": Destination URI (json-string)
> +
> +Example:
> +
> +{ "execute": "migrate", "arguments": { "uri": "tcp:0:4446" } }
> +
> +Notes:
> +
> +(1) The 'query-migrate' command should be used to check migration's progress
> +    and final result (this information is provided by the 'status' member)
>    

Is there an event the provides completion notification?  I don't see one 
in qemu-events.txt, but I think there should be one.

> +(2) All boolean arguments default to false
> +(3) The user Monitor's "detach" argument is invalid in QMP and should not
> +    be used
> +
> +
> +migrate_set_downtime
> +--------------------
> +
> +Set maximum tolerated downtime (in seconds) for migrations.
> +
> +Arguments:
> +
> +- "value": maximum downtime (json-number)
> +
> +Example:
> +
> +{ "execute": "migrate_set_downtime", "arguments": { "value": 60 } }
>    

The example doesn't match reality well, suggest 0.1.

Would have been nicer as migrate_set_parameters downtime: 0.1, we can do 
that when we add more parameters.

> +
> +migrate_set_speed
> +-----------------
> +
> +Set maximum speed for migrations.
> +
> +Arguments:
> +
> +- "value": maximum speed (json-number)
> +
> +Example:
> +
> +{ "execute": "migrate_set_speed", "arguments": { "value": 1024 } }
>    

Oh, we do have more.

Please document the units for this value (bits per second)?

> +pmemsave
> +--------
> +
> +Save to disk physical memory dump starting at 'val' of size 'size'.
> +
> +Arguments:
> +
> +- "val": the starting address (json-int)
>    

Why "val" instead of "address" or "physical-address"?

> +- "size": the memory size (json-int)
>    

In bytes.

> +- "filename": file path (json-string)
> +
> +Example:
> +
> +{ "execute": "pmemsave",
> +             "arguments": { "val": 10,
> +                            "size": 100,
> +                            "filename": "/tmp/physical-mem-dump" } }
>    

It would be nice to have non-file versions of memsave and pmemsave.

> +
> +screendump
> +----------
> +
> +Save screen into PPM image.
> +
> +Arguments:
> +
> +- "filename": file path (json-string)
> +
> +Example:
> +
> +{ "execute": "screendump", "arguments": { "filename": "/tmp/image" } }
>    

Wish:  , "format": "png"

> +query-blockstats
> +----------------
> +
> +Show block device statistics.
> +
> +Each device statistic information is stored in a json-object and the returned
> +value is a json-array of all devices.
>    

Future enhancement: request a summary, or stats for a list of devices.

> +
> +query-commands
> +--------------
> +
> +List QMP available commands.
> +
> +Each command is represented by a json-object, the returned value is a 
> json-array
> +of all commands.
> +
> +Each json-object contain:
> +
> +- "name": command's name (json-string)
> +
> +Example:
> +
> +{
> +   "return":[
> +      {
> +         "name":"query-balloon"
> +      },
> +      {
> +         "name":"system_powerdown"
> +      }
> +   ]
> +}
> +
> +Note: This example has been shortened as the real response is too long.
>    

Presumably, this will be extended with a description of the arguments 
and return types?

> +
> +query-cpus
> +----------
> +
> +Show CPU information.
> +
> +Return a json-array. Each CPU is represented by a json-object, which contains:
> +
> +- "cpu": CPU index (json-int)
> +- "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)
>    

That's a bit sad.  It makes it difficult to use with stub generators.

Can we force everything to "pc"?  It will break non-x86, but there 
aren't likely to be many QMP users using for those archs at present.

> +
> +query-mice
> +----------
> +
> +Show VM mice information.
> +
> +Each mouse is represented by a json-object, the returned value is a json-array
> +of all mice.
> +
> +The mouse json-object contains the following:
> +
> +- "name": mouse's name (json-string)
> +- "index": mouse's index (json-int)
> +- "current": true if this mouse is receiving events, false otherwise 
> (json-bool)
>    

What does this mean?

> +- "absolute": true if the mouse generates absolute input events (json-bool)
> +
>
> +
> +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)
>    

Please list all possible values here (what's the status before the first 
migration is started?

> +query-pci
> +---------
> +
> +PCI buses and devices information.
> +
> +The returned value is a json-array of all buses. Each bus is represented by
> +a json-object, which has a key with a json-array of all PCI devices attached
> +to it. Each device is represented by a json-object.
> +
> +The bus json-object contains the following:
> +
> +- "bus": bus number (json-int)
> +- "devices": a json-array of json-objects, each json-object represents a
> +             PCI device
> +
> +The PCI device json-object contains the following:
> +
> +- "bus": identical to the parent's bus number (json-int)
> +- "slot": slot number (json-int)
> +- "function": function number (json-int)
>    

Would have been nicer as a nested object (list of buses, each containing 
a list of slots, each containing a list of functions).

> +- "class_info": a json-object containing:
> +     - "desc": device class description (json-string, optional)
> +     - "class": device class number (json-int)
> +- "id": a json-object containing:
> +     - "device": device ID (json-int)
> +     - "vendor": vendor ID (json-int)
> +- "irq": device's IRQ if assigned (json-int, optional)
>    

Please note here that the OS may ignore it, so may not be authoritative.

> +- "qdev_id": qdev id string (json-string)
> +- "pci_bridge": It's a json-object, only present if this device is a
> +                PCI bridge, contains:
> +     - "bus": bus number (json-int)
> +     - "secondary": secondary bus number (json-int)
> +     - "subordinate": subordinate bus number (json-int)
> +     - "io_range": a json-object with memory range information (json-int)
>    

Object or int?

> +     - "memory_range": a json-object with memory range information (json-int)
>    
Object or int?

> +     - "prefetchable_range": a json-object with memory range
> +                             information (json-int)
>    

Format not understood.

> +     - "devices": a json-array of PCI devices if there's any attached 
> (optional)
>    

What is the format of an array element?

Very nice overall.  It is *way* easier to review documentation than 
piles of code, so I suggest starting with documentation from now on.  
I've adopted this approach in kvm.

--
error compiling committee.c: too many arguments to function