Re: [Qemu-devel] [RFC 1/6] qapi: Extract QAPI block and common definitions.

[Eric Blake]

On 05/28/2014 08:57 AM, Benoît Canet wrote:

Please don't use trailing '.' in the subject line.

> qapi/common.json contains the definitions required by all the json modules.
> 
> qapi/block-core.json contains the block QAPI part unrelated to system
> emulation.
> 
> qapi/block.json is a superset of qapi/block-core.json and contains all block
> QAPI definitions required for system emulation.
> 
> The purpose of the patch is to be able to generate C sources and headers from
> qapi/block-core.json and link their object file against qemu-nbd.
> 
> Signed-off-by: Benoit Canet <address@hidden>
> ---
>  qapi-schema.json     | 1881 
> +++-----------------------------------------------
>  qapi/block-core.json | 1447 ++++++++++++++++++++++++++++++++++++++
>  qapi/block.json      |  166 +++++
>  qapi/common.json     |   88 +++
>  4 files changed, 1801 insertions(+), 1781 deletions(-)
>  create mode 100644 qapi/block-core.json
>  create mode 100644 qapi/block.json
>  create mode 100644 qapi/common.json

This email is heavy, weighing in at 129 kB.  It would greatly ease
review to break this into several smaller pieces, one patch per new
file, and separate code motion from cleanups.  That way, I can QUICKLY
validate the code motion by post-processing each patch and looking at
only what lines weren't pure motion:

$ diff -u <(sed -n 's/^-//p' patch) <(sed -n 's/^\+//p' patch) |less

Doing this now shows places where you mixed in cleanups (for example:

 #          or BLOCK_JOB_ERROR)
 #
 # @enospc: same as @stop on ENOSPC, same as @report otherwise.
+#
 # @stop: for guest operations, stop the virtual machine;
 #        for jobs, pause the job
+#
 # Since: 1.3
+##
 { 'enum': 'BlockdevOnError',

and also gets confusing to read because there is no linear
correspondence between deletions in one file compared to additions in
another.

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

signature.asc
Description: OpenPGP digital signature