Re: [PATCH v2 2/1] qemu-img: Add "backing":true to unallocated map segments

[Nir Soffer]

On Fri, Jun 11, 2021 at 10:03 PM Eric Blake <eblake@redhat.com> wrote:
>
> To save the user from having to check 'qemu-img info --backing-chain'
> or other followup command to determine which "depth":n goes beyond the
> chain, add a boolean field "backing" that is set only for unallocated
> portions of the disk.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>
> Touches the same iotest output as 1/1.  If we decide that switching to
> "depth":n+1 is too risky, and that the mere addition of "backing":true
> while keeping "depth":n is good enough, then we'd have just one patch,
> instead of this double churn.  Preferences?
>
>  docs/tools/qemu-img.rst    |  3 ++
>  qapi/block-core.json       |  7 ++-
>  qemu-img.c                 | 15 +++++-
>  tests/qemu-iotests/122.out | 34 +++++++-------
>  tests/qemu-iotests/154.out | 96 +++++++++++++++++++-------------------
>  tests/qemu-iotests/179.out | 66 +++++++++++++-------------
>  tests/qemu-iotests/223.out | 24 +++++-----
>  tests/qemu-iotests/244.out |  6 +--
>  tests/qemu-iotests/252.out |  4 +-
>  tests/qemu-iotests/274.out | 16 +++----
>  tests/qemu-iotests/291.out |  8 ++--
>  tests/qemu-iotests/309.out |  4 +-
>  12 files changed, 150 insertions(+), 133 deletions(-)
>
> diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
> index c155b1bf3cc8..fbc623b645c3 100644
> --- a/docs/tools/qemu-img.rst
> +++ b/docs/tools/qemu-img.rst
> @@ -601,6 +601,9 @@ Command description:
>      a ``depth``; for example, a depth of 2 refers to the backing file
>      of the backing file of *FILENAME*.  Depth will be one larger than
>      the chain length if no file in the chain provides the data.
> +  - an optional ``backing`` field is present with value true if no
> +    file in the backing chain provides the data (making it easier to
> +    identify when ``depth`` exceeds the chain length).
>
>    In JSON format, the ``offset`` field is optional; it is absent in
>    cases where ``human`` format would omit the entry or exit with an error.
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 2ea294129e08..cebe12ba16a0 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -264,6 +264,9 @@
>  # @offset: if present, the image file stores the data for this range
>  #          in raw format at the given (host) offset
>  #
> +# @backing: if present, the range is not allocated within the backing
> +#           chain (since 6.1)
> +#
>  # @filename: filename that is referred to by @offset
>  #
>  # Since: 2.6
> @@ -271,8 +274,8 @@
>  ##
>  { 'struct': 'MapEntry',
>    'data': {'start': 'int', 'length': 'int', 'data': 'bool',
> -           'zero': 'bool', 'depth': 'int', '*offset': 'int',
> -           '*filename': 'str' } }
> +           'zero': 'bool', 'depth': 'int', '*backing': 'bool',
> +           '*offset': 'int', '*filename': 'str' } }
>
>  ##
>  # @BlockdevCacheInfo:
> diff --git a/qemu-img.c b/qemu-img.c
> index 33a5cd012b8b..4d357f534803 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -2977,8 +2977,13 @@ static int dump_map_entry(OutputFormat output_format, 
> MapEntry *e,
>          break;
>      case OFORMAT_JSON:
>          printf("{ \"start\": %"PRId64", \"length\": %"PRId64","
> -               " \"depth\": %"PRId64", \"zero\": %s, \"data\": %s",
> -               e->start, e->length, e->depth,
> +               " \"depth\": %"PRId64, e->start, e->length, e->depth);
> +        if (e->has_backing) {
> +            /* Backing should only be set at the end of the chain */
> +            assert(e->backing && e->depth > 0);
> +            printf(", \"backing\": true");
> +        }

It will be easier to inspect the output if common fields come before
optional fields.

> +        printf(", \"zero\": %s, \"data\": %s",
>                 e->zero ? "true" : "false",
>                 e->data ? "true" : "false");
>          if (e->has_offset) {
...
> diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out
> index 779dab4847f0..c5aa2c9866f1 100644
> --- a/tests/qemu-iotests/122.out
> +++ b/tests/qemu-iotests/122.out
> @@ -68,11 +68,11 @@ read 65536/65536 bytes at offset 4194304
>  read 65536/65536 bytes at offset 8388608
>  64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  [{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": true},
> -{ "start": 65536, "length": 4128768, "depth": 1, "zero": true, "data": 
> false},
> +{ "start": 65536, "length": 4128768, "depth": 1, "backing": true, "zero": 
> true, "data": false},

So this output would be:

    [{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": true},
     { "start": 65536, "length": 4128768, "depth": 1, "zero": true,
"data": false, "backing": true},