diff mbox

[v2,5/6] qemu-img: Specify backing file for commit

Message ID 1396961442-24046-6-git-send-email-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz April 8, 2014, 12:50 p.m. UTC
Introduce a new parameter for qemu-img commit which may be used to
explicitly specify the backing file unto which an image should be
committed if the backing chain has more than a single layer.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img-cmds.hx |  4 ++--
 qemu-img.c       | 22 +++++++++++++++-------
 qemu-img.texi    |  8 +++++++-
 3 files changed, 24 insertions(+), 10 deletions(-)

Comments

Eric Blake April 8, 2014, 5:01 p.m. UTC | #1
On 04/08/2014 06:50 AM, Max Reitz wrote:
> Introduce a new parameter for qemu-img commit which may be used to
> explicitly specify the backing file unto which an image should be

s/unto/into/

> committed if the backing chain has more than a single layer.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qemu-img-cmds.hx |  4 ++--
>  qemu-img.c       | 22 +++++++++++++++-------
>  qemu-img.texi    |  8 +++++++-
>  3 files changed, 24 insertions(+), 10 deletions(-)


> +If the backing chain of the given image file @var{filename} has more than one
> +layer, the backing file unto which the changes shall be committed may be

s/unto/into/
s/shall/will/

> +specified as @var{backing_file} (which has to be part of @var{filename}'s
> +backing chain). If @var{filename} is not specified, the immediate backing file
> +of the top image (which is @var{filename}) will be used.
> +

With those changes,
Reviewed-by: Eric Blake <eblake@redhat.com>
Fam Zheng April 10, 2014, 9:05 a.m. UTC | #2
On Tue, 04/08 14:50, Max Reitz wrote:
> Introduce a new parameter for qemu-img commit which may be used to
> explicitly specify the backing file unto which an image should be
> committed if the backing chain has more than a single layer.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qemu-img-cmds.hx |  4 ++--
>  qemu-img.c       | 22 +++++++++++++++-------
>  qemu-img.texi    |  8 +++++++-
>  3 files changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index 8bc55cd..7f62f6d 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -22,9 +22,9 @@ STEXI
>  ETEXI
>  
>  DEF("commit", img_commit,
> -    "commit [-q] [-f fmt] [-t cache] [-p] filename")
> +    "commit [-q] [-f fmt] [-t cache] [-b backing_file] [-p] filename")
>  STEXI
> -@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-p] @var{filename}
> +@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{backing_file}] [-p] @var{filename}
>  ETEXI
>  
>  DEF("compare", img_compare,
> diff --git a/qemu-img.c b/qemu-img.c
> index 0a9eff7..9d4bdbc 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -725,15 +725,16 @@ static void run_block_job(BlockJob *job, Error **errp)
>  static int img_commit(int argc, char **argv)
>  {
>      int c, ret, flags;
> -    const char *filename, *fmt, *cache;
> +    const char *filename, *fmt, *cache, *base;
>      BlockDriverState *bs, *base_bs;
>      bool progress = false, quiet = false;
>      Error *local_err = NULL;
>  
>      fmt = NULL;
>      cache = BDRV_DEFAULT_CACHE;
> +    base = NULL;
>      for(;;) {
> -        c = getopt(argc, argv, "f:ht:qp");
> +        c = getopt(argc, argv, "f:ht:b:qp");
>          if (c == -1) {
>              break;
>          }
> @@ -748,6 +749,9 @@ static int img_commit(int argc, char **argv)
>          case 't':
>              cache = optarg;
>              break;
> +        case 'b':
> +            base = optarg;
> +            break;
>          case 'p':
>              progress = true;
>              break;
> @@ -782,12 +786,16 @@ static int img_commit(int argc, char **argv)
>      qemu_progress_init(progress, 1.f);
>      qemu_progress_print(0.f, 100);
>  
> -    /* This is different from QMP, which by default uses the deepest file in the
> -     * backing chain (i.e., the very base); however, the traditional behavior of
> -     * qemu-img commit is using the immediate backing file. */
> -    base_bs = bs->backing_hd;
> +    if (base) {
> +        base_bs = bdrv_find_backing_image(bs, base);
> +    } else {
> +        /* This is different from QMP, which by default uses the deepest file in
> +         * the backing chain (i.e., the very base); however, the traditional
> +         * behavior of qemu-img commit is using the immediate backing file. */
> +        base_bs = bs->backing_hd;
> +    }
>      if (!base_bs) {
> -        error_set(&local_err, QERR_BASE_NOT_FOUND, "NULL");
> +        error_set(&local_err, QERR_BASE_NOT_FOUND, base ?: "NULL");
>          goto done;
>      }
>  
> diff --git a/qemu-img.texi b/qemu-img.texi
> index 1a9c08f..4a9f493 100644
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -140,7 +140,7 @@ this case. @var{backing_file} will never be modified unless you use the
>  The size can also be specified using the @var{size} option with @code{-o},
>  it doesn't need to be specified separately in this case.
>  
> -@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-p] @var{filename}
> +@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{backing_file}] [-p] @var{filename}
>  
>  Commit the changes recorded in @var{filename} in its base image or backing file.
>  If the backing file is smaller than the snapshot, then the backing file will be
> @@ -149,6 +149,12 @@ the backing file, the backing file will not be truncated.  If you want the
>  backing file to match the size of the smaller snapshot, you can safely truncate
>  it yourself once the commit operation successfully completes.
>  
> +If the backing chain of the given image file @var{filename} has more than one
> +layer, the backing file unto which the changes shall be committed may be
> +specified as @var{backing_file} (which has to be part of @var{filename}'s
> +backing chain). If @var{filename} is not specified, the immediate backing file

s/@var{filename}/@var{backing_file}/ ?

BTW how about just calling it 'base' as in qmp commands, because backing_file
has usages in (slightly) different context of create.

Other than the two questions,

Reviewed-by: Fam Zheng <famz@redhat.com>

> +of the top image (which is @var{filename}) will be used.
> +
>  @item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} @var{filename2}
>  
>  Check if two images have the same content. You can compare images with
> -- 
> 1.9.1
> 
>
Max Reitz April 10, 2014, 2:42 p.m. UTC | #3
On 08.04.2014 19:01, Eric Blake wrote:
> On 04/08/2014 06:50 AM, Max Reitz wrote:
>> Introduce a new parameter for qemu-img commit which may be used to
>> explicitly specify the backing file unto which an image should be
> s/unto/into/

