On 10.05.20 15:40, 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. > > Signed-off-by: Maxim Levitsky <[email protected]> > Reviewed-by: Daniel P. Berrangé <[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 79fbad9d76..fc494c7591 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -5520,83 +5520,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" \ > + } \
I think the last line should have a comma in it (otherwise the final
backslash doesn’t make much sense, because whenever we’d add a new
option, we would need to modify the line anyway to insert a comma).
Speaking of adding option, this requires a rebase due to the
compression_type option added (not trivial in the strict sense, but
still straightforward to handle).
> +
> static QemuOptsList qcow2_create_opts = {
> .name = "qcow2-create-opts",
> .head = QTAILQ_HEAD_INITIALIZER(qcow2_create_opts.head),
> .desc = {
>
[...]
> + 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 */ }
If QCOW_COMMON_OPTIONS were to already end in a comma (which I think it
should), then it would become superfluous here.
> }
> };
[...]
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 952b2f033a..0a71357b50 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -412,6 +412,10 @@ struct BlockDriver {
>
> /* List of options for creating images, terminated by name == NULL */
> QemuOptsList *create_opts;
> +
> + /* List of options for image amend*/
I don’t suppose we have a coding style requirement for this, but I still
think there should be a space before the closing asterisk.
With those things fixed:
Reviewed-by: Max Reitz <[email protected]>
> + QemuOptsList *amend_opts;
> +
signature.asc
Description: OpenPGP digital signature
