Re: [Qemu-devel] [PATCH v8 07/20] block: deprecate "encryption=on" in favor of "encrypt.format=aes"
On Wed, Jun 07, 2017 at 06:40:32PM +0200, Max Reitz wrote: > On 2017-06-01 19:27, Daniel P. Berrange wrote: > > Historically the qcow & qcow2 image formats supported a property > > "encryption=on" to enable their built-in AES encryption. We'll > > soon be supporting LUKS for qcow2, so need a more general purpose > > way to enable encryption, with a choice of formats. > > > > This introduces an "encrypt.format" option, which will later be > > joined by a number of other "encrypt.XXX" options. The use of > > a "encrypt." prefix instead of "encrypt-" is done to facilitate > > mapping to a nested QAPI schema at later date. > > > > e.g. the preferred syntax is now > > > > qemu-img create -f qcow2 -o encrypt.format=aes demo.qcow2 > > > > Reviewed-by: Eric Blake> > Reviewed-by: Alberto Garcia > > Signed-off-by: Daniel P. Berrange > > --- > > block/qcow.c | 30 ++--- > > block/qcow2.c | 33 +++ > > include/block/block_int.h | 2 +- > > qemu-img.c | 4 ++- > > tests/qemu-iotests/082.out | 81 > > ++ > > 5 files changed, 110 insertions(+), 40 deletions(-) > > > > diff --git a/block/qcow.c b/block/qcow.c > > index 6738bc7..42f83b2 100644 > > --- a/block/qcow.c > > +++ b/block/qcow.c > > [...] > > > @@ -818,8 +818,16 @@ static int qcow_create(const char *filename, QemuOpts > > *opts, Error **errp) > > } > > > > backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); > > -if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) { > > -flags |= BLOCK_FLAG_ENCRYPT; > > +encryptfmt = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT_FORMAT); > > +if (encryptfmt) { > > +if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) { > > You should probably just use qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT) > here, because otherwise you can do this: > > $ ./qemu-img create -f qcow -o encryption=off,encrypt.format=aes \ > foo.qcow 64M > Formatting 'foo.qcow', fmt=qcow size=67108864 encryption=off > encrypt.format=aes Yes, will fix it as you suggest. 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 :|
Re: [Qemu-devel] [PATCH v8 07/20] block: deprecate "encryption=on" in favor of "encrypt.format=aes"
On 2017-06-01 19:27, Daniel P. Berrange wrote: > Historically the qcow & qcow2 image formats supported a property > "encryption=on" to enable their built-in AES encryption. We'll > soon be supporting LUKS for qcow2, so need a more general purpose > way to enable encryption, with a choice of formats. > > This introduces an "encrypt.format" option, which will later be > joined by a number of other "encrypt.XXX" options. The use of > a "encrypt." prefix instead of "encrypt-" is done to facilitate > mapping to a nested QAPI schema at later date. > > e.g. the preferred syntax is now > > qemu-img create -f qcow2 -o encrypt.format=aes demo.qcow2 > > Reviewed-by: Eric Blake> Reviewed-by: Alberto Garcia > Signed-off-by: Daniel P. Berrange > --- > block/qcow.c | 30 ++--- > block/qcow2.c | 33 +++ > include/block/block_int.h | 2 +- > qemu-img.c | 4 ++- > tests/qemu-iotests/082.out | 81 > ++ > 5 files changed, 110 insertions(+), 40 deletions(-) > > diff --git a/block/qcow.c b/block/qcow.c > index 6738bc7..42f83b2 100644 > --- a/block/qcow.c > +++ b/block/qcow.c [...] > @@ -818,8 +818,16 @@ static int qcow_create(const char *filename, QemuOpts > *opts, Error **errp) > } > > backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); > -if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) { > -flags |= BLOCK_FLAG_ENCRYPT; > +encryptfmt = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT_FORMAT); > +if (encryptfmt) { > +if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) { You should probably just use qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT) here, because otherwise you can do this: $ ./qemu-img create -f qcow -o encryption=off,encrypt.format=aes \ foo.qcow 64M Formatting 'foo.qcow', fmt=qcow size=67108864 encryption=off encrypt.format=aes > +error_setg(errp, "Options " BLOCK_OPT_ENCRYPT " and " > + BLOCK_OPT_ENCRYPT_FORMAT " are mutually exclusive"); > +ret = -EINVAL; > +goto cleanup; > +} > +} else if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) { > +encryptfmt = "aes"; > } > > ret = bdrv_create_file(filename, opts, _err); [...] > diff --git a/block/qcow2.c b/block/qcow2.c > index b3ba5da..19dfcd1 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c [...] > @@ -2366,8 +2373,16 @@ static int qcow2_create(const char *filename, QemuOpts > *opts, Error **errp) > BDRV_SECTOR_SIZE); > backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); > backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT); > -if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) { > -flags |= BLOCK_FLAG_ENCRYPT; > +encryptfmt = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT_FORMAT); > +if (encryptfmt) { > +if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) { Same here. With these places fixed: Reviewed-by: Max Reitz > +error_setg(errp, "Options " BLOCK_OPT_ENCRYPT " and " > + BLOCK_OPT_ENCRYPT_FORMAT " are mutually exclusive"); > +ret = -EINVAL; > +goto finish; > +} > +} else if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) { > +encryptfmt = "aes"; > } > cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE, > DEFAULT_CLUSTER_SIZE); signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH v8 07/20] block: deprecate "encryption=on" in favor of "encrypt.format=aes"
Historically the qcow & qcow2 image formats supported a property "encryption=on" to enable their built-in AES encryption. We'll soon be supporting LUKS for qcow2, so need a more general purpose way to enable encryption, with a choice of formats. This introduces an "encrypt.format" option, which will later be joined by a number of other "encrypt.XXX" options. The use of a "encrypt." prefix instead of "encrypt-" is done to facilitate mapping to a nested QAPI schema at later date. e.g. the preferred syntax is now qemu-img create -f qcow2 -o encrypt.format=aes demo.qcow2 Reviewed-by: Eric BlakeReviewed-by: Alberto Garcia Signed-off-by: Daniel P. Berrange --- block/qcow.c | 30 ++--- block/qcow2.c | 33 +++ include/block/block_int.h | 2 +- qemu-img.c | 4 ++- tests/qemu-iotests/082.out | 81 ++ 5 files changed, 110 insertions(+), 40 deletions(-) diff --git a/block/qcow.c b/block/qcow.c index 6738bc7..42f83b2 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -803,10 +803,10 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp) uint8_t *tmp; int64_t total_size = 0; char *backing_file = NULL; -int flags = 0; Error *local_err = NULL; int ret; BlockBackend *qcow_blk; +const char *encryptfmt = NULL; /* Read out options */ total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0), @@ -818,8 +818,16 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp) } backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); -if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) { -flags |= BLOCK_FLAG_ENCRYPT; +encryptfmt = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT_FORMAT); +if (encryptfmt) { +if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) { +error_setg(errp, "Options " BLOCK_OPT_ENCRYPT " and " + BLOCK_OPT_ENCRYPT_FORMAT " are mutually exclusive"); +ret = -EINVAL; +goto cleanup; +} +} else if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) { +encryptfmt = "aes"; } ret = bdrv_create_file(filename, opts, _err); @@ -872,7 +880,13 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp) l1_size = (total_size + (1LL << shift) - 1) >> shift; header.l1_table_offset = cpu_to_be64(header_size); -if (flags & BLOCK_FLAG_ENCRYPT) { +if (encryptfmt) { +if (!g_str_equal(encryptfmt, "aes")) { +error_setg(errp, "Unknown encryption format '%s', expected 'aes'", + encryptfmt); +ret = -EINVAL; +goto exit; +} header.crypt_method = cpu_to_be32(QCOW_CRYPT_AES); } else { header.crypt_method = cpu_to_be32(QCOW_CRYPT_NONE); @@ -1046,9 +1060,15 @@ static QemuOptsList qcow_create_opts = { { .name = BLOCK_OPT_ENCRYPT, .type = QEMU_OPT_BOOL, -.help = "Encrypt the image", +.help = "Encrypt the image with format 'aes'. (Deprecated " +"in favor of " BLOCK_OPT_ENCRYPT_FORMAT "=aes)", .def_value_str = "off" }, +{ +.name = BLOCK_OPT_ENCRYPT_FORMAT, +.type = QEMU_OPT_STRING, +.help = "Encrypt the image, format choices: 'aes'", +}, { /* end of list */ } } }; diff --git a/block/qcow2.c b/block/qcow2.c index b3ba5da..19dfcd1 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2100,7 +2100,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, const char *backing_file, const char *backing_format, int flags, size_t cluster_size, PreallocMode prealloc, QemuOpts *opts, int version, int refcount_order, - Error **errp) + const char *encryptfmt, Error **errp) { int cluster_bits; QDict *options; @@ -2229,7 +2229,13 @@ static int qcow2_create2(const char *filename, int64_t total_size, .header_length = cpu_to_be32(sizeof(*header)), }; -if (flags & BLOCK_FLAG_ENCRYPT) { +if (encryptfmt) { +if (!g_str_equal(encryptfmt, "aes")) { +error_setg(errp, "Unknown encryption format '%s', expected 'aes'", + encryptfmt); +ret = -EINVAL; +goto out; +} header->crypt_method = cpu_to_be32(QCOW_CRYPT_AES); } else { header->crypt_method = cpu_to_be32(QCOW_CRYPT_NONE); @@ -2358,6 +2364,7 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp) int version = 3; uint64_t refcount_bits = 16; int refcount_order; +const char