[Qemu-devel] [6215] snapshot subcommand for qemu-img (Kevin Wolf)

[Qemu-devel] [6215] snapshot subcommand for qemu-img (Kevin Wolf)

[Anthony Liguori]

Revision: 6215
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6215
Author:   aliguori
Date:     2009-01-07 17:40:15 +0000 (Wed, 07 Jan 2009)

Log Message:
-----------
snapshot subcommand for qemu-img (Kevin Wolf)

Add snapshot subcommand to qemu-img which allows to list, create, apply
and delete snapshots on qcow2 images.

Signed-off-by: Kevin Wolf <kwolf@suse.de>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Modified Paths:
--------------
    trunk/osdep.h
    trunk/qemu-img.c

Modified: trunk/osdep.h
===================================================================
--- trunk/osdep.h	2009-01-07 17:32:33 UTC (rev 6214)
+++ trunk/osdep.h	2009-01-07 17:40:15 UTC (rev 6215)
@@ -7,6 +7,10 @@
 #include <sys/signal.h>
 #endif
 
+#ifndef _WIN32
+#include <sys/time.h>
+#endif
+
 #ifndef glue
 #define xglue(x, y) x ## y
 #define glue(x, y) xglue(x, y)

Modified: trunk/qemu-img.c
===================================================================
--- trunk/qemu-img.c	2009-01-07 17:32:33 UTC (rev 6214)
+++ trunk/qemu-img.c	2009-01-07 17:40:15 UTC (rev 6215)
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu-common.h"
+#include "osdep.h"
 #include "block_int.h"
 #include <assert.h>
 
@@ -60,6 +61,7 @@
            "  commit [-f fmt] filename\n"
            "  convert [-c] [-e] [-6] [-f fmt] [-O output_fmt] [-B output_base_image] filename [filename2 [...]] output_filename\n"
            "  info [-f fmt] filename\n"
+           "  snapshot [-l|-a snapshot|-c snapshot|-d snapshot] filename\n"
            "\n"
            "Command parameters:\n"
            "  'filename' is a disk image filename\n"
@@ -77,6 +79,13 @@
            "  '-c' indicates that target image must be compressed (qcow format only)\n"
            "  '-e' indicates that the target image must be encrypted (qcow format only)\n"
            "  '-6' indicates that the target image must use compatibility level 6 (vmdk format only)\n"
+           "\n"
+           "  Parameters to snapshot subcommand:\n"
+           "    'snapshot' is the name of the snapshot to create, apply or delete\n"
+           "    '-a' applies a snapshot (revert disk to saved state)\n"
+           "    '-c' creates a snapshot\n"
+           "    '-d' deletes a snapshot\n"
+           "    '-l' lists all snapshots in the given image\n"
            );
     printf("\nSupported format:");
     bdrv_iterate_format(format_print, NULL);
@@ -732,6 +741,116 @@
     return 0;
 }
 
+#define SNAPSHOT_LIST   1
+#define SNAPSHOT_CREATE 2
+#define SNAPSHOT_APPLY  3
+#define SNAPSHOT_DELETE 4
+
+static void img_snapshot(int argc, char **argv)
+{
+    BlockDriverState *bs;
+    QEMUSnapshotInfo sn;
+    char *filename, *snapshot_name = NULL;
+    char c;
+    int ret;
+    int action = 0;
+    qemu_timeval tv;
+
+    /* Parse commandline parameters */
+    for(;;) {
+        c = getopt(argc, argv, "la:c:d:h");
+        if (c == -1)
+            break;
+        switch(c) {
+        case 'h':
+            help();
+            return;
+        case 'l':
+            if (action) {
+                help();
+                return;
+            }
+            action = SNAPSHOT_LIST;
+            break;
+        case 'a':
+            if (action) {
+                help();
+                return;
+            }
+            action = SNAPSHOT_APPLY;
+            snapshot_name = optarg;
+            break;
+        case 'c':
+            if (action) {
+                help();
+                return;
+            }
+            action = SNAPSHOT_CREATE;
+            snapshot_name = optarg;
+            break;
+        case 'd':
+            if (action) {
+                help();
+                return;
+            }
+            action = SNAPSHOT_DELETE;
+            snapshot_name = optarg;
+            break;
+        }
+    }
+
+    if (optind >= argc)
+        help();
+    filename = argv[optind++];
+
+    /* Open the image */
+    bs = bdrv_new("");
+    if (!bs)
+        error("Not enough memory");
+
+    if (bdrv_open2(bs, filename, 0, NULL) < 0) {
+        error("Could not open '%s'", filename);
+    }
+
+    /* Perform the requested action */
+    switch(action) {
+    case SNAPSHOT_LIST:
+        dump_snapshots(bs);
+        break;
+
+    case SNAPSHOT_CREATE:
+        memset(&sn, 0, sizeof(sn));
+        pstrcpy(sn.name, sizeof(sn.name), snapshot_name);
+
+        qemu_gettimeofday(&tv);
+        sn.date_sec = tv.tv_sec;
+        sn.date_nsec = tv.tv_usec * 1000;
+
+        ret = bdrv_snapshot_create(bs, &sn);
+        if (ret)
+            error("Could not create snapshot '%s': %d (%s)",
+                snapshot_name, ret, strerror(-ret));
+        break;
+
+    case SNAPSHOT_APPLY:
+        ret = bdrv_snapshot_goto(bs, snapshot_name);
+        if (ret)
+            error("Could not apply snapshot '%s': %d (%s)",
+                snapshot_name, ret, strerror(-ret));
+        break;
+
+    case SNAPSHOT_DELETE:
+        ret = bdrv_snapshot_delete(bs, snapshot_name);
+        if (ret)
+            error("Could not delete snapshot '%s': %d (%s)",
+                snapshot_name, ret, strerror(-ret));
+        break;
+    }
+
+    /* Cleanup */
+    bdrv_delete(bs);
+}
+
 int main(int argc, char **argv)
 {
     const char *cmd;
@@ -749,6 +868,8 @@
         img_convert(argc, argv);
     } else if (!strcmp(cmd, "info")) {
         img_info(argc, argv);
+    } else if (!strcmp(cmd, "snapshot")) {
+        img_snapshot(argc, argv);
     } else {
         help();
     }

