[Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Stefan Hajnoczi]

The qemu-img info --backing-chain option enumerates the backing file
chain.  For example, for base.qcow2 <- snap1.qcow2 <- snap2.qcow2 the
output becomes:

  $ qemu-img info --backing-chain snap2.qcow2
  image: snap2.qcow2
  file format: qcow2
  virtual size: 100M (104857600 bytes)
  disk size: 196K
  cluster_size: 65536
  backing file: snap1.qcow2
  backing file format: qcow2

  image: snap1.qcow2
  file format: qcow2
  virtual size: 100M (104857600 bytes)
  disk size: 196K
  cluster_size: 65536
  backing file: base.qcow2
  backing file format: qcow2

  image: base.qcow2
  file format: qcow2
  virtual size: 100M (104857600 bytes)
  disk size: 136K
  cluster_size: 65536

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qemu-img.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 76 insertions(+), 22 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index f17f187..c717f3e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1249,7 +1249,10 @@ static void dump_human_image_info(ImageInfo *info)
     }
 }
 
-enum {OPTION_OUTPUT = 256};
+enum {
+    OPTION_OUTPUT = 256,
+    OPTION_BACKING_CHAIN = 257,
+};
 
 typedef enum OutputFormat {
     OFORMAT_JSON,
@@ -1260,7 +1263,9 @@ static int img_info(int argc, char **argv)
 {
     int c;
     OutputFormat output_format = OFORMAT_HUMAN;
-    const char *filename, *fmt, *output;
+    bool chain = false;
+    const char *output;
+    char *filename, *fmt;
     BlockDriverState *bs;
     ImageInfo *info;
 
@@ -1272,6 +1277,7 @@ static int img_info(int argc, char **argv)
             {"help", no_argument, 0, 'h'},
             {"format", required_argument, 0, 'f'},
             {"output", required_argument, 0, OPTION_OUTPUT},
+            {"backing-chain", no_argument, 0, OPTION_BACKING_CHAIN},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, "f:h",
@@ -1285,17 +1291,20 @@ static int img_info(int argc, char **argv)
             help();
             break;
         case 'f':
-            fmt = optarg;
+            fmt = g_strdup(optarg);
             break;
         case OPTION_OUTPUT:
             output = optarg;
             break;
+        case OPTION_BACKING_CHAIN:
+            chain = true;
+            break;
         }
     }
     if (optind >= argc) {
         help();
     }
-    filename = argv[optind++];
+    filename = g_strdup(argv[optind++]);
 
     if (output && !strcmp(output, "json")) {
         output_format = OFORMAT_JSON;
@@ -1303,31 +1312,76 @@ static int img_info(int argc, char **argv)
         output_format = OFORMAT_HUMAN;
     } else if (output) {
         error_report("--output must be used with human or json as argument.");
-        return 1;
+        goto err;
     }
 
-    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING, false);
-    if (!bs) {
-        return 1;
+    if (chain && output_format == OFORMAT_JSON) {
+        printf("[\n");
     }
 
-    info = g_new0(ImageInfo, 1);
-    collect_image_info(bs, info, filename, fmt);
+    do {
+        bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING,
+                           false);
+        if (!bs) {
+            goto err;
+        }
 
-    switch (output_format) {
-    case OFORMAT_HUMAN:
-        dump_human_image_info(info);
-        dump_snapshots(bs);
-        break;
-    case OFORMAT_JSON:
-        collect_snapshots(bs, info);
-        dump_json_image_info(info);
-        break;
-    }
+        info = g_new0(ImageInfo, 1);
+        collect_image_info(bs, info, filename, fmt);
 
-    qapi_free_ImageInfo(info);
-    bdrv_delete(bs);
+        switch (output_format) {
+        case OFORMAT_HUMAN:
+            dump_human_image_info(info);
+            dump_snapshots(bs);
+            break;
+        case OFORMAT_JSON:
+            collect_snapshots(bs, info);
+            dump_json_image_info(info);
+            break;
+        }
+
+        g_free(filename);
+        g_free(fmt);
+        filename = NULL;
+        fmt = NULL;
+
+        if (chain) {
+            if (info->has_full_backing_filename) {
+                filename = g_strdup(info->full_backing_filename);
+            } else if (info->has_backing_filename) {
+                filename = g_strdup(info->backing_filename);
+            }
+
+            if (filename && info->has_backing_filename_format) {
+                fmt = g_strdup(info->backing_filename_format);
+            }
+
+            /* Print delimiters between items */
+            if (filename) {
+                switch (output_format) {
+                case OFORMAT_HUMAN:
+                    printf("\n");
+                    break;
+                case OFORMAT_JSON:
+                    printf(",\n");
+                    break;
+                }
+            }
+        }
+
+        qapi_free_ImageInfo(info);
+        bdrv_delete(bs);
+    } while (filename);
+
+    if (chain && output_format == OFORMAT_JSON) {
+        printf("]\n");
+    }
     return 0;
+
+err:
+    g_free(filename);
+    g_free(fmt);
+    return 1;
 }
 
 #define SNAPSHOT_LIST   1
-- 
1.7.11.4

Re: [Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1039 bytes --]

