Re: [Qemu-devel] [PATCH v3 13/19] qemu-img: add a "map" subcommand

[Kevin Wolf]

Am 30.07.2013 um 17:22 hat Paolo Bonzini geschrieben:
> Il 30/07/2013 17:13, Kevin Wolf ha scritto:
> > Am 25.07.2013 um 16:23 hat Paolo Bonzini geschrieben:
> >> This command dumps the metadata of an entire chain, in either tabular or 
> >> JSON
> >> format.
> >>
> >> Signed-off-by: Paolo Bonzini <address@hidden>
> > 
> > Hm, we have a 'map' command in qemu-io, which isn't exactly the same,
> > but then not much different either.
> > 
> > Depending on the use cases, should we move this to qemu-io, should we
> > remove the old function from qemu-io, or should we really keep both?
> 
> I would remove the one in qemu-io (but I haven't checked if qemu-iotests
> uses it).
> 
> >> diff --git a/qemu-img.c b/qemu-img.c
> >> index c5c8ebc..b28d388 100644
> >> --- a/qemu-img.c
> >> +++ b/qemu-img.c
> >> @@ -1768,6 +1768,192 @@ static int img_info(int argc, char **argv)
> >>      return 0;
> >>  }
> >>  
> >> +
> >> +typedef struct MapEntry {
> >> +    int flags;
> >> +    int depth;
> >> +    int64_t start;
> >> +    int64_t length;
> >> +    int64_t offset;
> >> +} MapEntry;
> >> +
> >> +static void dump_map_entry(OutputFormat output_format, MapEntry *e,
> >> +                           MapEntry *next)
> >> +{
> >> +    switch (output_format) {
> >> +    case OFORMAT_HUMAN:
> >> +        if ((e->flags & BDRV_BLOCK_DATA) &&
> >> +            !(e->flags & BDRV_BLOCK_OFFSET_VALID)) {
> >> +            error_report("File contains external, encrypted or compressed 
> >> clusters.");
> >> +            exit(1);
> >> +        }
> >> +        if ((e->flags & (BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO)) == 
> >> BDRV_BLOCK_DATA) {
> >> +            printf("%"PRId64" %"PRId64" %d %"PRId64"\n",
> >> +                   e->start, e->length, e->depth, e->offset);
> > 
> > Is this really human-readable output?
> 
> I will change it to use tabs and add a heading line.

The documentation patch contains a line like this:

0       131072       2        327680

A heading line and tabs (or even better, fixed printf column widths)
sounds good, but I think if it's really only for human users and not for
shell scripts, we can further improve the output:

Offset      Length      Mapped to       File

0         + 128k     -> 320k            /tmp/backing.qcow2
128k      + 256k     -> 2M              /tmp/overlay.qcow2

We could add another exact, more technical format like this, which could
leave out things like + and -> so that it is easier to parse from shell
scripts, too:

Offset      Length      Mapped to       Depth   File

        0    0x20000       0x50000      1       /tmp/backing.qcow2
  0x20000    0x40000      0x200000      0       /tmp/overlay.qcow2

What do you think?

Kevin