Patchwork qemu: new option for snapshot_blkdev to avoid image creation

login
register
mail settings
Submitter Federico Simoncelli
Date Oct. 3, 2011, 4:09 p.m.
Message ID <1317658142-21383-2-git-send-email-fsimonce@redhat.com>
Download mbox | patch
Permalink /patch/117478/
State New
Headers show

Comments

Federico Simoncelli - Oct. 3, 2011, 4:09 p.m.
Add the new option [-n] for snapshot_blkdev to avoid the image creation.
The file provided as [new-image-file] is considered as already initialized
and will be used after passing a check for the backing file.

Signed-off-by: Federico Simoncelli <fsimonce@redhat.com>
---
 blockdev.c      |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 hmp-commands.hx |    7 ++++---
 qmp-commands.hx |    4 ++--
 3 files changed, 58 insertions(+), 7 deletions(-)
Dor Laor - Oct. 6, 2011, 10:45 p.m.
On 10/03/2011 06:09 PM, Federico Simoncelli wrote:
> Add the new option [-n] for snapshot_blkdev to avoid the image creation.
> The file provided as [new-image-file] is considered as already initialized
> and will be used after passing a check for the backing file.

Seems ok to me as a way to go around fdget and still have selinux gain.
Worth to get Kevin's view too.

Federico, would you like to ack or extend the design:
http://wiki.qemu.org/Features/Snapshots

>
> Signed-off-by: Federico Simoncelli<fsimonce@redhat.com>
> ---
>   blockdev.c      |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
>   hmp-commands.hx |    7 ++++---
>   qmp-commands.hx |    4 ++--
>   3 files changed, 58 insertions(+), 7 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 0827bf7..bd46808 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -550,8 +550,53 @@ void do_commit(Monitor *mon, const QDict *qdict)
>       }
>   }
>
> +static int check_snapshot_file(const char *filename, const char *oldfilename,
> +                               int flags, BlockDriver *drv)
> +{
> +    BlockDriverState *bs;
> +    char bak_filename[1024], *abs_filename;
> +    int ret = 0;
> +
> +    bs = bdrv_new("");
> +    if (!bs) {
> +        return -1;
> +    }
> +
> +    ret = bdrv_open(bs, filename, flags, drv);
> +    if (ret) {
> +        qerror_report(QERR_OPEN_FILE_FAILED, filename);
> +        goto err0;
> +    }
> +
> +    if (bs->backing_file) {
> +        path_combine(bak_filename, sizeof(bak_filename),
> +                     filename, bs->backing_file);
> +
> +        abs_filename = realpath(bak_filename, NULL);
> +        if (!abs_filename) {
> +            ret = -1;
> +            goto err1;
> +        }
> +
> +        if (strcmp(abs_filename, oldfilename)) {
> +            qerror_report(QERR_OPEN_FILE_FAILED, filename);
> +            ret = -1;
> +        }
> +
> +        free(abs_filename);
> +    }
> +
> +err1:
> +    bdrv_close(bs);
> +
> +err0:
> +    bdrv_delete(bs);
> +    return ret;
> +}
> +
>   int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
>   {
> +    const int nocreate = qdict_get_try_bool(qdict, "nocreate", 0);
>       const char *device = qdict_get_str(qdict, "device");
>       const char *filename = qdict_get_try_str(qdict, "snapshot-file");
>       const char *format = qdict_get_try_str(qdict, "format");
> @@ -597,8 +642,13 @@ int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
>           goto out;
>       }
>
> -    ret = bdrv_img_create(filename, format, bs->filename,
> -                          bs->drv->format_name, NULL, -1, flags);
> +    if (nocreate) {
> +        ret = check_snapshot_file(filename, bs->filename, flags, drv);
> +    } else {
> +        ret = bdrv_img_create(filename, format, bs->filename,
> +                              bs->drv->format_name, NULL, -1, flags);
> +    }
> +
>       if (ret) {
>           goto out;
>       }
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 9e1cca8..eb9fcd4 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -840,11 +840,12 @@ ETEXI
>
>       {
>           .name       = "snapshot_blkdev",
> -        .args_type  = "device:B,snapshot-file:s?,format:s?",
> -        .params     = "device [new-image-file] [format]",
> +        .args_type  = "nocreate:-n,device:B,snapshot-file:s?,format:s?",
> +        .params     = "[-n] device [new-image-file] [format]",
>           .help       = "initiates a live snapshot\n\t\t\t"
>                         "of device. If a new image file is specified, the\n\t\t\t"
> -                      "new image file will become the new root image.\n\t\t\t"
> +                      "new image file will be created (unless -n is\n\t\t\t"
> +                      "specified) and will become the new root image.\n\t\t\t"
>                         "If format is specified, the snapshot file will\n\t\t\t"
>                         "be created in that format. Otherwise the\n\t\t\t"
>                         "snapshot will be internal! (currently unsupported)",
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index d83bce5..7af36d8 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -695,8 +695,8 @@ EQMP
>
>       {
>           .name       = "blockdev-snapshot-sync",
> -        .args_type  = "device:B,snapshot-file:s?,format:s?",
> -        .params     = "device [new-image-file] [format]",
> +        .args_type  = "nocreate:-n,device:B,snapshot-file:s?,format:s?",
> +        .params     = "[-n] device [new-image-file] [format]",
>           .user_print = monitor_user_noop,
>           .mhandler.cmd_new = do_snapshot_blkdev,
>       },
Federico Simoncelli - Oct. 11, 2011, 11:56 a.m.
----- Original Message -----
> From: "Dor Laor" <dlaor@redhat.com>
> To: "Federico Simoncelli" <fsimonce@redhat.com>
> Cc: qemu-devel@nongnu.org, abaron@redhat.com, "Kevin Wolf" <kwolf@redhat.com>
> Sent: Friday, October 7, 2011 12:45:06 AM
> Subject: Re: [Qemu-devel] [PATCH] qemu: new option for snapshot_blkdev to avoid image creation
> 
> On 10/03/2011 06:09 PM, Federico Simoncelli wrote:
> > Add the new option [-n] for snapshot_blkdev to avoid the image
> > creation.
> > The file provided as [new-image-file] is considered as already
> > initialized
> > and will be used after passing a check for the backing file.
> 
> Seems ok to me as a way to go around fdget and still have selinux
> gain.
> Worth to get Kevin's view too.

Hi Kevin, any news on this?

> Federico, would you like to ack or extend the design:
> http://wiki.qemu.org/Features/Snapshots

Dor, should I do it now or only after my patch is acked?
Dor Laor - Oct. 11, 2011, noon
On 10/11/2011 01:56 PM, Federico Simoncelli wrote:
> ----- Original Message -----
>> From: "Dor Laor"<dlaor@redhat.com>
>> To: "Federico Simoncelli"<fsimonce@redhat.com>
>> Cc: qemu-devel@nongnu.org, abaron@redhat.com, "Kevin Wolf"<kwolf@redhat.com>
>> Sent: Friday, October 7, 2011 12:45:06 AM
>> Subject: Re: [Qemu-devel] [PATCH] qemu: new option for snapshot_blkdev to avoid image creation
>>
>> On 10/03/2011 06:09 PM, Federico Simoncelli wrote:
>>> Add the new option [-n] for snapshot_blkdev to avoid the image
>>> creation.
>>> The file provided as [new-image-file] is considered as already
>>> initialized
>>> and will be used after passing a check for the backing file.
>>
>> Seems ok to me as a way to go around fdget and still have selinux
>> gain.
>> Worth to get Kevin's view too.
>
> Hi Kevin, any news on this?
>
>> Federico, would you like to ack or extend the design:
>> http://wiki.qemu.org/Features/Snapshots
>
> Dor, should I do it now or only after my patch is acked?

We're not that strict in the OSS world ;) you can do it before and mark 
as a pending request.
btw: Kevin is back so he will probably drain his queue soon.

Patch

diff --git a/blockdev.c b/blockdev.c
index 0827bf7..bd46808 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -550,8 +550,53 @@  void do_commit(Monitor *mon, const QDict *qdict)
     }
 }
 