On 10/12/2012 08:09 AM, Stefan Hajnoczi wrote:
> The qemu-img info --backing-chain option enumerates the backing file
> chain.  For example, for base.qcow2 <- snap1.qcow2 <- snap2.qcow2 the
> output becomes:
> 
>   $ qemu-img info --backing-chain snap2.qcow2
>   image: snap2.qcow2
>   file format: qcow2
>   virtual size: 100M (104857600 bytes)
>   disk size: 196K
>   cluster_size: 65536
>   backing file: snap1.qcow2
>   backing file format: qcow2
> 
>   image: snap1.qcow2
>   file format: qcow2
>   virtual size: 100M (104857600 bytes)
>   disk size: 196K
>   cluster_size: 65536
>   backing file: base.qcow2
>   backing file format: qcow2
> 
>   image: base.qcow2
>   file format: qcow2
>   virtual size: 100M (104857600 bytes)
>   disk size: 136K
>   cluster_size: 65536
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Very useful.

Reviewed-by: Eric Blake <eblake@redhat.com>

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

Re: [Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

On 10/12/2012 08:09 AM, Stefan Hajnoczi wrote:
> The qemu-img info --backing-chain option enumerates the backing file
> chain.  For example, for base.qcow2 <- snap1.qcow2 <- snap2.qcow2 the
> output becomes:
> 

> +    do {
> +        bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING,
> +                           false);
> +        if (!bs) {
> +            goto err;
> +        }

> +    } while (filename);

Eww - infinite loop if presented with malicious data where someone has
used 'qemu-img rebase -u' to create a cycle.  I think you need a
followup patch that hashes which files have been opened to date, and
abort the loop once a cycle is detected.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

Re: [Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Kevin Wolf]

Am 12.10.2012 16:24, schrieb Eric Blake:
> On 10/12/2012 08:09 AM, Stefan Hajnoczi wrote:
>> The qemu-img info --backing-chain option enumerates the backing file
>> chain.  For example, for base.qcow2 <- snap1.qcow2 <- snap2.qcow2 the
>> output becomes:
>>
> 
>> +    do {
>> +        bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING,
>> +                           false);
>> +        if (!bs) {
>> +            goto err;
>> +        }
> 
>> +    } while (filename);
> 
> Eww - infinite loop if presented with malicious data where someone has
> used 'qemu-img rebase -u' to create a cycle.  I think you need a
> followup patch that hashes which files have been opened to date, and
> abort the loop once a cycle is detected.

That would already cause problems in bdrv_open(), so I'd consider it a
separate bug. We should fail gracefully when trying to open such an
image. Once it's open, other code can trust that the chain makes sense.

Kevin

Re: [Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1812 bytes --]

On 10/12/2012 08:27 AM, Kevin Wolf wrote:
> Am 12.10.2012 16:24, schrieb Eric Blake:
>> On 10/12/2012 08:09 AM, Stefan Hajnoczi wrote:
>>> The qemu-img info --backing-chain option enumerates the backing file
>>> chain.  For example, for base.qcow2 <- snap1.qcow2 <- snap2.qcow2 the
>>> output becomes:
>>>
>>
>>> +    do {
>>> +        bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING,
>>> +                           false);
>>> +        if (!bs) {
>>> +            goto err;
>>> +        }
>>
>>> +    } while (filename);
>>
>> Eww - infinite loop if presented with malicious data where someone has
>> used 'qemu-img rebase -u' to create a cycle.  I think you need a
>> followup patch that hashes which files have been opened to date, and
>> abort the loop once a cycle is detected.
> 
> That would already cause problems in bdrv_open(), so I'd consider it a
> separate bug. We should fail gracefully when trying to open such an
> image. Once it's open, other code can trust that the chain makes sense.

Hmm.  For 'qemu-img info', I can see two behaviors, both useful, when
presented with a corrupt image.  One is to error out right away (because
qemu would be unable to use the image).  But the other is for debugging
WHY the image is corrupt, at which point I want qemu-img info to display
as much information as possible, INCLUDING what backing file is recorded
in the header, so that I can follow the loop and decide where to break
the loop.  Sounds like we might need another flag to bdrv_open() on
whether to detect cycles; as well as fixing qemu-img info to check for
cycles on its own when it bypasses normal cycle-checking in bdrv_open.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

Re: [Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Kevin Wolf]

Am 12.10.2012 16:32, schrieb Eric Blake:
> On 10/12/2012 08:27 AM, Kevin Wolf wrote:
>> Am 12.10.2012 16:24, schrieb Eric Blake:
>>> On 10/12/2012 08:09 AM, Stefan Hajnoczi wrote:
>>>> The qemu-img info --backing-chain option enumerates the backing file
>>>> chain.  For example, for base.qcow2 <- snap1.qcow2 <- snap2.qcow2 the
>>>> output becomes:
>>>>
>>>
>>>> +    do {
>>>> +        bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING,
>>>> +                           false);
>>>> +        if (!bs) {
>>>> +            goto err;
>>>> +        }
>>>
>>>> +    } while (filename);
>>>
>>> Eww - infinite loop if presented with malicious data where someone has
>>> used 'qemu-img rebase -u' to create a cycle.  I think you need a
>>> followup patch that hashes which files have been opened to date, and
>>> abort the loop once a cycle is detected.
>>
>> That would already cause problems in bdrv_open(), so I'd consider it a
>> separate bug. We should fail gracefully when trying to open such an
>> image. Once it's open, other code can trust that the chain makes sense.
> 
> Hmm.  For 'qemu-img info', I can see two behaviors, both useful, when
> presented with a corrupt image.  One is to error out right away (because
> qemu would be unable to use the image).  But the other is for debugging
> WHY the image is corrupt, at which point I want qemu-img info to display
> as much information as possible, INCLUDING what backing file is recorded
> in the header, so that I can follow the loop and decide where to break
> the loop.  Sounds like we might need another flag to bdrv_open() on
> whether to detect cycles; as well as fixing qemu-img info to check for
> cycles on its own when it bypasses normal cycle-checking in bdrv_open.

