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 <pbonz...@redhat.com>
>
> 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.
>> + }
>> + /* This format ignores the distinction between 0, ZERO and
>> ZERO|DATA.
>> + * Modify the flags here to allow more coalescing.
>> + */
>> + if (next &&
>> + (next->flags & (BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO)) !=
>> BDRV_BLOCK_DATA) {
>> + next->flags &= ~BDRV_BLOCK_DATA;
>> + next->flags |= BDRV_BLOCK_ZERO;
>> + }
>> + break;
>> + case OFORMAT_JSON:
>> + printf("%s{ 'start': %"PRId64", 'length': %"PRId64", 'depth': %d, "
>> + "'zero': %s, 'data': %s",
>> + (e->start == 0 ? "[" : ",\n"),
>> + e->start, e->length, e->depth,
>> + (e->flags & BDRV_BLOCK_ZERO) ? "true" : "false",
>> + (e->flags & BDRV_BLOCK_DATA) ? "true" : "false");
>
> Correct JSON uses double quotes.
Will fix.
>> + if (e->flags & BDRV_BLOCK_OFFSET_VALID) {
>> + printf(", 'offset': %"PRId64"", e->offset);
>> + }
>> + putchar('}');
>> +
>> + if (!next) {
>> + printf("]\n");
>> + }
>> + break;
>> + }
>> +}
>> +
>> +static int64_t get_block_status(BlockDriverState *bs, int64_t sector_num,
>> + int *nb_sectors, int *depth)
>> +{
>> + int64_t ret;
>> +
>> + /* As an optimization, we could cache the current range of unallocated
>> + * clusters in each file of the chain, and avoid querying the same
>> + * range repeatedly.
>> + */
>> +
>> + *depth = 0;
>> + for (;;) {
>> + int orig_nb_sectors = *nb_sectors;
>> +
>> + ret = bdrv_get_block_status(bs, sector_num, *nb_sectors,
>> nb_sectors);
>> + if (ret < 0) {
>> + return ret;
>> + }
>> + if (ret & (BDRV_BLOCK_ZERO|BDRV_BLOCK_DATA)) {
>> + return ret;
>> + }
>> + if (!*nb_sectors) {
>> + /* Beyond the end of this image. The extra data is read as
>> zeroes.
>> + * We're in the range of the BlockDriverState above this one, so
>> + * adjust depth.
>> + */
>> + *nb_sectors = orig_nb_sectors;
>> + (*depth)--;
>> + return BDRV_BLOCK_ZERO;
>
> If you implement my suggestion that bdrv_co_get_block_status() returns
> BDRV_BLOCK_ZERO instead of 0 if the image has no backing file, it might
> also make sense to put this check there and return BDRV_BLOCK_ZERO even
> if it has a backing file, but we're after its end.
Good idea. I'll make this change in a separate patch.
>> + }
>> +
>> + bs = bs->backing_hd;
>> + if (bs == NULL) {
>> + return 0;
>> + }
>> +
>> + (*depth)++;
>> + }
>> +}
>> +
>> +static int img_map(int argc, char **argv)
>> +{
>> + int c;
>> + OutputFormat output_format = OFORMAT_HUMAN;
>> + BlockDriverState *bs;
>> + const char *filename, *fmt, *output;
>> + int64_t length;
>> + MapEntry curr = { .length = 0 }, next;
>> +
>> + fmt = NULL;
>> + output = NULL;
>> + for (;;) {
>> + int option_index = 0;
>> + static const struct option long_options[] = {
>> + {"help", no_argument, 0, 'h'},
>> + {"format", required_argument, 0, 'f'},
>> + {"output", required_argument, 0, OPTION_OUTPUT},
>> + {0, 0, 0, 0}
>> + };
>> + c = getopt_long(argc, argv, "f:h",
>> + long_options, &option_index);
>> + if (c == -1) {
>> + break;
>> + }
>> + switch (c) {
>> + case '?':
>> + case 'h':
>> + help();
>> + break;
>> + case 'f':
>> + fmt = optarg;
>> + break;
>> + case OPTION_OUTPUT:
>> + output = optarg;
>> + break;
>> + }
>> + }
>> + if (optind >= argc) {
>> + help();
>> + }
>> + filename = argv[optind++];
>> +
>> + if (output && !strcmp(output, "json")) {
>> + output_format = OFORMAT_JSON;
>> + } else if (output && !strcmp(output, "human")) {
>> + output_format = OFORMAT_HUMAN;
>> + } else if (output) {
>> + error_report("--output must be used with human or json as
>> argument.");
>> + return 1;
>> + }
>> +
>> + bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS, true, false);
>> + if (!bs) {
>> + return 1;
>> + }
>> +
>> + length = bdrv_getlength(bs);
>
> bdrv_getlength() can fail.
I'll make it always use bs->total_sectors instead of fixing this.
>> + while (curr.start + curr.length < length) {
>> + int64_t nsectors_left, ret;
>> + int64_t sector_num;
>> + int n, depth, flags;
>> +
>> + /* Probe up to 1 G at a time. */
>> + sector_num = (curr.start + curr.length) >> BDRV_SECTOR_BITS;
>> + nsectors_left = DIV_ROUND_UP(length, BDRV_SECTOR_SIZE) - sector_num;
>> + n = MIN(1 << (30 - BDRV_SECTOR_BITS), nsectors_left);
>> + ret = get_block_status(bs, sector_num, &n, &depth);
>> +
>> + if (ret < 0) {
>> + error_report("Could not read file metadata: %s",
>> strerror(-ret));
>> + return 1;
>
> This leaks bs.
Right.
Paolo
>> + }
>> +
>> + flags = ret & ~BDRV_BLOCK_OFFSET_MASK;
>> + ret &= BDRV_BLOCK_OFFSET_MASK;
>> + if (curr.length == 0 || curr.flags != flags || curr.depth != depth
>> ||
>> + ((curr.flags & BDRV_BLOCK_OFFSET_VALID) &&
>> + curr.offset + curr.length != ret)) {
>> + next.flags = flags;
>> + next.depth = depth;
>> + next.start = sector_num << BDRV_SECTOR_BITS;
>> + next.offset = ret;
>> + next.length = 0;
>> + if (curr.length > 0) {
>> + dump_map_entry(output_format, &curr, &next);
>> + }
>> + curr = next;
>> + }
>> + curr.length += n << BDRV_SECTOR_BITS;
>> + }
>> +
>> + dump_map_entry(output_format, &curr, NULL);
>> + return 0;
>> +}
>> +
>> #define SNAPSHOT_LIST 1
>> #define SNAPSHOT_CREATE 2
>> #define SNAPSHOT_APPLY 3
>
> Kevin
>
>
