On Tue, Jan 14, 2020 at 09:33:41PM +0200, Maxim Levitsky wrote:
> Some options are only useful for creation
> (or hard to be amended, like cluster size for qcow2), while some other
> options are only useful for amend, like upcoming keyslot management
> options for luks
>
> Since currently only qcow2 supports amend, move all its options
> to a common macro and then include it in each action option list.
>
> In future it might be useful to remove some options which are
> not supported anyway from amend list, which currently
> cause an error message if amended.
I think I would have done that in this commit. At least the
encrypt.* options shouldn't be added to the amend_opts list,
since they're being removed from it again a few patches later.
>
> Signed-off-by: Maxim Levitsky <mlevi...@redhat.com>
> ---
> block/qcow2.c | 160 +++++++++++++++++++++-----------------
> include/block/block_int.h | 4 +
> qemu-img.c | 18 ++---
> 3 files changed, 100 insertions(+), 82 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 6bcf4a5fc4..c6c2deee75 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -5445,83 +5445,96 @@ void qcow2_signal_corruption(BlockDriverState *bs,
> bool fatal, int64_t offset,
> s->signaled_corruption = true;
> }
>
> +#define QCOW_COMMON_OPTIONS \
> + { \
> + .name = BLOCK_OPT_SIZE, \
> + .type = QEMU_OPT_SIZE, \
> + .help = "Virtual disk size" \
> + }, \
> + { \
> + .name = BLOCK_OPT_COMPAT_LEVEL, \
> + .type = QEMU_OPT_STRING, \
> + .help = "Compatibility level (v2 [0.10] or v3 [1.1])" \
> + }, \
> + { \
> + .name = BLOCK_OPT_BACKING_FILE, \
> + .type = QEMU_OPT_STRING, \
> + .help = "File name of a base image" \
> + }, \
> + { \
> + .name = BLOCK_OPT_BACKING_FMT, \
> + .type = QEMU_OPT_STRING, \
> + .help = "Image format of the base image" \
> + }, \
> + { \
> + .name = BLOCK_OPT_DATA_FILE, \
> + .type = QEMU_OPT_STRING, \
> + .help = "File name of an external data file" \
> + }, \
> + { \
> + .name = BLOCK_OPT_DATA_FILE_RAW, \
> + .type = QEMU_OPT_BOOL, \
> + .help = "The external data file must stay valid " \
> + "as a raw image" \
> + }, \
> + { \
> + .name = BLOCK_OPT_ENCRYPT, \
> + .type = QEMU_OPT_BOOL, \
> + .help = "Encrypt the image with format 'aes'. (Deprecated " \
> + "in favor of " BLOCK_OPT_ENCRYPT_FORMAT "=aes)", \
> + }, \
> + { \
> + .name = BLOCK_OPT_ENCRYPT_FORMAT, \
> + .type = QEMU_OPT_STRING, \
> + .help = "Encrypt the image, format choices: 'aes', 'luks'", \
> + }, \
> + BLOCK_CRYPTO_OPT_DEF_KEY_SECRET("encrypt.", \
> + "ID of secret providing qcow AES key or LUKS passphrase"), \
> + BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_ALG("encrypt."), \
> + BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_MODE("encrypt."), \
> + BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_ALG("encrypt."), \
> + BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG("encrypt."), \
> + BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG("encrypt."), \
> + BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME("encrypt."), \
> + { \
> + .name = BLOCK_OPT_CLUSTER_SIZE, \
> + .type = QEMU_OPT_SIZE, \
> + .help = "qcow2 cluster size", \
> + .def_value_str = stringify(DEFAULT_CLUSTER_SIZE) \
> + }, \
> + { \
> + .name = BLOCK_OPT_PREALLOC, \
> + .type = QEMU_OPT_STRING, \
> + .help = "Preallocation mode (allowed values: off, " \
> + "metadata, falloc, full)" \
> + }, \
> + { \
> + .name = BLOCK_OPT_LAZY_REFCOUNTS, \
> + .type = QEMU_OPT_BOOL, \
> + .help = "Postpone refcount updates", \
> + .def_value_str = "off" \
> + }, \
> + { \
> + .name = BLOCK_OPT_REFCOUNT_BITS, \
> + .type = QEMU_OPT_NUMBER, \
> + .help = "Width of a reference count entry in bits", \
> + .def_value_str = "16" \
> + } \
> +
> static QemuOptsList qcow2_create_opts = {
> .name = "qcow2-create-opts",
> .head = QTAILQ_HEAD_INITIALIZER(qcow2_create_opts.head),
> .desc = {
> - {
> - .name = BLOCK_OPT_SIZE,
> - .type = QEMU_OPT_SIZE,
> - .help = "Virtual disk size"
> - },
> - {
> - .name = BLOCK_OPT_COMPAT_LEVEL,
> - .type = QEMU_OPT_STRING,
> - .help = "Compatibility level (v2 [0.10] or v3 [1.1])"
> - },
> - {
> - .name = BLOCK_OPT_BACKING_FILE,
> - .type = QEMU_OPT_STRING,
> - .help = "File name of a base image"
> - },
> - {
> - .name = BLOCK_OPT_BACKING_FMT,
> - .type = QEMU_OPT_STRING,
> - .help = "Image format of the base image"
> - },
> - {
> - .name = BLOCK_OPT_DATA_FILE,
> - .type = QEMU_OPT_STRING,
> - .help = "File name of an external data file"
> - },
> - {
> - .name = BLOCK_OPT_DATA_FILE_RAW,
> - .type = QEMU_OPT_BOOL,
> - .help = "The external data file must stay valid as a raw image"
> - },
> - {
> - .name = BLOCK_OPT_ENCRYPT,
> - .type = QEMU_OPT_BOOL,
> - .help = "Encrypt the image with format 'aes'. (Deprecated "
> - "in favor of " BLOCK_OPT_ENCRYPT_FORMAT "=aes)",
> - },
> - {
> - .name = BLOCK_OPT_ENCRYPT_FORMAT,
> - .type = QEMU_OPT_STRING,
> - .help = "Encrypt the image, format choices: 'aes', 'luks'",
> - },
> - BLOCK_CRYPTO_OPT_DEF_KEY_SECRET("encrypt.",
> - "ID of secret providing qcow AES key or LUKS passphrase"),
> - BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_ALG("encrypt."),
> - BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_MODE("encrypt."),
> - BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_ALG("encrypt."),
> - BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG("encrypt."),
> - BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG("encrypt."),
> - BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME("encrypt."),
> - {
> - .name = BLOCK_OPT_CLUSTER_SIZE,
> - .type = QEMU_OPT_SIZE,
> - .help = "qcow2 cluster size",
> - .def_value_str = stringify(DEFAULT_CLUSTER_SIZE)
> - },
> - {
> - .name = BLOCK_OPT_PREALLOC,
> - .type = QEMU_OPT_STRING,
> - .help = "Preallocation mode (allowed values: off, metadata, "
> - "falloc, full)"
> - },
> - {
> - .name = BLOCK_OPT_LAZY_REFCOUNTS,
> - .type = QEMU_OPT_BOOL,
> - .help = "Postpone refcount updates",
> - .def_value_str = "off"
> - },
> - {
> - .name = BLOCK_OPT_REFCOUNT_BITS,
> - .type = QEMU_OPT_NUMBER,
> - .help = "Width of a reference count entry in bits",
> - .def_value_str = "16"
> - },
> + QCOW_COMMON_OPTIONS,
> + { /* end of list */ }
> + }
> +};
> +
> +static QemuOptsList qcow2_amend_opts = {
> + .name = "qcow2-amend-opts",
> + .head = QTAILQ_HEAD_INITIALIZER(qcow2_amend_opts.head),
> + .desc = {
> + QCOW_COMMON_OPTIONS,
> { /* end of list */ }
> }
> };
> @@ -5581,6 +5594,7 @@ BlockDriver bdrv_qcow2 = {
> .bdrv_inactivate = qcow2_inactivate,
>
> .create_opts = &qcow2_create_opts,
> + .amend_opts = &qcow2_amend_opts,
> .strong_runtime_opts = qcow2_strong_runtime_opts,
> .mutable_opts = mutable_opts,
> .bdrv_co_check = qcow2_co_check,
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 810a9ecb86..6f0abf8544 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -407,6 +407,10 @@ struct BlockDriver {
>
> /* List of options for creating images, terminated by name == NULL */
> QemuOptsList *create_opts;
> +
> + /* List of options for image amend*/
> + QemuOptsList *amend_opts;
> +
> /*
> * If this driver supports reopening images this contains a
> * NULL-terminated list of the runtime options that can be
> diff --git a/qemu-img.c b/qemu-img.c
> index a79f3904db..befd53943b 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -3878,11 +3878,11 @@ static int print_amend_option_help(const char *format)
> return 1;
> }
>
> - /* Every driver supporting amendment must have create_opts */
> - assert(drv->create_opts);
> + /* Every driver supporting amendment must have amend_opts */
> + assert(drv->amend_opts);
>
> printf("Creation options for '%s':\n", format);
> - qemu_opts_print_help(drv->create_opts, false);
> + qemu_opts_print_help(drv->amend_opts, false);
> printf("\nNote that not all of these options may be amendable.\n");
> return 0;
> }
> @@ -3892,7 +3892,7 @@ static int img_amend(int argc, char **argv)
> Error *err = NULL;
> int c, ret = 0;
> char *options = NULL;
> - QemuOptsList *create_opts = NULL;
> + QemuOptsList *amend_opts = NULL;
> QemuOpts *opts = NULL;
> const char *fmt = NULL, *filename, *cache;
> int flags;
> @@ -4031,11 +4031,11 @@ static int img_amend(int argc, char **argv)
> goto out;
> }
>
> - /* Every driver supporting amendment must have create_opts */
> - assert(bs->drv->create_opts);
> + /* Every driver supporting amendment must have amend_opts */
> + assert(bs->drv->amend_opts);
>
> - create_opts = qemu_opts_append(create_opts, bs->drv->create_opts);
> - opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
> + amend_opts = qemu_opts_append(amend_opts, bs->drv->amend_opts);
> + opts = qemu_opts_create(amend_opts, NULL, 0, &error_abort);
> qemu_opts_do_parse(opts, options, NULL, &err);
> if (err) {
> error_report_err(err);
> @@ -4058,7 +4058,7 @@ out:
> out_no_progress:
> blk_unref(blk);
> qemu_opts_del(opts);
> - qemu_opts_free(create_opts);
> + qemu_opts_free(amend_opts);
> g_free(options);
>
> if (ret) {
> --
> 2.17.2
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