Makes sense. Though I think BDRV_O_NO_BACKING is enough to implement
this functionality in qemu-img. We'd just have to have an error code
that allows qemu-img to check if we detected a loop so that it can start
searching the broken image.

Kevin

Re: [Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1675 bytes --]

On 10/12/2012 08:38 AM, Kevin Wolf wrote:
>> Hmm.  For 'qemu-img info', I can see two behaviors, both useful, when
>> presented with a corrupt image.  One is to error out right away (because
>> qemu would be unable to use the image).  But the other is for debugging
>> WHY the image is corrupt, at which point I want qemu-img info to display
>> as much information as possible, INCLUDING what backing file is recorded
>> in the header, so that I can follow the loop and decide where to break
>> the loop.  Sounds like we might need another flag to bdrv_open() on
>> whether to detect cycles; as well as fixing qemu-img info to check for
>> cycles on its own when it bypasses normal cycle-checking in bdrv_open.
> 
> Makes sense. Though I think BDRV_O_NO_BACKING is enough to implement
> this functionality in qemu-img. We'd just have to have an error code
> that allows qemu-img to check if we detected a loop so that it can start
> searching the broken image.

Indeed - BDRV_O_NO_BACKING seems to be the flag that prevents bdrv_open
from failing on a cycle, so the logic would be something like:

Try a normal bdrv_open()
if it succeeds
   image is fine, so recursively print it
else if error indicated a loop
   init hash table
   while (1)
       if file in hash table
           break, since we identified the loop
       bdrv_open(BDRV_O_NO_BACKING)
       print this file, then add it to the hash table
       file = backing

At any rate, adding this logic should be a separate patch, and not hold
up Stefan's current patch.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

Re: [Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Kashyap Chamarthy]

