Re: [Qemu-devel] [PATCH 7/8] qemu: command line option for dirty bitmaps

[John Snow]

On 06/08/2015 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
> From: Vladimir Sementsov-Ogievskiy <address@hidden>
> 
> The patch adds the following command line option:
> 
> -dirty-bitmap [option1=val1][,option2=val2]...
>     Available options are:
>     name         The name for the bitmap (necessary).
> 
>     file         The file to load the bitmap from.
> 
>     file_id      When specified with 'file' option, then this file will
>                  be available through this id for other -dirty-bitmap
>                  options when specified without 'file' option, then it
>                  is a reference to 'file', specified with another
>                  -dirty-bitmap option, and it will be used to load the
>                  bitmap from.
> 
>     drive        The drive to bind the bitmap to. It should be specified
>                  as 'id' suboption of one of -drive options. If nor
>                  'file' neither 'file_id' are specified, then the bitmap
>                  will be loaded from that drive (internal dirty bitmap).
> 
>     granularity  The granularity for the bitmap. Not necessary, the
>                  default value may be used.
> 
>     enabled      on|off. Default is 'on'. Disabled bitmaps are not
>                  changing regardless of writes to corresponding drive.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  blockdev.c                |  38 ++++++++++++++++++
>  include/sysemu/blockdev.h |   1 +
>  include/sysemu/sysemu.h   |   1 +
>  qemu-options.hx           |  37 +++++++++++++++++
>  vl.c                      | 100 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 177 insertions(+)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 5eaf77e..2a74395 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -176,6 +176,11 @@ QemuOpts *drive_def(const char *optstr)
>      return qemu_opts_parse(qemu_find_opts("drive"), optstr, 0);
>  }
>  
> +QemuOpts *dirty_bitmap_def(const char *optstr)
> +{
> +    return qemu_opts_parse(qemu_find_opts("dirty-bitmap"), optstr, 0);
> +}
> +
>  QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
>                      const char *optstr)
>  {
> @@ -3093,6 +3098,39 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
>      return head;
>  }
>  
> +QemuOptsList qemu_dirty_bitmap_opts = {
> +    .name = "dirty-bitmap",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_dirty_bitmap_opts.head),
> +    .desc = {
> +        {
> +            .name = "name",
> +            .type = QEMU_OPT_STRING,
> +            .help = "Name of the dirty bitmap",
> +        },{
> +            .name = "file",
> +            .type = QEMU_OPT_STRING,
> +            .help = "file name to load the bitmap from",
> +        },{
> +            .name = "file_id",
> +            .type = QEMU_OPT_STRING,
> +            .help = "node name to load the bitmap from (or to set id for"
> +                    " for file, opened by previous option)",
> +        },{
> +            .name = "drive",
> +            .type = QEMU_OPT_STRING,
> +            .help = "drive id to bind the bitmap to",
> +        },{
> +            .name = "granularity",
> +            .type = QEMU_OPT_NUMBER,
> +            .help = "granularity",
> +        },{
> +            .name = "enabled",
> +            .type = QEMU_OPT_BOOL,
> +            .help = "enabled flag (default is 'on')",
> +        }
> +    }
> +};
> +
>  QemuOptsList qemu_common_drive_opts = {
>      .name = "drive",
>      .head = QTAILQ_HEAD_INITIALIZER(qemu_common_drive_opts.head),
> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> index 7ca59b5..5b101b8 100644
> --- a/include/sysemu/blockdev.h
> +++ b/include/sysemu/blockdev.h
> @@ -57,6 +57,7 @@ int drive_get_max_devs(BlockInterfaceType type);
>  DriveInfo *drive_get_next(BlockInterfaceType type);
>  
>  QemuOpts *drive_def(const char *optstr);
> +QemuOpts *dirty_bitmap_def(const char *optstr);
>  QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
>                      const char *optstr);
>  DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type);
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 8a52934..681a8f3 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -207,6 +207,7 @@ bool usb_enabled(void);
>  
>  extern QemuOptsList qemu_legacy_drive_opts;
>  extern QemuOptsList qemu_common_drive_opts;
> +extern QemuOptsList qemu_dirty_bitmap_opts;
>  extern QemuOptsList qemu_drive_opts;
>  extern QemuOptsList qemu_chardev_opts;
>  extern QemuOptsList qemu_device_opts;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index ec356f6..5e93122 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -614,6 +614,43 @@ qemu-system-i386 -hda a -hdb b
>  @end example
>  ETEXI
>  
> +DEF("dirty-bitmap", HAS_ARG, QEMU_OPTION_dirty_bitmap,
> +    "-dirty-bitmap name=name[,file=file][,file_id=file_id][,address@hidden"
> +    "              [,granularity=granularity][,enabled=on|off]\n",
> +    QEMU_ARCH_ALL)
> +STEXI
> address@hidden -dirty-bitmap @var{option}[,@var{option}[,@var{option}[,...]]]
> address@hidden -dirty-bitmap
> +
> +Define a dirty-bitmap. Valid options are:
> +
> address@hidden @option
> address@hidden address@hidden
> +The name of the bitmap. Should be unique per @var{file}/@var{drive} and per
> address@hidden
> address@hidden address@hidden
> +The separate qcow2 file for loading the bitmap @var{name} from it.
> address@hidden address@hidden
> +When specified with @var{file} option, then this @var{file} will be available
> +through this @var{file_id} for other @option{-dirty-bitmap} options.
> +When specified without @var{file} option, then it is a reference to 
> @var{file},
> +specified with another @option{-dirty-bitmap} option, and it will be used to
> +load the bitmap from.
> address@hidden address@hidden
> +The drive to bind the bitmap to. It should be specified as @var{id} suboption
> +of one of @option{-drive} options.
> +If nor @var{file} neither @var{file_id} are specified, then the bitmap will 
> be
> +loaded from that drive (internal dirty bitmap).
> address@hidden address@hidden
> +Granularity (in bytes) for created dirty bitmap. If the bitmap is already
> +exists in specified @var{file}/@var{file_id}/@var{device} it's granularity 
> will
> +not be changed but only checked (an error will be generated if this check
> +fails).
> address@hidden address@hidden
> +Enabled flag for the bitmap. By default the bitmap will be enabled.
> address@hidden table
> +ETEXI
> +
>  DEF("mtdblock", HAS_ARG, QEMU_OPTION_mtdblock,
>      "-mtdblock file  use 'file' as on-board Flash memory image\n",
>      QEMU_ARCH_ALL)
> diff --git a/vl.c b/vl.c
> index 83871f5..fb16d0c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1091,6 +1091,95 @@ static int cleanup_add_fd(QemuOpts *opts, void *opaque)
>  #define MTD_OPTS ""
>  #define SD_OPTS ""
>  
> +static int dirty_bitmap_func(QemuOpts *opts, void *opaque)
> +{
> +    Error *local_err = NULL;
> +    Error **errp = &local_err;
> +    BlockDriverState *file_bs = NULL, *for_bs = NULL;
> +    BdrvDirtyBitmap *bitmap = NULL;
> +
> +    const char *name = qemu_opt_get(opts, "name");
> +    const char *drive = qemu_opt_get(opts, "drive");
> +    const char *file = qemu_opt_get(opts, "file");
> +    const char *file_id = qemu_opt_get(opts, "file_id");
> +
> +    uint64_t granularity = qemu_opt_get_number(opts, "granularity", 0);
> +    bool enabled = qemu_opt_get_bool(opts, "enabled", true);
> +
> +    if (name == NULL) {
> +        error_setg(errp, "'name' option is necessary");
> +        goto fail;
> +    }
> +
> +    if (drive == NULL) {
> +        error_setg(errp, "'drive' option is necessary");
> +        goto fail;
> +    }
> +
> +    for_bs = bdrv_lookup_bs(drive, NULL, errp);
> +    if (for_bs == NULL) {
> +        goto fail;
> +    }
> +
> +    if (file != NULL) {
> +        QDict *options = NULL;
> +        if (file_id != NULL) {
> +            options = qdict_new();
> +            qdict_put(options, "node-name", qstring_from_str(file_id));
> +        }
> +
> +        bdrv_open(&file_bs, file, NULL, options, 0, NULL, errp);

This will open the file read-only without BDRV_O_RDWR in the flags field
(at least), so this doesn't work.

Please add a test case for specifying file= in addition to the current
one that tests the "file_bs = for_bs" case below.

> +        if (options) {
> +            QDECREF(options);
> +        }
> +        if (file_bs == NULL) {
> +            goto fail;
> +        }
> +    } else if (file_id != NULL) {
> +        file_bs = bdrv_find_node(file_id);
> +        if (file_bs == NULL) {
> +            error_setg(errp, "node '%s' is not found", drive);
> +            goto fail;
> +        }
> +    } else {
> +        file_bs = for_bs;
> +    }
> +
> +    if (granularity == 0) {
> +        granularity = bdrv_get_default_bitmap_granularity(for_bs);
> +    }
> +
> +    bitmap = bdrv_load_dirty_bitmap(for_bs, file_bs, granularity, name,
> +                                    errp);
> +    if (*errp != NULL) {
> +        goto fail;
> +    }
> +
> +    if (bitmap == NULL) {
> +        /* bitmap is not found in file_bs */
> +        bitmap = bdrv_create_dirty_bitmap(for_bs, granularity, name, errp);
> +        if (!bitmap) {
> +            goto fail;
> +        }
> +    }
> +
> +    bdrv_dirty_bitmap_set_file(bitmap, file_bs);
> +
> +    if (!enabled) {
> +        bdrv_disable_dirty_bitmap(bitmap);
> +    }
> +
> +    return 0;
> +
> +fail:
> +    error_report("-dirty-bitmap: %s", error_get_pretty(local_err));
> +    error_free(local_err);
> +    if (file_bs != NULL) {
> +        bdrv_close(file_bs);
> +    }
> +    return -1;
> +}
> +
>  static int drive_init_func(QemuOpts *opts, void *opaque)
>  {
>      BlockInterfaceType *block_default_type = opaque;
> @@ -2790,6 +2879,7 @@ int main(int argc, char **argv, char **envp)
>      module_call_init(MODULE_INIT_QOM);
>  
>      qemu_add_opts(&qemu_drive_opts);
> +    qemu_add_opts(&qemu_dirty_bitmap_opts);
>      qemu_add_drive_opts(&qemu_legacy_drive_opts);
>      qemu_add_drive_opts(&qemu_common_drive_opts);
>      qemu_add_drive_opts(&qemu_drive_opts);
> @@ -2918,6 +3008,11 @@ int main(int argc, char **argv, char **envp)
>                      exit(1);
>                  }
>                  break;
> +            case QEMU_OPTION_dirty_bitmap:
> +                if (dirty_bitmap_def(optarg) == NULL) {
> +                    exit(1);
> +                }
> +                break;
>              case QEMU_OPTION_set:
>                  if (qemu_set_option(optarg) != 0)
>                      exit(1);
> @@ -4198,6 +4293,11 @@ int main(int argc, char **argv, char **envp)
>  
>      parse_numa_opts(machine_class);
>  
> +    if (qemu_opts_foreach(qemu_find_opts("dirty-bitmap"), dirty_bitmap_func,
> +                          NULL, 1) != 0) {
> +        exit(1);
> +    }
> +
>      if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 
> 0) {
>          exit(1);
>      }
> 

-- 
—js