Re: [Qemu-devel] [6215] snapshot subcommand for qemu-img (Kevin Wolf)

[Anthony Liguori]

Anthony Liguori wrote:
> +static void img_snapshot(int argc, char **argv)
> +{
> +    BlockDriverState *bs;
> +    QEMUSnapshotInfo sn;
> +    char *filename, *snapshot_name = NULL;
> +    char c;
> +    int ret;
> +    int action = 0;
> +    qemu_timeval tv;
> +
> +    /* Parse commandline parameters */
> +    for(;;) {
> +        c = getopt(argc, argv, "la:c:d:h");
> +        if (c == -1)
> +            break;
>   

char's are not always signed so this code is incorrect.  You should use 
an int instead.  Can you send another patch fixing this please?

Regards,

Anthony Liguori

Re: [Qemu-devel] [6215] snapshot subcommand for qemu-img (Kevin Wolf)

[M. Warner Losh]

In message: <496CE528.5090509@codemonkey.ws>
            Anthony Liguori <anthony@codemonkey.ws> writes:
: Anthony Liguori wrote:
: > +static void img_snapshot(int argc, char **argv)
: > +{
: > +    BlockDriverState *bs;
: > +    QEMUSnapshotInfo sn;
: > +    char *filename, *snapshot_name = NULL;
: > +    char c;
: > +    int ret;
: > +    int action = 0;
: > +    qemu_timeval tv;
: > +
: > +    /* Parse commandline parameters */
: > +    for(;;) {
: > +        c = getopt(argc, argv, "la:c:d:h");
: > +        if (c == -1)
: > +            break;
: >   
: 
: char's are not always signed so this code is incorrect.  You should use 
: an int instead.  Can you send another patch fixing this please?

More importantly, getopt returns an 'int' not a 'signed char'.  This
means it can report values that can't be represented by a 'signed
char'.

Warner

[Qemu-devel] [PATCH] qemu-img: Fix type of getopt return value

[Kevin Wolf]

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

Anthony Liguori schrieb:
> Anthony Liguori wrote:
>> +static void img_snapshot(int argc, char **argv)
>> +{
>> +    BlockDriverState *bs;
>> +    QEMUSnapshotInfo sn;
>> +    char *filename, *snapshot_name = NULL;
>> +    char c;
>> +    int ret;
>> +    int action = 0;
>> +    qemu_timeval tv;
>> +
>> +    /* Parse commandline parameters */
>> +    for(;;) {
>> +        c = getopt(argc, argv, "la:c:d:h");
>> +        if (c == -1)
>> +            break;
>>   
> 
> char's are not always signed so this code is incorrect.  You should use
> an int instead.  Can you send another patch fixing this please?

You're right. I better should have copied not only the parsing but also
the declarations... Patch is attached.

Kevin

[-- Attachment #2: qemu-img-fix-type.patch --]
[-- Type: text/x-patch, Size: 733 bytes --]

>From 6fd77e3094a6bdd94e353ac7380cac70f5490024 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@suse.de>
Date: Wed, 14 Jan 2009 09:39:33 +0100
Subject: [PATCH] qemu-img: Fix type of getopt return value

getopt doesn't return a char but an int.

Signed-off-by: Kevin Wolf <kwolf@suse.de>
---
 qemu-img.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 964b28b..ed61d3a 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -751,8 +751,7 @@ static void img_snapshot(int argc, char **argv)
     BlockDriverState *bs;
     QEMUSnapshotInfo sn;
     char *filename, *snapshot_name = NULL;
-    char c;
-    int ret;
+    int c, ret;
     int action = 0;
     qemu_timeval tv;
 
-- 
1.6.0.2

[Qemu-devel] Re: [PATCH] qemu-img: Fix type of getopt return value

[Anthony Liguori]

Kevin Wolf wrote:
> Anthony Liguori schrieb:
>   
>> Anthony Liguori wrote:
>>     
>>> +static void img_snapshot(int argc, char **argv)
>>> +{
>>> +    BlockDriverState *bs;
>>> +    QEMUSnapshotInfo sn;
>>> +    char *filename, *snapshot_name = NULL;
>>> +    char c;
>>> +    int ret;
>>> +    int action = 0;
>>> +    qemu_timeval tv;
>>> +
>>> +    /* Parse commandline parameters */
>>> +    for(;;) {
>>> +        c = getopt(argc, argv, "la:c:d:h");
>>> +        if (c == -1)
>>> +            break;
>>>   
>>>       
>> char's are not always signed so this code is incorrect.  You should use
>> an int instead.  Can you send another patch fixing this please?
>>     
>
> You're right. I better should have copied not only the parsing but also
> the declarations... Patch is attached.
>   

Applied.  Thanks.

Regards,

Anthony Liguori

> Kevin
>