On Fri, Oct 12, 2012 at 7:39 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The qemu-img info --backing-chain option enumerates the backing file
> chain.  For example, for base.qcow2 <- snap1.qcow2 <- snap2.qcow2 the
> output becomes:
>
>   $ qemu-img info --backing-chain snap2.qcow2
>   image: snap2.qcow2
>   file format: qcow2
>   virtual size: 100M (104857600 bytes)
>   disk size: 196K
>   cluster_size: 65536
>   backing file: snap1.qcow2
>   backing file format: qcow2
>
>   image: snap1.qcow2
>   file format: qcow2
>   virtual size: 100M (104857600 bytes)
>   disk size: 196K
>   cluster_size: 65536
>   backing file: base.qcow2
>   backing file format: qcow2
>
>   image: base.qcow2
>   file format: qcow2
>   virtual size: 100M (104857600 bytes)
>   disk size: 136K
>   cluster_size: 65536
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  qemu-img.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 76 insertions(+), 22 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index f17f187..c717f3e 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1249,7 +1249,10 @@ static void dump_human_image_info(ImageInfo *info)
>      }
>  }
>
> -enum {OPTION_OUTPUT = 256};
> +enum {
> +    OPTION_OUTPUT = 256,
> +    OPTION_BACKING_CHAIN = 257,
> +};
>
>  typedef enum OutputFormat {
>      OFORMAT_JSON,
> @@ -1260,7 +1263,9 @@ static int img_info(int argc, char **argv)
>  {
>      int c;
>      OutputFormat output_format = OFORMAT_HUMAN;
> -    const char *filename, *fmt, *output;
> +    bool chain = false;
> +    const char *output;
> +    char *filename, *fmt;
>      BlockDriverState *bs;
>      ImageInfo *info;
>
> @@ -1272,6 +1277,7 @@ static int img_info(int argc, char **argv)
>              {"help", no_argument, 0, 'h'},
>              {"format", required_argument, 0, 'f'},
>              {"output", required_argument, 0, OPTION_OUTPUT},
> +            {"backing-chain", no_argument, 0, OPTION_BACKING_CHAIN},
>              {0, 0, 0, 0}
>          };
>          c = getopt_long(argc, argv, "f:h",
> @@ -1285,17 +1291,20 @@ static int img_info(int argc, char **argv)
>              help();
>              break;
>          case 'f':
> -            fmt = optarg;
> +            fmt = g_strdup(optarg);
>              break;
>          case OPTION_OUTPUT:
>              output = optarg;
>              break;
> +        case OPTION_BACKING_CHAIN:
> +            chain = true;
> +            break;
>          }
>      }
>      if (optind >= argc) {
>          help();
>      }
> -    filename = argv[optind++];
> +    filename = g_strdup(argv[optind++]);
>
>      if (output && !strcmp(output, "json")) {
>          output_format = OFORMAT_JSON;
> @@ -1303,31 +1312,76 @@ static int img_info(int argc, char **argv)
>          output_format = OFORMAT_HUMAN;
>      } else if (output) {
>          error_report("--output must be used with human or json as argument.");
> -        return 1;
> +        goto err;
>      }
>
> -    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING, false);
> -    if (!bs) {
> -        return 1;
> +    if (chain && output_format == OFORMAT_JSON) {
> +        printf("[\n");
>      }
>
> -    info = g_new0(ImageInfo, 1);
> -    collect_image_info(bs, info, filename, fmt);
> +    do {
> +        bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING,
> +                           false);
> +        if (!bs) {
> +            goto err;
> +        }
>
> -    switch (output_format) {
> -    case OFORMAT_HUMAN:
> -        dump_human_image_info(info);
> -        dump_snapshots(bs);
> -        break;
> -    case OFORMAT_JSON:
> -        collect_snapshots(bs, info);
> -        dump_json_image_info(info);
> -        break;
> -    }
> +        info = g_new0(ImageInfo, 1);
> +        collect_image_info(bs, info, filename, fmt);
>
> -    qapi_free_ImageInfo(info);
> -    bdrv_delete(bs);
> +        switch (output_format) {
> +        case OFORMAT_HUMAN:
> +            dump_human_image_info(info);
> +            dump_snapshots(bs);
> +            break;
> +        case OFORMAT_JSON:
> +            collect_snapshots(bs, info);
> +            dump_json_image_info(info);
> +            break;
> +        }
> +
> +        g_free(filename);
> +        g_free(fmt);
> +        filename = NULL;
> +        fmt = NULL;
> +
> +        if (chain) {
> +            if (info->has_full_backing_filename) {
> +                filename = g_strdup(info->full_backing_filename);
> +            } else if (info->has_backing_filename) {
> +                filename = g_strdup(info->backing_filename);
> +            }
> +
> +            if (filename && info->has_backing_filename_format) {
> +                fmt = g_strdup(info->backing_filename_format);
> +            }
> +
> +            /* Print delimiters between items */
> +            if (filename) {
> +                switch (output_format) {
> +                case OFORMAT_HUMAN:
> +                    printf("\n");
> +                    break;
> +                case OFORMAT_JSON:
> +                    printf(",\n");
> +                    break;
> +                }
> +            }
> +        }
> +
> +        qapi_free_ImageInfo(info);
> +        bdrv_delete(bs);
> +    } while (filename);
> +
> +    if (chain && output_format == OFORMAT_JSON) {
> +        printf("]\n");
> +    }
>      return 0;
> +
> +err:
> +    g_free(filename);
> +    g_free(fmt);
> +    return 1;
>  }
>
>  #define SNAPSHOT_LIST   1
> --
> 1.7.11.4
>

Thanks a lot Stefan for making this. Just tested this with latest qemu
git. Works like a charm:

[kashyap@moon qemu]$ ./qemu-img info --backing-chain
/var/lib/libvirt/images/snap4-of-test-f17base.qcow2
image: /var/lib/libvirt/images/snap4-of-test-f17base.qcow2
file format: qcow2
virtual size: 1.0G (1073741824 bytes)
disk size: 3.6M
cluster_size: 65536
backing file: /var/lib/libvirt/images/snap1-of-test-f17base.qcow2
backing file format: qcow2

image: /var/lib/libvirt/images/snap1-of-test-f17base.qcow2
file format: qcow2
virtual size: 1.0G (1073741824 bytes)
disk size: 968K
cluster_size: 65536
backing file: /export/vmimgs2/test-f17base.img
backing file format: raw

image: /export/vmimgs2/test-f17base.img
file format: raw
virtual size: 1.0G (1073741824 bytes)
disk size: 607M
[kashyap@moon qemu]$

Re: [Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Kashyap Chamarthy]

