Re: [Qemu-devel] [PATCH v11 3/9] qemu-io: Switch 'map' output to byte-based reporting

[Max Reitz]

On 29.04.2017 21:14, Eric Blake wrote:
> Mixing byte offset and sector allocation counts is a bit
> confusing.  Also, reporting n/m sectors, where m decreases
> according to the remaining size of the file, isn't really
> adding any useful information; and reporting an offset at
> both the front and end of the line, with large amounts of
> whitespace, is pointless.  Update the output to use byte
> counts and shorter lines, then adjust the affected tests
> (./check -qcow2 102, ./check -vpc 146).
> 
> Note that 'qemu-io map' is MUCH weaker than 'qemu-img map';
> the former only shows which regions of the active layer are
> allocated, without regards to where the allocation comes from
> or whether the allocated portion is known to read as zero
> (because it is using the weaker bdrv_is_allocated()); while the
> latter (especially in --output=json mode) reports more details
> from bdrv_get_block_status().
> 
> Signed-off-by: Eric Blake <address@hidden>
> 
> ---
> v11: rebase to occur before test 179, shrink the output
> v10: rebase to updated test 179
> v9: new patch
> ---
>  qemu-io-cmds.c             | 11 ++++++-----
>  tests/qemu-iotests/102.out |  4 ++--
>  tests/qemu-iotests/146.out | 30 +++++++++++++++---------------
>  3 files changed, 23 insertions(+), 22 deletions(-)

Reviwed-by: Max Reitz <address@hidden>

> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 1e0ebb4..4b2278f 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c

[...]

> @@ -1894,10 +1894,11 @@ static int map_f(BlockBackend *blk, int argc, char 
> **argv)
>          }
> 
>          retstr = ret ? "    allocated" : "not allocated";
> -        cvtstr(offset << 9ULL, s1, sizeof(s1));
> -        printf("[% 24" PRId64 "] % 8" PRId64 "/% 8" PRId64 " sectors %s "
> -               "at offset %s (%d)\n",
> -               offset << 9ULL, num, nb_sectors, retstr, s1, ret);
> +        cvtstr(num << BDRV_SECTOR_BITS, s1, sizeof(s1));
> +        cvtstr(offset << BDRV_SECTOR_BITS, s2, sizeof(s2));
> +        printf("%s (0x%" PRIx64 ") bytes %s at offset %s (0x%" PRIx64 ")\n",

By the way, %# " PRIx64 " would work, too.


Just wanted to get out my kind-of-obscure printf knowledge.

Sorry.

> +               s1, num << BDRV_SECTOR_BITS, retstr,
> +               s2, offset << BDRV_SECTOR_BITS);
> 
>          offset += num;
>          nb_sectors -= num;

signature.asc
Description: OpenPGP digital signature