+static int check_snapshot_file(const char *filename, const char *oldfilename,
+                               int flags, BlockDriver *drv)
+{
+    BlockDriverState *bs;
+    char bak_filename[1024], *abs_filename;
+    int ret = 0;
+
+    bs = bdrv_new("");
+    if (!bs) {
+        return -1;
+    }
+
+    ret = bdrv_open(bs, filename, flags, drv);
+    if (ret) {
+        qerror_report(QERR_OPEN_FILE_FAILED, filename);
+        goto err0;
+    }
+
+    if (bs->backing_file) {
+        path_combine(bak_filename, sizeof(bak_filename),
+                     filename, bs->backing_file);
+
+        abs_filename = realpath(bak_filename, NULL);
+        if (!abs_filename) {
+            ret = -1;
+            goto err1;
+        }
+
+        if (strcmp(abs_filename, oldfilename)) {
+            qerror_report(QERR_OPEN_FILE_FAILED, filename);
+            ret = -1;
+        }
+
+        free(abs_filename);
+    }
+
+err1:
+    bdrv_close(bs);
+
+err0:
+    bdrv_delete(bs);
+    return ret;
+}
+
 int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
+    const int nocreate = qdict_get_try_bool(qdict, "nocreate", 0);
     const char *device = qdict_get_str(qdict, "device");
     const char *filename = qdict_get_try_str(qdict, "snapshot-file");
     const char *format = qdict_get_try_str(qdict, "format");
@@ -597,8 +642,13 @@  int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
         goto out;
     }
 
-    ret = bdrv_img_create(filename, format, bs->filename,
-                          bs->drv->format_name, NULL, -1, flags);
+    if (nocreate) {
+        ret = check_snapshot_file(filename, bs->filename, flags, drv);
+    } else {
+        ret = bdrv_img_create(filename, format, bs->filename,
+                              bs->drv->format_name, NULL, -1, flags);
+    }
+
     if (ret) {
         goto out;
     }
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9e1cca8..eb9fcd4 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -840,11 +840,12 @@  ETEXI
 
     {
         .name       = "snapshot_blkdev",
-        .args_type  = "device:B,snapshot-file:s?,format:s?",
-        .params     = "device [new-image-file] [format]",
+        .args_type  = "nocreate:-n,device:B,snapshot-file:s?,format:s?",
+        .params     = "[-n] device [new-image-file] [format]",
         .help       = "initiates a live snapshot\n\t\t\t"
                       "of device. If a new image file is specified, the\n\t\t\t"
-                      "new image file will become the new root image.\n\t\t\t"
+                      "new image file will be created (unless -n is\n\t\t\t"
+                      "specified) and will become the new root image.\n\t\t\t"
                       "If format is specified, the snapshot file will\n\t\t\t"
                       "be created in that format. Otherwise the\n\t\t\t"
                       "snapshot will be internal! (currently unsupported)",
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d83bce5..7af36d8 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -695,8 +695,8 @@  EQMP
 
     {
         .name       = "blockdev-snapshot-sync",
-        .args_type  = "device:B,snapshot-file:s?,format:s?",
-        .params     = "device [new-image-file] [format]",
+        .args_type  = "nocreate:-n,device:B,snapshot-file:s?,format:s?",
+        .params     = "[-n] device [new-image-file] [format]",
         .user_print = monitor_user_noop,
         .mhandler.cmd_new = do_snapshot_blkdev,
     },