| Message ID | 20200510134037.18487-5-mlevitsk@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | LUKS: encryption slot management using amend interface | expand |
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 <mlevitsk@redhat.com> > Reviewed-by: Daniel P. Berrangé <berrange@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 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 <mreitz@redhat.com> > + QemuOptsList *amend_opts; > +
On 5/14/20 7:28 AM, Max Reitz wrote: > 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 >> >> >> +#define QCOW_COMMON_OPTIONS \ >> + { \ >> + .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). Except that... > > 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 */ } ...the intended usage is to use the macro name followed by a comma, so including a trailing comma in the macro itself would lead to a syntax error. I think the better fix is to eliminate the trailing \ on the final line, and have '}' without a trailing comma in the macro.
On 14.05.20 18:10, Eric Blake wrote: > On 5/14/20 7:28 AM, Max Reitz wrote: >> 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 >>> > >>> +#define QCOW_COMMON_OPTIONS \ >>> + { \ > >>> + .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). > > Except that... > >> >> 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 */ } > > ...the intended usage is to use the macro name followed by a comma, so > including a trailing comma in the macro itself would lead to a syntax > error. But why is that the indended usage? Is there something in our coding style that forbids macros that don’t allow a separator to be placed after them? Max
On 5/15/20 1:22 AM, Max Reitz wrote: >>> >>>> + QCOW_COMMON_OPTIONS, >>>> + { /* end of list */ } >> >> ...the intended usage is to use the macro name followed by a comma, so >> including a trailing comma in the macro itself would lead to a syntax >> error. > > But why is that the indended usage? Is there something in our coding > style that forbids macros that don’t allow a separator to be placed > after them? If we have more than one such macro, it is easier to write and indent (especially when using your editor's ability to decipher enough syntax to suggest how to indent): myarray = { COMMON_ELEMENTS, MORE_ELEMENTS, { /* end of list */ } }; than it is: myarray = { COMMON_ELEMENTS MORE_ELEMENTS { /* end of list */ } }; which in turn implies that it is better to NOT stick a trailing comma in the macro itself. Similarly, for macros intended to replace statements, we tend to avoid the trailing ; in the macro itself, because it is easier to read: { code; MACRO(); more code; } than it is: { code; MACRO() more code; }
On Fri, 2020-05-15 at 12:24 -0500, Eric Blake wrote: > On 5/15/20 1:22 AM, Max Reitz wrote: > > > > > > > > > > + QCOW_COMMON_OPTIONS, > > > > > + { /* end of list */ } > > > > > > ...the intended usage is to use the macro name followed by a comma, so > > > including a trailing comma in the macro itself would lead to a syntax > > > error. > > > > But why is that the indended usage? Is there something in our coding > > style that forbids macros that don’t allow a separator to be placed > > after them? > > If we have more than one such macro, it is easier to write and indent > (especially when using your editor's ability to decipher enough syntax > to suggest how to indent): > > myarray = { > COMMON_ELEMENTS, > MORE_ELEMENTS, > { /* end of list */ } > }; > > than it is: > > myarray = { > COMMON_ELEMENTS > MORE_ELEMENTS > { /* end of list */ } > }; > > which in turn implies that it is better to NOT stick a trailing comma in > the macro itself. Similarly, for macros intended to replace statements, > we tend to avoid the trailing ; in the macro itself, because it is > easier to read: > > { > code; > MACRO(); > more code; > } > > than it is: > > { > code; > MACRO() > more code; > } > 100% agree with that. Here something a bit off-topic, but something that I find a bit amusing and is somewhat related to hiding punctuation in macros: I once wasted about hour of my life trying to understand why kernel ABI macro I added for a backport didn't work as intended. (This was a macro we are supposed to wrap each new struct field in it to inform to the ABI checker that it is OK). I was almost ready to poke my eyes out as I were comparing what I wrote to what is present in few more places that use that macro, till I finally understood that the macro expects you to not stick ';' after it. It does compile fine with an extra ';', since it is just an empty statement, but this was tripping some regular expression in the ABI checker script or something. (It didn't give me any meaningful error). Back to topic, I'll rebase this patch, as I always do prior to sending a new patch series. Best regards, Maxim Levitsky
On Thu, 2020-05-14 at 14:28 +0200, Max Reitz wrote: > 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 <mlevitsk@redhat.com> > > Reviewed-by: Daniel P. Berrangé <berrange@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 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). As discussed with Eric Blake, lets leave it as is. > > 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). Done! > > > + > > 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. Absolutely! Thanks! > > With those things fixed: > > Reviewed-by: Max Reitz <mreitz@redhat.com> Thanks! > > > + QemuOptsList *amend_opts; > > + > > Best regards, Maxim Levitsky
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" \ + } \ + 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 */ } } }; @@ -5656,6 +5669,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 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*/ + 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 ef422d5471..8f69366f03 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -3936,11 +3936,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; } @@ -3950,7 +3950,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; @@ -4081,11 +4081,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); @@ -4108,7 +4108,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) {