Re: [Qemu-devel] [PATCH V13 0/6] enhancement for qmp/hmp interfaces of block info

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4845 bytes --]

On 05/24/2013 10:24 PM, Wenchao Xia wrote:

The cover letter doesn't get committed into git; but if you do respin,
it might be worth cleaning up some of the grammar.  I know English is
not your native language, and you're doing a fine job of coping as it
is, but this might help you improve your skills.

>   This serial let qmp interface show delaied info, including internal snapshot

s/serial let/series lets/

not sure what word you meant for 'delaied'

> /backing chain on all block device at runtime, which helps management stack and
> human user, by retrieving exactly the same info of what qemu saws.

s/saws/sees/

> 
> Example:
> -> { "execute": "query-block" }
> <- {
>       "return":[
>          {
>             "io-status": "ok",
>             "device":"ide0-hd0",
>             "locked":false,
>             "removable":false,
>             "inserted":{
>                "ro":false,
>                "drv":"qcow2",
>                "encrypted":false,
>                "file":"disks/test.qcow2",

Here...

>                "backing_file_depth":1,
>                "bps":1000000,
>                "bps_rd":0,
>                "bps_wr":0,
>                "iops":1000000,
>                "iops_rd":0,
>                "iops_wr":0,
>                "image":{
>                   "filename":"disks/test.qcow2",

...and here, it looks like we have redundant information.  But that's
okay; I think we're stuck with preserving backwards compatibility while
still providing the new information in a saner structure.

>                   "format":"qcow2",
>                   "virtual-size":2048000,
>                   "backing_file":"base.qcow2",
>                   "full-backing-filename":"disks/base.qcow2",
>                   "backing-filename-format:"qcow2",

And here's another case of information...

>                   "snapshots":[
>                      {
>                         "id": "1",
>                         "name": "snapshot1",
>                         "vm-state-size": 0,
>                         "date-sec": 10000200,
>                         "date-nsec": 12,
>                         "vm-clock-sec": 206,
>                         "vm-clock-nsec": 30
>                      }
>                   ],
>                   "backing-image":{
>                       "filename":"disks/base.qcow2",
>                       "format":"qcow2",

...that gets repeated later.  Oh well.

> 
>   These patches follows the rule that use qmp to retieve information,

s/retieve/retrieve/

> hmp layer just does a translation from qmp object it got. To make code
> graceful, snapshot and image info retrieving code in qemu and qemu-img are
> merged into block layer, and some function name was adjusted to make it tips

s/name was/names were/

> better. For the part touch by the serial, it works as:

s/make it tips better/describe them better/

s/touch/touched/

s/serial/series/

> 
>    qemu          qemu-img
> 
> dump_monitor    dump_stdout
>      |--------------| 
>             |
>        block/qapi.c
> 
>   Special thanks for Markus, Stefan, Kevin, Eric reviewing many times.
> 
> v13:
>   Renamed the serial as "enhancement for qmp/hmp interfaces of block info".
>   Seperated the common part of code moving and hmp printf as a standalone

s/Seperated/Separated/

> serial, which can be used by both mine and Pavel's work. This serial depend

s/serial/series/

> on it: "[PATCH V3 0/4] qapi and snapshot code clean up in block layer",
> https://lists.gnu.org/archive/html/qemu-devel/2013-05/msg03539.html
>   Removed the VM snapshot info part, since it relate to VM snapshot creating
> logic, which should be changed together with Pavel's serial.

s/serial/series/

>   Address Eric's comments:
>   2/6: bdrv_query_image_info() returns void now, only use *errp to tip error.

s/tip/record/

> 
> Wenchao Xia (6):
>   1 block: add snapshot info query function bdrv_query_snapshot_info_list()
>   2 block: add image info query function bdrv_query_image_info()
>   3 qmp: add recursive member in ImageInfo
>   4 qmp: add ImageInfo in BlockDeviceInfo used by query-block
>   5 hmp: show ImageInfo in 'info block'
>   6 hmp: add parameters device and -v for info block
> 
>  block/qapi.c         |  148 ++++++++++++++++++++++++++++++++++++++++++--------
>  hmp.c                |   21 +++++++
>  include/block/qapi.h |   14 +++--
>  monitor.c            |    7 ++-
>  qapi-schema.json     |   10 +++-
>  qemu-img.c           |   10 +++-
>  qmp-commands.hx      |   69 +++++++++++++++++++++++-
>  7 files changed, 242 insertions(+), 37 deletions(-)
> 
> 
> 
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]