I was wondering about that and asked someone about it (German as well, 
however), who said “unto” would be correct. And since you did not 
complain in v1… ;-)

I'll fix it.

>> committed if the backing chain has more than a single layer.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   qemu-img-cmds.hx |  4 ++--
>>   qemu-img.c       | 22 +++++++++++++++-------
>>   qemu-img.texi    |  8 +++++++-
>>   3 files changed, 24 insertions(+), 10 deletions(-)
>
>> +If the backing chain of the given image file @var{filename} has more than one
>> +layer, the backing file unto which the changes shall be committed may be
> s/unto/into/
> s/shall/will/

As long as there are no bugs, right. *g*

>> +specified as @var{backing_file} (which has to be part of @var{filename}'s
>> +backing chain). If @var{filename} is not specified, the immediate backing file
>> +of the top image (which is @var{filename}) will be used.
>> +
> With those changes,
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
Max Reitz April 10, 2014, 2:45 p.m. UTC | #4
On 10.04.2014 11:05, Fam Zheng wrote:
> On Tue, 04/08 14:50, Max Reitz wrote:
>> Introduce a new parameter for qemu-img commit which may be used to
>> explicitly specify the backing file unto which an image should be
>> committed if the backing chain has more than a single layer.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   qemu-img-cmds.hx |  4 ++--
>>   qemu-img.c       | 22 +++++++++++++++-------
>>   qemu-img.texi    |  8 +++++++-
>>   3 files changed, 24 insertions(+), 10 deletions(-)
>>
>> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
>> index 8bc55cd..7f62f6d 100644
>> --- a/qemu-img-cmds.hx
>> +++ b/qemu-img-cmds.hx
>> @@ -22,9 +22,9 @@ STEXI
>>   ETEXI
>>   
>>   DEF("commit", img_commit,
>> -    "commit [-q] [-f fmt] [-t cache] [-p] filename")
>> +    "commit [-q] [-f fmt] [-t cache] [-b backing_file] [-p] filename")
>>   STEXI
>> -@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-p] @var{filename}
>> +@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{backing_file}] [-p] @var{filename}
>>   ETEXI
>>   
>>   DEF("compare", img_compare,
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 0a9eff7..9d4bdbc 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -725,15 +725,16 @@ static void run_block_job(BlockJob *job, Error **errp)
>>   static int img_commit(int argc, char **argv)
>>   {
>>       int c, ret, flags;
>> -    const char *filename, *fmt, *cache;
>> +    const char *filename, *fmt, *cache, *base;
>>       BlockDriverState *bs, *base_bs;
>>       bool progress = false, quiet = false;
>>       Error *local_err = NULL;
>>   
>>       fmt = NULL;
>>       cache = BDRV_DEFAULT_CACHE;
>> +    base = NULL;
>>       for(;;) {
>> -        c = getopt(argc, argv, "f:ht:qp");
>> +        c = getopt(argc, argv, "f:ht:b:qp");
>>           if (c == -1) {
>>               break;
>>           }
>> @@ -748,6 +749,9 @@ static int img_commit(int argc, char **argv)
>>           case 't':
>>               cache = optarg;
>>               break;
>> +        case 'b':
>> +            base = optarg;
>> +            break;
>>           case 'p':
>>               progress = true;
>>               break;
>> @@ -782,12 +786,16 @@ static int img_commit(int argc, char **argv)
>>       qemu_progress_init(progress, 1.f);
>>       qemu_progress_print(0.f, 100);
>>   
>> -    /* This is different from QMP, which by default uses the deepest file in the
>> -     * backing chain (i.e., the very base); however, the traditional behavior of
>> -     * qemu-img commit is using the immediate backing file. */
>> -    base_bs = bs->backing_hd;
>> +    if (base) {
>> +        base_bs = bdrv_find_backing_image(bs, base);
>> +    } else {
>> +        /* This is different from QMP, which by default uses the deepest file in
>> +         * the backing chain (i.e., the very base); however, the traditional
>> +         * behavior of qemu-img commit is using the immediate backing file. */
>> +        base_bs = bs->backing_hd;
>> +    }
>>       if (!base_bs) {
>> -        error_set(&local_err, QERR_BASE_NOT_FOUND, "NULL");
>> +        error_set(&local_err, QERR_BASE_NOT_FOUND, base ?: "NULL");
>>           goto done;
>>       }
>>   
>> diff --git a/qemu-img.texi b/qemu-img.texi
>> index 1a9c08f..4a9f493 100644
>> --- a/qemu-img.texi
>> +++ b/qemu-img.texi
>> @@ -140,7 +140,7 @@ this case. @var{backing_file} will never be modified unless you use the
>>   The size can also be specified using the @var{size} option with @code{-o},
>>   it doesn't need to be specified separately in this case.
>>   
>> -@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-p] @var{filename}
>> +@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{backing_file}] [-p] @var{filename}
>>   
>>   Commit the changes recorded in @var{filename} in its base image or backing file.
>>   If the backing file is smaller than the snapshot, then the backing file will be
>> @@ -149,6 +149,12 @@ the backing file, the backing file will not be truncated.  If you want the
>>   backing file to match the size of the smaller snapshot, you can safely truncate
>>   it yourself once the commit operation successfully completes.
>>   
>> +If the backing chain of the given image file @var{filename} has more than one
>> +layer, the backing file unto which the changes shall be committed may be
>> +specified as @var{backing_file} (which has to be part of @var{filename}'s
>> +backing chain). If @var{filename} is not specified, the immediate backing file
> s/@var{filename}/@var{backing_file}/ ?

Right.

> BTW how about just calling it 'base' as in qmp commands, because backing_file
> has usages in (slightly) different context of create.

I just called it “backing_file”, as there are currently no qemu-img 
commands with a “base” parameter; then again, there aren't any qemu-img 
commands for which a filename of the backing chain may be specified, so 
you're correct.

Max

> Other than the two questions,
>
> Reviewed-by: Fam Zheng <famz@redhat.com>
>
>> +of the top image (which is @var{filename}) will be used.
>> +
>>   @item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} @var{filename2}
>>   
>>   Check if two images have the same content. You can compare images with
>> -- 
>> 1.9.1
>>
>>
diff mbox