[-- Attachment #1: Type: text/plain, Size: 6733 bytes --]

On Sat, Oct 13, 2012 at 12:46 AM, Kashyap Chamarthy
<kashyap.cv@gmail.com> wrote:
> On Fri, Oct 12, 2012 at 7:39 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>> The qemu-img info --backing-chain option enumerates the backing file
>> chain.  For example, for base.qcow2 <- snap1.qcow2 <- snap2.qcow2 the
>> output becomes:
>>
>>   $ qemu-img info --backing-chain snap2.qcow2
>>   image: snap2.qcow2
>>   file format: qcow2
>>   virtual size: 100M (104857600 bytes)
>>   disk size: 196K
>>   cluster_size: 65536
>>   backing file: snap1.qcow2
>>   backing file format: qcow2
>>
>>   image: snap1.qcow2
>>   file format: qcow2
>>   virtual size: 100M (104857600 bytes)
>>   disk size: 196K
>>   cluster_size: 65536
>>   backing file: base.qcow2
>>   backing file format: qcow2
>>
>>   image: base.qcow2
>>   file format: qcow2
>>   virtual size: 100M (104857600 bytes)
>>   disk size: 136K
>>   cluster_size: 65536
>>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>>  qemu-img.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
>>  1 file changed, 76 insertions(+), 22 deletions(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index f17f187..c717f3e 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -1249,7 +1249,10 @@ static void dump_human_image_info(ImageInfo *info)
>>      }
>>  }
>>
>> -enum {OPTION_OUTPUT = 256};
>> +enum {
>> +    OPTION_OUTPUT = 256,
>> +    OPTION_BACKING_CHAIN = 257,
>> +};
>>
>>  typedef enum OutputFormat {
>>      OFORMAT_JSON,
>> @@ -1260,7 +1263,9 @@ static int img_info(int argc, char **argv)
>>  {
>>      int c;
>>      OutputFormat output_format = OFORMAT_HUMAN;
>> -    const char *filename, *fmt, *output;
>> +    bool chain = false;
>> +    const char *output;
>> +    char *filename, *fmt;
>>      BlockDriverState *bs;
>>      ImageInfo *info;
>>
>> @@ -1272,6 +1277,7 @@ static int img_info(int argc, char **argv)
>>              {"help", no_argument, 0, 'h'},
>>              {"format", required_argument, 0, 'f'},
>>              {"output", required_argument, 0, OPTION_OUTPUT},
>> +            {"backing-chain", no_argument, 0, OPTION_BACKING_CHAIN},
>>              {0, 0, 0, 0}
>>          };
>>          c = getopt_long(argc, argv, "f:h",
>> @@ -1285,17 +1291,20 @@ static int img_info(int argc, char **argv)
>>              help();
>>              break;
>>          case 'f':
>> -            fmt = optarg;
>> +            fmt = g_strdup(optarg);
>>              break;
>>          case OPTION_OUTPUT:
>>              output = optarg;
>>              break;
>> +        case OPTION_BACKING_CHAIN:
>> +            chain = true;
>> +            break;
>>          }
>>      }
>>      if (optind >= argc) {
>>          help();
>>      }
>> -    filename = argv[optind++];
>> +    filename = g_strdup(argv[optind++]);
>>
>>      if (output && !strcmp(output, "json")) {
>>          output_format = OFORMAT_JSON;
>> @@ -1303,31 +1312,76 @@ static int img_info(int argc, char **argv)
>>          output_format = OFORMAT_HUMAN;
>>      } else if (output) {
>>          error_report("--output must be used with human or json as argument.");
>> -        return 1;
>> +        goto err;
>>      }
>>
>> -    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING, false);
>> -    if (!bs) {
>> -        return 1;
>> +    if (chain && output_format == OFORMAT_JSON) {
>> +        printf("[\n");
>>      }
>>
>> -    info = g_new0(ImageInfo, 1);
>> -    collect_image_info(bs, info, filename, fmt);
>> +    do {
>> +        bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING,
>> +                           false);
>> +        if (!bs) {
>> +            goto err;
>> +        }
>>
>> -    switch (output_format) {
>> -    case OFORMAT_HUMAN:
>> -        dump_human_image_info(info);
>> -        dump_snapshots(bs);
>> -        break;
>> -    case OFORMAT_JSON:
>> -        collect_snapshots(bs, info);
>> -        dump_json_image_info(info);
>> -        break;
>> -    }
>> +        info = g_new0(ImageInfo, 1);
>> +        collect_image_info(bs, info, filename, fmt);
>>
>> -    qapi_free_ImageInfo(info);
>> -    bdrv_delete(bs);
>> +        switch (output_format) {
>> +        case OFORMAT_HUMAN:
>> +            dump_human_image_info(info);
>> +            dump_snapshots(bs);
>> +            break;
>> +        case OFORMAT_JSON:
>> +            collect_snapshots(bs, info);
>> +            dump_json_image_info(info);
>> +            break;
>> +        }
>> +
>> +        g_free(filename);
>> +        g_free(fmt);
>> +        filename = NULL;
>> +        fmt = NULL;
>> +
>> +        if (chain) {
>> +            if (info->has_full_backing_filename) {
>> +                filename = g_strdup(info->full_backing_filename);
>> +            } else if (info->has_backing_filename) {
>> +                filename = g_strdup(info->backing_filename);
>> +            }
>> +
>> +            if (filename && info->has_backing_filename_format) {
>> +                fmt = g_strdup(info->backing_filename_format);
>> +            }
>> +
>> +            /* Print delimiters between items */
>> +            if (filename) {
>> +                switch (output_format) {
>> +                case OFORMAT_HUMAN:
>> +                    printf("\n");
>> +                    break;
>> +                case OFORMAT_JSON:
>> +                    printf(",\n");
>> +                    break;
>> +                }
>> +            }
>> +        }
>> +
>> +        qapi_free_ImageInfo(info);
>> +        bdrv_delete(bs);
>> +    } while (filename);
>> +
>> +    if (chain && output_format == OFORMAT_JSON) {
>> +        printf("]\n");
>> +    }
>>      return 0;
>> +
>> +err:
>> +    g_free(filename);
>> +    g_free(fmt);
>> +    return 1;
>>  }
>>
>>  #define SNAPSHOT_LIST   1
>> --
>> 1.7.11.4
>>
>
> Thanks a lot Stefan for making this. Just tested this with latest qemu
> git. Works like a charm:
>
> [kashyap@moon qemu]$ ./qemu-img info --backing-chain
> /var/lib/libvirt/images/snap4-of-test-f17base.qcow2
> image: /var/lib/libvirt/images/snap4-of-test-f17base.qcow2
> file format: qcow2
> virtual size: 1.0G (1073741824 bytes)
> disk size: 3.6M
> cluster_size: 65536
> backing file: /var/lib/libvirt/images/snap1-of-test-f17base.qcow2
> backing file format: qcow2
>
> image: /var/lib/libvirt/images/snap1-of-test-f17base.qcow2
> file format: qcow2
> virtual size: 1.0G (1073741824 bytes)
> disk size: 968K
> cluster_size: 65536
> backing file: /export/vmimgs2/test-f17base.img
> backing file format: raw
>
> image: /export/vmimgs2/test-f17base.img
> file format: raw
> virtual size: 1.0G (1073741824 bytes)
> disk size: 607M
> [kashyap@moon qemu]$


