[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 04/14] block/amend: separate amend and create options for
|
From: |
Daniel P . Berrangé |
|
Subject: |
Re: [PATCH v2 04/14] block/amend: separate amend and create options for qemu-img |
|
Date: |
Tue, 28 Apr 2020 16:49:07 +0100 |
|
User-agent: |
Mutt/1.13.3 (2020-01-12) |
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é <address@hidden>
>
> >
> > Signed-off-by: Maxim Levitsky <address@hidden>
> > ---
> > 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 :|