Patch

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 8bc55cd..7f62f6d 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -22,9 +22,9 @@  STEXI
 ETEXI
 
 DEF("commit", img_commit,
-    "commit [-q] [-f fmt] [-t cache] [-p] filename")
+    "commit [-q] [-f fmt] [-t cache] [-b backing_file] [-p] filename")
 STEXI
-@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-p] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{backing_file}] [-p] @var{filename}
 ETEXI
 
 DEF("compare", img_compare,
diff --git a/qemu-img.c b/qemu-img.c
index 0a9eff7..9d4bdbc 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -725,15 +725,16 @@  static void run_block_job(BlockJob *job, Error **errp)
 static int img_commit(int argc, char **argv)
 {
     int c, ret, flags;
-    const char *filename, *fmt, *cache;
+    const char *filename, *fmt, *cache, *base;
     BlockDriverState *bs, *base_bs;
     bool progress = false, quiet = false;
     Error *local_err = NULL;
 
     fmt = NULL;
     cache = BDRV_DEFAULT_CACHE;
+    base = NULL;
     for(;;) {
-        c = getopt(argc, argv, "f:ht:qp");
+        c = getopt(argc, argv, "f:ht:b:qp");
         if (c == -1) {
             break;
         }
@@ -748,6 +749,9 @@  static int img_commit(int argc, char **argv)
         case 't':
             cache = optarg;
             break;
+        case 'b':
+            base = optarg;
+            break;
         case 'p':
             progress = true;
             break;
@@ -782,12 +786,16 @@  static int img_commit(int argc, char **argv)
     qemu_progress_init(progress, 1.f);
     qemu_progress_print(0.f, 100);
 
-    /* This is different from QMP, which by default uses the deepest file in the
-     * backing chain (i.e., the very base); however, the traditional behavior of
-     * qemu-img commit is using the immediate backing file. */
-    base_bs = bs->backing_hd;
+    if (base) {
+        base_bs = bdrv_find_backing_image(bs, base);
+    } else {
+        /* This is different from QMP, which by default uses the deepest file in
+         * the backing chain (i.e., the very base); however, the traditional
+         * behavior of qemu-img commit is using the immediate backing file. */
+        base_bs = bs->backing_hd;
+    }
     if (!base_bs) {
-        error_set(&local_err, QERR_BASE_NOT_FOUND, "NULL");
+        error_set(&local_err, QERR_BASE_NOT_FOUND, base ?: "NULL");
         goto done;
     }
 
diff --git a/qemu-img.texi b/qemu-img.texi
index 1a9c08f..4a9f493 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -140,7 +140,7 @@  this case. @var{backing_file} will never be modified unless you use the
 The size can also be specified using the @var{size} option with @code{-o},
 it doesn't need to be specified separately in this case.
 
-@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-p] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{backing_file}] [-p] @var{filename}
 
 Commit the changes recorded in @var{filename} in its base image or backing file.
 If the backing file is smaller than the snapshot, then the backing file will be
@@ -149,6 +149,12 @@  the backing file, the backing file will not be truncated.  If you want the
 backing file to match the size of the smaller snapshot, you can safely truncate
 it yourself once the commit operation successfully completes.
 
+If the backing chain of the given image file @var{filename} has more than one
+layer, the backing file unto which the changes shall be committed may be
+specified as @var{backing_file} (which has to be part of @var{filename}'s
+backing chain). If @var{filename} is not specified, the immediate backing file
+of the top image (which is @var{filename}) will be used.
+
 @item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} @var{filename2}
 
 Check if two images have the same content. You can compare images with