On Tue, Apr 28, 2020 at 04:03:33PM +0100, Daniel P. Berrangé wrote: > On Sun, Mar 08, 2020 at 05:18:53PM +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. > > In the v1 posting I had suggested changing this patch, so that it > only adds things to the amend list that actually can be amended. > > https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg07570.html
Never mind, I should have read ahead in the series to see the next patch So for this patch Reviewed-by: Daniel P. Berrangé <[email protected]> > > > > > Signed-off-by: Maxim Levitsky <[email protected]> > > --- > > 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 b55e5b7c1f..9574085772 100644 > > --- a/block/qcow2.c > > +++ b/block/qcow2.c > > @@ -5440,83 +5440,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 */ } > > } > > }; > > @@ -5576,6 +5589,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 24d00fbf48..48a4c2fa1c 100644 > > --- a/include/block/block_int.h > > +++ b/include/block/block_int.h > > @@ -406,6 +406,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 551388676f..2555aedee9 100644 > > --- a/qemu-img.c > > +++ b/qemu-img.c > > @@ -3898,11 +3898,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; > > } > > @@ -3912,7 +3912,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; > > @@ -4051,11 +4051,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); > > @@ -4078,7 +4078,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 :| > > 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 :|
