[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 04/14] block/amend: separate amend and create options for
|
From: |
Max Reitz |
|
Subject: |
Re: [PATCH v6 04/14] block/amend: separate amend and create options for qemu-img |
|
Date: |
Thu, 14 May 2020 14:28:41 +0200 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 |
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 <address@hidden>
> Reviewed-by: Daniel P. Berrangé <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 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 <address@hidden>
> + QemuOptsList *amend_opts;
> +
signature.asc
Description: OpenPGP digital signature
[PATCH v6 05/14] block/amend: refactor qcow2 amend options, Maxim Levitsky, 2020/05/10
[PATCH v6 08/14] block/qcow2: extend qemu-img amend interface with crypto options, Maxim Levitsky, 2020/05/10