Re: [Qemu-devel] [PATCH] qemu: new option for snapshot_blkdev to avoid image creation

[Dor Laor <dlaor@redhat.com>]

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,
>       },