Re: [PATCH v6 05/14] block/amend: refactor qcow2 amend options

2020-05-17 Thread Maxim Levitsky
On Thu, 2020-05-14 at 15:36 +0200, Max Reitz wrote:
> On 10.05.20 15:40, Maxim Levitsky wrote:
> > Some qcow2 create options can't be used for amend.
> > Remove them from the qcow2 create options and add generic logic to detect
> > such options in qemu-img
> > 
> > Signed-off-by: Maxim Levitsky 
> > Reviewed-by: Daniel P. Berrangé 
> > ---
> >  block/qcow2.c  | 108 ++---
> >  qemu-img.c |  18 +++-
> >  tests/qemu-iotests/049.out | 102 ++--
> >  tests/qemu-iotests/061.out |  12 ++-
> >  tests/qemu-iotests/079.out |  18 ++--
> >  tests/qemu-iotests/082.out | 149 
> >  tests/qemu-iotests/085.out |  38 
> >  tests/qemu-iotests/087.out |   6 +-
> >  tests/qemu-iotests/115.out |   2 +-
> >  tests/qemu-iotests/121.out |   4 +-
> >  tests/qemu-iotests/125.out | 192 ++---
> >  tests/qemu-iotests/134.out |   2 +-
> >  tests/qemu-iotests/144.out |   4 +-
> >  tests/qemu-iotests/158.out |   4 +-
> >  tests/qemu-iotests/182.out |   2 +-
> >  tests/qemu-iotests/185.out |   8 +-
> >  tests/qemu-iotests/188.out |   2 +-
> >  tests/qemu-iotests/189.out |   4 +-
> >  tests/qemu-iotests/198.out |   4 +-
> >  tests/qemu-iotests/243.out |  16 ++--
> >  tests/qemu-iotests/250.out |   2 +-
> >  tests/qemu-iotests/255.out |   8 +-
> >  tests/qemu-iotests/259.out |   2 +-
> >  tests/qemu-iotests/263.out |   4 +-
> >  tests/qemu-iotests/280.out |   2 +-
> 
> These reference output hunks need some rebasing due to the new
> compression_type option.
Done. I so hope to get it merged so I won't need to rebase it again :-)