Attached a documentation patch for the '--backing-chain' option.

/kashyap

[-- Attachment #2: 0001-Add-documentation-for-qemu-img-info-backing-chain.patch --]
[-- Type: application/octet-stream, Size: 2365 bytes --]

From eb4c4bc92c035c42c23c30c5e7ee73b54f9cf3a8 Mon Sep 17 00:00:00 2001
From: Kashyap Chamarthy <kashyap.cv@gmail.com>
Date: Sat, 13 Oct 2012 01:30:37 +0530
Subject: [PATCH] Add documentation for 'qemu-img info --backing-chain'

Signed-off-by: Kashyap Chamarthy <kashyap.cv@gmail.com>
---
 qemu-img.texi | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/qemu-img.texi b/qemu-img.texi
index 8b05f2c42801a2535ab4390b47bc415eb880625a..6bbb7a774797fb0cb702d2ff6fc585d90cc0984a 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -28,6 +28,10 @@ Command parameters:
 is the disk image format. It is guessed automatically in most cases. See below
 for a description of the supported disk formats.
 
+@item --backing-chain 
+will enumerate information about backing files in a disk image chain. Refer
+below for further description.
+
 @item size
 is the disk image size in bytes. Optional suffixes @code{k} or @code{K}
 (kilobyte, 1024) @code{M} (megabyte, 1024k) and @code{G} (gigabyte, 1024M)
@@ -129,7 +133,7 @@ created as a copy on write image of the specified base image; the
 @var{backing_file} should have the same content as the input's base image,
 however the path, image format, etc may differ.
 
-@item info [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
+@item info [-f @var{fmt}] [--output=@var{ofamt}] [--backing-chain] @var{filename}
 
 Give information about the disk image @var{filename}. Use it in
 particular to know the size reserved on disk which can be different
@@ -137,6 +141,19 @@ from the displayed size. If VM snapshots are stored in the disk image,
 they are displayed too. The command can output in the format @var{ofmt}
 which is either @code{human} or @code{json}.
 
+If a disk image has a backing file chain, information about each disk image in
+the chain can be recursively enumerated by using the option @code{--backing-chain} 
+
+For instance, if you have an image chain like: 
+
+    base.qcow2 <- snap1.qcow2 <- snap2.qcow2 
+
+To enumerate information about each disk image in the above chain, starting from top to base, do:
+    
+    @example
+    qemu-img info --backing-chain snap2.qcow2
+    @end example
+
 @item snapshot [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot} ] @var{filename}
 
 List, apply, create or delete snapshots in image @var{filename}.
-- 
1.7.12.1

Re: [Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1643 bytes --]

On 10/12/2012 02:19 PM, Kashyap Chamarthy wrote:

> 
> From eb4c4bc92c035c42c23c30c5e7ee73b54f9cf3a8 Mon Sep 17 00:00:00 2001
> From: Kashyap Chamarthy <kashyap.cv@gmail.com>
> Date: Sat, 13 Oct 2012 01:30:37 +0530
> Subject: [PATCH] Add documentation for 'qemu-img info --backing-chain'
> 
> Signed-off-by: Kashyap Chamarthy <kashyap.cv@gmail.com>
> ---
>  qemu-img.texi | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)

We also need a patch to qemu-img-cmds.hx.

> @@ -129,7 +133,7 @@ created as a copy on write image of the specified base image; the
>  @var{backing_file} should have the same content as the input's base image,
>  however the path, image format, etc may differ.
>  
> -@item info [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
> +@item info [-f @var{fmt}] [--output=@var{ofamt}] [--backing-chain] @var{filename}

Introduced a spurious typo:
s/ofamt/ofmt/

>  
>  Give information about the disk image @var{filename}. Use it in
>  particular to know the size reserved on disk which can be different
> @@ -137,6 +141,19 @@ from the displayed size. If VM snapshots are stored in the disk image,
>  they are displayed too. The command can output in the format @var{ofmt}
>  which is either @code{human} or @code{json}.
>  
> +If a disk image has a backing file chain, information about each disk image in
> +the chain can be recursively enumerated by using the option @code{--backing-chain} 

Trailing whitespace.  Needs a '.' to end the sentence.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

Re: [Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Kashyap Chamarthy]

[-- Attachment #1: Type: text/plain, Size: 1946 bytes --]

On Sat, Oct 13, 2012 at 2:01 AM, Eric Blake <eblake@redhat.com> wrote:
> On 10/12/2012 02:19 PM, Kashyap Chamarthy wrote:
>
>>
>> From eb4c4bc92c035c42c23c30c5e7ee73b54f9cf3a8 Mon Sep 17 00:00:00 2001
>> From: Kashyap Chamarthy <kashyap.cv@gmail.com>
>> Date: Sat, 13 Oct 2012 01:30:37 +0530
>> Subject: [PATCH] Add documentation for 'qemu-img info --backing-chain'
>>
>> Signed-off-by: Kashyap Chamarthy <kashyap.cv@gmail.com>
>> ---
>>  qemu-img.texi | 19 ++++++++++++++++++-
>>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> We also need a patch to qemu-img-cmds.hx.
>
>> @@ -129,7 +133,7 @@ created as a copy on write image of the specified base image; the
>>  @var{backing_file} should have the same content as the input's base image,
>>  however the path, image format, etc may differ.
>>
>> -@item info [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
>> +@item info [-f @var{fmt}] [--output=@var{ofamt}] [--backing-chain] @var{filename}
>
> Introduced a spurious typo:
> s/ofamt/ofmt/
>
>>
>>  Give information about the disk image @var{filename}. Use it in
>>  particular to know the size reserved on disk which can be different
>> @@ -137,6 +141,19 @@ from the displayed size. If VM snapshots are stored in the disk image,
>>  they are displayed too. The command can output in the format @var{ofmt}
>>  which is either @code{human} or @code{json}.
>>
>> +If a disk image has a backing file chain, information about each disk image in
>> +the chain can be recursively enumerated by using the option @code{--backing-chain}
>
> Trailing whitespace.  Needs a '.' to end the sentence.

Thanks for the review Eric, new patch attached.

[PS: I haven't used 'git-send-email' for this trivial patch. Should I
have used that, instead of attaching it? I was wondering if
attachments like these are frowned upon. ]

/kashyap

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

[-- Attachment #2: 0001-Add-documentation-for-qemu-img-info-backing-chain-wi.patch --]
[-- Type: application/octet-stream, Size: 2978 bytes --]

From 68b22497ad1c9318ae57092f7e6af543b4106e1b Mon Sep 17 00:00:00 2001
From: Kashyap Chamarthy <kashyap.cv@gmail.com>
Date: Sat, 13 Oct 2012 20:54:28 +0530
Subject: [PATCH] Add documentation for 'qemu-img info --backing-chain' (with
 Eric's comments fixed)

Signed-off-by: Kashyap Chamarthy <kashyap.cv@gmail.com>
---
 qemu-img-cmds.hx |  4 ++--
 qemu-img.texi    | 19 ++++++++++++++++++-
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 0ef82e9ac7a89d607e1f2c8e38461cee5b438dba..a18136302d2ca7a7540672f1b9ef89603e89edc0 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -34,9 +34,9 @@ STEXI
 ETEXI
 
 DEF("info", img_info,
-    "info [-f fmt] [--output=ofmt] filename")
+    "info [-f fmt] [--output=ofmt] [--backing-chain] filename")
 STEXI
-@item info [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
+@item info [-f @var{fmt}] [--output=@var{ofmt}] [--backing-chain] @var{filename}
 ETEXI
 
 DEF("snapshot", img_snapshot,
diff --git a/qemu-img.texi b/qemu-img.texi
index 8b05f2c42801a2535ab4390b47bc415eb880625a..4f659e90039aba27abf91ca32a9703515b13a130 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -28,6 +28,10 @@ Command parameters:
 is the disk image format. It is guessed automatically in most cases. See below
 for a description of the supported disk formats.
 
+@item --backing-chain 
+will enumerate information about backing files in a disk image chain. Refer
+below for further description.
+
 @item size
 is the disk image size in bytes. Optional suffixes @code{k} or @code{K}
 (kilobyte, 1024) @code{M} (megabyte, 1024k) and @code{G} (gigabyte, 1024M)
@@ -129,7 +133,7 @@ created as a copy on write image of the specified base image; the
 @var{backing_file} should have the same content as the input's base image,
 however the path, image format, etc may differ.
 
-@item info [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
+@item info [-f @var{fmt}] [--output=@var{oft}] [--backing-chain] @var{filename}
 
 Give information about the disk image @var{filename}. Use it in
 particular to know the size reserved on disk which can be different
@@ -137,6 +141,19 @@ from the displayed size. If VM snapshots are stored in the disk image,
 they are displayed too. The command can output in the format @var{ofmt}
 which is either @code{human} or @code{json}.
 
+If a disk image has a backing file chain, information about each disk image in
+the chain can be recursively enumerated by using the option @code{--backing-chain}.
+
+For instance, if you have an image chain like: 
+
+    base.qcow2 <- snap1.qcow2 <- snap2.qcow2 
+
+To enumerate information about each disk image in the above chain, starting from top to base, do:
+    
+    @example
+    qemu-img info --backing-chain snap2.qcow2
+    @end example
+
 @item snapshot [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot} ] @var{filename}
 
 List, apply, create or delete snapshots in image @var{filename}.
-- 
1.7.12.1

Re: [Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2379 bytes --]

On 10/13/2012 09:50 AM, Kashyap Chamarthy wrote:

> 
> [PS: I haven't used 'git-send-email' for this trivial patch. Should I
> have used that, instead of attaching it? I was wondering if
> attachments like these are frowned upon. ]

That depends on who is applying the patch (it won't be me, since I'm not
listed as maintainer); but even in my view as a reviewer, attachments
embedded to a lengthy chain of reply text is harder to reply to than a
fresh thread.  So, given that I have review comments below, you might as
well use 'git send-email' to post a v2 as a fresh thread, so we don't
have to worry about whether this will be spotted deeply embedded in a
thread.

> From 68b22497ad1c9318ae57092f7e6af543b4106e1b Mon Sep 17 00:00:00 2001
> From: Kashyap Chamarthy <kashyap.cv@gmail.com>
> Date: Sat, 13 Oct 2012 20:54:28 +0530
> Subject: [PATCH] Add documentation for 'qemu-img info --backing-chain' (with
>  Eric's comments fixed)

The subject line becomes part of the permanent git history, and as such,
it should avoid versioning information, and just describe the change (or
in other words, leave my name out of the 'git log --one-line' output).
Better would be:

qemu-img: document 'info --backing-chain'

> 
> Signed-off-by: Kashyap Chamarthy <kashyap.cv@gmail.com>
> ---
>  qemu-img-cmds.hx |  4 ++--
>  qemu-img.texi    | 19 ++++++++++++++++++-
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 

>  
> +@item --backing-chain 
> +will enumerate information about backing files in a disk image chain. Refer
> +below for further description.
> +
>  @item size
>  is the disk image size in bytes. Optional suffixes @code{k} or @code{K}
>  (kilobyte, 1024) @code{M} (megabyte, 1024k) and @code{G} (gigabyte, 1024M)
> @@ -129,7 +133,7 @@ created as a copy on write image of the specified base image; the
>  @var{backing_file} should have the same content as the input's base image,
>  however the path, image format, etc may differ.
>  
> -@item info [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
> +@item info [-f @var{fmt}] [--output=@var{oft}] [--backing-chain] @var{filename}

Last time, you added a spurious 'a'; this time, you accidentally nuked
an 'm'.  Please, leave @var{ofmt} untouched.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

Re: [Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Kashyap Chamarthy]

On Sun, Oct 14, 2012 at 3:06 AM, Eric Blake <eblake@redhat.com> wrote:
> On 10/13/2012 09:50 AM, Kashyap Chamarthy wrote:
>
>>
>> [PS: I haven't used 'git-send-email' for this trivial patch. Should I
>> have used that, instead of attaching it? I was wondering if
>> attachments like these are frowned upon. ]
>
> That depends on who is applying the patch (it won't be me, since I'm not
> listed as maintainer); but even in my view as a reviewer, attachments
> embedded to a lengthy chain of reply text is harder to reply to than a
> fresh thread.  So, given that I have review comments below, you might as
> well use 'git send-email' to post a v2 as a fresh thread, so we don't
> have to worry about whether this will be spotted deeply embedded in a
> thread.

Noted.

>
>> From 68b22497ad1c9318ae57092f7e6af543b4106e1b Mon Sep 17 00:00:00 2001
>> From: Kashyap Chamarthy <kashyap.cv@gmail.com>
>> Date: Sat, 13 Oct 2012 20:54:28 +0530
>> Subject: [PATCH] Add documentation for 'qemu-img info --backing-chain' (with
>>  Eric's comments fixed)
>
> The subject line becomes part of the permanent git history, and as such,
> it should avoid versioning information, and just describe the change (or
> in other words, leave my name out of the 'git log --one-line' output).
> Better would be:
>
> qemu-img: document 'info --backing-chain'

Noted.

>
>>
>> Signed-off-by: Kashyap Chamarthy <kashyap.cv@gmail.com>
>> ---
>>  qemu-img-cmds.hx |  4 ++--
>>  qemu-img.texi    | 19 ++++++++++++++++++-
>>  2 files changed, 20 insertions(+), 3 deletions(-)
>>
>
>>
>> +@item --backing-chain
>> +will enumerate information about backing files in a disk image chain. Refer
>> +below for further description.
>> +
>>  @item size
>>  is the disk image size in bytes. Optional suffixes @code{k} or @code{K}
>>  (kilobyte, 1024) @code{M} (megabyte, 1024k) and @code{G} (gigabyte, 1024M)
>> @@ -129,7 +133,7 @@ created as a copy on write image of the specified base image; the
>>  @var{backing_file} should have the same content as the input's base image,
>>  however the path, image format, etc may differ.
>>
>> -@item info [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
>> +@item info [-f @var{fmt}] [--output=@var{oft}] [--backing-chain] @var{filename}
>
> Last time, you added a spurious 'a'; this time, you accidentally nuked
> an 'm'.  Please, leave @var{ofmt} untouched.

Sorry for the overlook, fixed and sent a new one as a fresh thread.

/kashyap

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