> 
> >  25 files changed, 284 insertions(+), 429 deletions(-)
> > 
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index fc494c7591..db86500839 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> 
> [...]
> 
> > @@ -5552,37 +5506,6 @@ void qcow2_signal_corruption(BlockDriverState *bs, 
> > bool fatal, int64_t offset,
> >  .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,  \
> > @@ -5600,6 +5523,37 @@ static QemuOptsList qcow2_create_opts = {
> >  .name = "qcow2-create-opts",
> >  .head = QTAILQ_HEAD_INITIALIZER(qcow2_create_opts.head),
> >  .desc = {
> > +{ 

Re: [PATCH v6 05/14] block/amend: refactor qcow2 amend options

2020-05-14 Thread Max Reitz
On 10.05.20 15:40, Maxim Levitsky wrote:
> Some qcow2 create options can't be used for amend.
> Remove them from the qcow2 create options and add generic logic to detect
> such options in qemu-img
> 
> Signed-off-by: Maxim Levitsky 
> Reviewed-by: Daniel P. Berrangé 
> ---
>  block/qcow2.c  | 108 ++---
>  qemu-img.c |  18 +++-
>  tests/qemu-iotests/049.out | 102 ++--
>  tests/qemu-iotests/061.out |  12 ++-
>  tests/qemu-iotests/079.out |  18 ++--
>  tests/qemu-iotests/082.out | 149 
>  tests/qemu-iotests/085.out |  38 
>  tests/qemu-iotests/087.out |   6 +-
>  tests/qemu-iotests/115.out |   2 +-
>  tests/qemu-iotests/121.out |   4 +-
>  tests/qemu-iotests/125.out | 192 ++---
>  tests/qemu-iotests/134.out |   2 +-
>  tests/qemu-iotests/144.out |   4 +-
>  tests/qemu-iotests/158.out |   4 +-
>  tests/qemu-iotests/182.out |   2 +-
>  tests/qemu-iotests/185.out |   8 +-
>  tests/qemu-iotests/188.out |   2 +-
>  tests/qemu-iotests/189.out |   4 +-
>  tests/qemu-iotests/198.out |   4 +-
>  tests/qemu-iotests/243.out |  16 ++--
>  tests/qemu-iotests/250.out |   2 +-
>  tests/qemu-iotests/255.out |   8 +-
>  tests/qemu-iotests/259.out |   2 +-
>  tests/qemu-iotests/263.out |   4 +-
>  tests/qemu-iotests/280.out |   2 +-

These reference output hunks need some rebasing due to the new
compression_type option.

>  25 files changed, 284 insertions(+), 429 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index fc494c7591..db86500839 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c

[...]

> @@ -5552,37 +5506,6 @@ void qcow2_signal_corruption(BlockDriverState *bs, 
> bool fatal, int64_t offset,
>  .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,  \
> @@ -5600,6 +5523,37 @@ static QemuOptsList qcow2_create_opts = {
>  .name = "qcow2-create-opts",
>  .head = QTAILQ_HEAD_INITIALIZER(qcow2_create_opts.head),
>  .desc = {
> +{   \
> +.name = BLOCK_OPT_ENCRYPT,  \
> +.type = QEMU_OPT_BOOL,  \
> +.help = "Encrypt the image with format 'aes'. (Deprecated " \
> +"in favor of " 

[PATCH v6 05/14] block/amend: refactor qcow2 amend options

2020-05-10 Thread Maxim Levitsky
Some qcow2 create options can't be used for amend.
Remove them from the qcow2 create options and add generic logic to detect
such options in qemu-img

Signed-off-by: Maxim Levitsky 
Reviewed-by: Daniel P. Berrangé 
---
 block/qcow2.c  | 108 ++---
 qemu-img.c |  18 +++-
 tests/qemu-iotests/049.out | 102 ++--
 tests/qemu-iotests/061.out |  12 ++-
 tests/qemu-iotests/079.out |  18 ++--
 tests/qemu-iotests/082.out | 149 
 tests/qemu-iotests/085.out |  38 
 tests/qemu-iotests/087.out |   6 +-
 tests/qemu-iotests/115.out |   2 +-
 tests/qemu-iotests/121.out |   4 +-
 tests/qemu-iotests/125.out | 192 ++---
 tests/qemu-iotests/134.out |   2 +-
 tests/qemu-iotests/144.out |   4 +-
 tests/qemu-iotests/158.out |   4 +-
 tests/qemu-iotests/182.out |   2 +-
 tests/qemu-iotests/185.out |   8 +-
 tests/qemu-iotests/188.out |   2 +-
 tests/qemu-iotests/189.out |   4 +-
 tests/qemu-iotests/198.out |   4 +-
 tests/qemu-iotests/243.out |  16 ++--
 tests/qemu-iotests/250.out |   2 +-
 tests/qemu-iotests/255.out |   8 +-
 tests/qemu-iotests/259.out |   2 +-
 tests/qemu-iotests/263.out |   4 +-
 tests/qemu-iotests/280.out |   2 +-
 25 files changed, 284 insertions(+), 429 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index fc494c7591..db86500839 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2971,17 +2971,6 @@ static int qcow2_change_backing_file(BlockDriverState 
*bs,
 return qcow2_update_header(bs);
 }
 
-static int qcow2_crypt_method_from_format(const char *encryptfmt)
-{
-if (g_str_equal(encryptfmt, "luks")) {
-return QCOW_CRYPT_LUKS;
-} else if (g_str_equal(encryptfmt, "aes")) {
-return QCOW_CRYPT_AES;
-} else {
-return -EINVAL;
-}
-}
-
 static int qcow2_set_up_encryption(BlockDriverState *bs,
QCryptoBlockCreateOptions *cryptoopts,
Error **errp)
@@ -5237,9 +5226,6 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 bool lazy_refcounts = s->use_lazy_refcounts;
 bool data_file_raw = data_file_is_raw(bs);
 const char *compat = NULL;
-uint64_t cluster_size = s->cluster_size;
-bool encrypt;
-int encformat;
 int refcount_bits = s->refcount_bits;
 int ret;
 QemuOptDesc *desc = opts->list->desc;
@@ -5264,44 +5250,12 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 error_setg(errp, "Unknown compatibility level %s", compat);
 return -EINVAL;
 }
-} else if (!strcmp(desc->name, BLOCK_OPT_PREALLOC)) {
-error_setg(errp, "Cannot change preallocation mode");
-return -ENOTSUP;
 } else if (!strcmp(desc->name, BLOCK_OPT_SIZE)) {
 new_size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
 } else if (!strcmp(desc->name, BLOCK_OPT_BACKING_FILE)) {
 backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE);
 } else if (!strcmp(desc->name, BLOCK_OPT_BACKING_FMT)) {
 backing_format = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);
-} else if (!strcmp(desc->name, BLOCK_OPT_ENCRYPT)) {
-encrypt = qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT,
-!!s->crypto);
-
-if (encrypt != !!s->crypto) {
-error_setg(errp,
-   "Changing the encryption flag is not supported");
-return -ENOTSUP;
-}
-} else if (!strcmp(desc->name, BLOCK_OPT_ENCRYPT_FORMAT)) {
-encformat = qcow2_crypt_method_from_format(
-qemu_opt_get(opts, BLOCK_OPT_ENCRYPT_FORMAT));
-
-if (encformat != s->crypt_method_header) {
-error_setg(errp,
-   "Changing the encryption format is not supported");
-return -ENOTSUP;
-}
-} else if (g_str_has_prefix(desc->name, "encrypt.")) {
-error_setg(errp,
-   "Changing the encryption parameters is not supported");
-return -ENOTSUP;
-} else if (!strcmp(desc->name, BLOCK_OPT_CLUSTER_SIZE)) {
-cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE,
- cluster_size);
-if (cluster_size != s->cluster_size) {
-error_setg(errp, "Changing the cluster size is not supported");
-return -ENOTSUP;
-}
 } else if (!strcmp(desc->name, BLOCK_OPT_LAZY_REFCOUNTS)) {
 lazy_refcounts = qemu_opt_get_bool(opts, BLOCK_OPT_LAZY_REFCOUNTS,
lazy_refcounts);
@@ -5552,37 +5506,6 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool 
fatal, int64_t offset,
 .help = "The external data file must stay valid "   \