luks: Implement .bdrv_co_create
cover luks: Implement .bdrv_co_create
This series implements the .bdrv_co_create callback for luks, adds an image creation test for it and contains some bonus fixes for bugs that the test triggered.
Kevin Wolf (6):
luks: Separate image file creation from formatting
luks: Create block_crypto_co_create_generic()
luks: Support .bdrv_co_create
luks: Turn invalid assertion into check
luks: Catch integer overflow for huge sizes
qemu-iotests: Test luks QMP image creation
qapi/block-core.json | 17 +++- block/crypto.c | 150 +++++++++++++++++++++--------- tests/qemu-iotests/208 | 211 +++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/208.out | 136 ++++++++++++++++++++++++++++ tests/qemu-iotests/common.rc | 2 +- tests/qemu-iotests/group | 1 + 6 files changed, 473 insertions(+), 44 deletions(-) create mode 100755 tests/qemu-iotests/208 create mode 100644 tests/qemu-iotests/208.out
Signature · 1 line
2.13.6
PATCH 1/6 luks: Separate image file creation from formatting
The crypto driver used to create the image file in a callback from the crypto subsystem. If we want to implement .bdrv_co_create, this needs to go away because that callback will get a reference to an already existing block node.
Move the image file creation to block_crypto_create_generic().
--- block/crypto.c | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-)
block/crypto.c +17 -20
@@ -71,8 +71,6 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,struct BlockCryptoCreateData { -const char *filename; -QemuOpts *opts;BlockBackend *blk;uint64_t size; };
@@ -103,27 +101,13 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block, Error **errp) {struct BlockCryptoCreateData *data = opaque; -int ret;/* User provided size should reflect amount of space made * available to the guest, so we must take account of that * which will be used by the crypto header */ - data->size += headerlen; - -qemu_opt_set_number (data->opts,BLOCK_OPT_SIZE , data->size, &error_abort); - ret =bdrv_create_file (data->filename, data->opts, errp); -if (ret <0 ) { -return -1 ; - } - - data->blk =blk_new_open (data->filename,NULL ,NULL , -BDRV_O_RDWR |BDRV_O_PROTOCOL , errp); -if (!data->blk) { -return -1 ; - } - -return 0 ; +return blk_truncate (data->blk, data->size + headerlen,PREALLOC_MODE_OFF , + errp); }
@@ -333,11 +317,10 @@ static int block_crypto_create_generic(QCryptoBlockFormat format,struct BlockCryptoCreateData data = { .size =ROUND_UP (qemu_opt_get_size_del (opts,BLOCK_OPT_SIZE ,0 ),BDRV_SECTOR_SIZE ), - .opts = opts, - .filename = filename, };QDict *cryptoopts; +/* Parse options */ cryptoopts =qemu_opts_to_qdict (opts,NULL ); create_opts =block_crypto_create_opts_init (format, cryptoopts, errp);
@@ -345,6 +328,20 @@ static int block_crypto_create_generic(QCryptoBlockFormat format,return -1 ; } +/* Create protocol layer */ + ret =bdrv_create_file (filename, opts, errp); +if (ret <0 ) { +return ret; + } + + data.blk =blk_new_open (filename,NULL ,NULL , +BDRV_O_RDWR |BDRV_O_RESIZE |BDRV_O_PROTOCOL , + errp); +if (!data.blk) { +return -EINVAL ; + } + +/* Create format layer */ crypto = qcrypto_block_create(create_opts, NULL, block_crypto_init_func, block_crypto_write_func,
Signature · 1 line
2.13.6
Re: [Qemu-devel] [PATCH 1/6] luks: Separate image file creation from formatting
Quoted 5 lines · Kevin Wolf
The crypto driver used to create the image file in a callback from the crypto subsystem. If we want to implement .bdrv_co_create, this needs to go away because that callback will get a reference to an already existing block node.
Quoted 7 lines
Move the image file creation to
block_crypto_create_generic().Signed-off-by Kevin Wolf <[email protected]>--- block/crypto.c | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-)
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 1/6] luks: Separate image file creation from formatting
In reply to Kevin WolfQuoted 43 lines · Kevin Wolf
The crypto driver used to create the image file in a callback from the crypto subsystem. If we want to implement .bdrv_co_create, this needs to go away because that callback will get a reference to an already existing block node.
Move the image file creation to
block_crypto_create_generic().Signed-off-by Kevin Wolf <[email protected]>--- block/crypto.c | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-)block/crypto.c +0
-11@@ -71,8 +71,6 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,struct BlockCryptoCreateData { -const char *filename; -QemuOpts *opts;BlockBackend *blk;uint64_t size; };@@ -103,27 +101,13 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block, Error **errp) {struct BlockCryptoCreateData *data = opaque; -int ret;/* User provided size should reflect amount of space made * available to the guest, so we must take account of that * which will be used by the crypto header */ - data->size += headerlen; - -qemu_opt_set_number (data->opts,BLOCK_OPT_SIZE , data->size, &error_abort); - ret =bdrv_create_file (data->filename, data->opts, errp); -if (ret <0 ) { -return -1 ; - } -
block/crypto.c static ssize_t block_crypto_init_func(QCryptoBlock *block, · line 111
Quoted 10 lines
block/crypto.c +2
-7- data->blk =blk_new_open (data->filename,NULL ,NULL , -BDRV_O_RDWR |BDRV_O_PROTOCOL , errp); -if (!data->blk) { -return -1 ; - } - -return 0 ; +return blk_truncate (data->blk, data->size + headerlen,PREALLOC_MODE_OFF , + errp); }
I wonder if we should pass in the value of the PREALLOC_MODE from the block_crypto_create_generic() function via BlockCryptoCreateData, so that we honour it. By using PREALLOC_MODE_OFF here, all LUKS volumes created with PREALLOC_MODE_FULL are going to have several MB of unallocated extents at the end of the file.
Quoted 39 lines
block/crypto.c +15
-2@@ -333,11 +317,10 @@ static int block_crypto_create_generic(QCryptoBlockFormat format,struct BlockCryptoCreateData data = { .size =ROUND_UP (qemu_opt_get_size_del (opts,BLOCK_OPT_SIZE ,0 ),BDRV_SECTOR_SIZE ), - .opts = opts, - .filename = filename, };QDict *cryptoopts; +/* Parse options */ cryptoopts =qemu_opts_to_qdict (opts,NULL ); create_opts =block_crypto_create_opts_init (format, cryptoopts, errp);@@ -345,6 +328,20 @@ static int block_crypto_create_generic(QCryptoBlockFormat format,return -1 ; } +/* Create protocol layer */ + ret =bdrv_create_file (filename, opts, errp); +if (ret <0 ) { +return ret; + } + + data.blk =blk_new_open (filename,NULL ,NULL , +BDRV_O_RDWR |BDRV_O_RESIZE |BDRV_O_PROTOCOL , + errp); +if (!data.blk) { +return -EINVAL ; + } + +/* Create format layer */ crypto = qcrypto_block_create(create_opts, NULL, block_crypto_init_func, block_crypto_write_func,Signature · 1 line
2.13.6
Regards,
Daniel
Signature · 3 lines
|: 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 1/6] luks: Separate image file creation from formatting
Am 12.03.2018 um 12:35 hat Daniel P. Berrangé geschrieben:
Quoted 52 lines · Kevin Wolf
Quoted 51 lines
The crypto driver used to create the image file in a callback from the crypto subsystem. If we want to implement .bdrv_co_create, this needs to go away because that callback will get a reference to an already existing block node.
Move the image file creation to
block_crypto_create_generic().Signed-off-by Kevin Wolf <[email protected]>--- block/crypto.c | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-)block/crypto.c +1
-18@@ -71,8 +71,6 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,struct BlockCryptoCreateData { -const char *filename; -QemuOpts *opts;BlockBackend *blk;uint64_t size; };@@ -103,27 +101,13 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block, Error **errp) {struct BlockCryptoCreateData *data = opaque; -int ret;/* User provided size should reflect amount of space made * available to the guest, so we must take account of that * which will be used by the crypto header */ - data->size += headerlen; - -qemu_opt_set_number (data->opts,BLOCK_OPT_SIZE , data->size, &error_abort); - ret =bdrv_create_file (data->filename, data->opts, errp); -if (ret <0 ) { -return -1 ; - } - - data->blk =blk_new_open (data->filename,NULL ,NULL , -BDRV_O_RDWR |BDRV_O_PROTOCOL , errp); -if (!data->blk) { -return -1 ; - } - -return 0 ; + return blk_truncate(data->blk, data->size + headerlen, PREALLOC_MODE_OFF,
Quoted 8 lines
block/crypto.c static ssize_t block_crypto_init_func(QCryptoBlock *block, · line 111
Quoted 2 lines
block/crypto.c +1
-0+ errp); }I wonder if we should pass in the value of the
PREALLOC_MODEfrom theblock_crypto_create_generic()function via BlockCryptoCreateData, so that we honour it. By usingPREALLOC_MODE_OFFhere, all LUKS volumes created withPREALLOC_MODE_FULLare going to have several MB of unallocated extents at the end of the file.
That would be an easy way to implement preallocation support. But you can't create LUKS volumes with PREALLOC_MODE_FULL today because it doesn't support preallocation at all (there's no option for it in &block_crypto_create_opts_luks), so implementing that is matter for a different patch.
Kevin
Daniel P. Berrangé
Reviewed-byRe: [Qemu-devel] [PATCH 1/6] luks: Separate image file creation from formatting
Quoted 62 lines · Kevin Wolf
Am 12.03.2018 um 12:35 hat Daniel P. Berrangé geschrieben:
Quoted 52 lines · Kevin Wolf
Quoted 51 lines
The crypto driver used to create the image file in a callback from the crypto subsystem. If we want to implement .bdrv_co_create, this needs to go away because that callback will get a reference to an already existing block node.
Move the image file creation to
block_crypto_create_generic().Signed-off-by Kevin Wolf <[email protected]>--- block/crypto.c | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-)block/crypto.c +1
-18@@ -71,8 +71,6 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,struct BlockCryptoCreateData { -const char *filename; -QemuOpts *opts;BlockBackend *blk;uint64_t size; };@@ -103,27 +101,13 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block, Error **errp) {struct BlockCryptoCreateData *data = opaque; -int ret;/* User provided size should reflect amount of space made * available to the guest, so we must take account of that * which will be used by the crypto header */ - data->size += headerlen; - -qemu_opt_set_number (data->opts,BLOCK_OPT_SIZE , data->size, &error_abort); - ret =bdrv_create_file (data->filename, data->opts, errp); -if (ret <0 ) { -return -1 ; - } - - data->blk =blk_new_open (data->filename,NULL ,NULL , -BDRV_O_RDWR |BDRV_O_PROTOCOL , errp); -if (!data->blk) { -return -1 ; - } - -return 0 ; + return blk_truncate(data->blk, data->size + headerlen, PREALLOC_MODE_OFF,Quoted 8 lines
block/crypto.c static ssize_t block_crypto_init_func(QCryptoBlock *block, · line 111
Quoted 2 lines
block/crypto.c +1
-0+ errp); }I wonder if we should pass in the value of the
PREALLOC_MODEfrom theblock_crypto_create_generic()function via BlockCryptoCreateData, so that we honour it. By usingPREALLOC_MODE_OFFhere, all LUKS volumes created withPREALLOC_MODE_FULLare going to have several MB of unallocated extents at the end of the file.
Quoted 5 lines
That would be an easy way to implement preallocation support. But you can't create LUKS volumes with
PREALLOC_MODE_FULLtoday because it doesn't support preallocation at all (there's no option for it in&block_crypto_create_opts_luks), so implementing that is matter for a different patch.
Hah, ops, I forgot that ! In that case,
Regards,
Daniel
Signature · 3 lines
|: 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 :|
PATCH 2/6 luks: Create block_crypto_co_create_generic()
In reply to Kevin WolfEverything that refers to the protocol layer or QemuOpts is moved out of block_crypto_create_generic(), so that the remaining function is suitable to be called by a .bdrv_co_create implementation.
LUKS is the only driver that actually implements the old interface, and we don't intend to use it in any new drivers, so put the moved out code directly into a LUKS function rather than creating a generic intermediate one.
--- block/crypto.c | 95 +++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 61 insertions(+), 34 deletions(-)
block/crypto.c +61 -34
@@ -306,43 +306,29 @@ static int block_crypto_open_generic(QCryptoBlockFormat format, } -static int block_crypto_create_generic (QCryptoBlockFormat format, -const char *filename, -QemuOpts *opts, -Error **errp) +static int block_crypto_co_create_generic (BlockDriverState *bs, +int64_t size, +QCryptoBlockCreateOptions *opts, +Error **errp) { -int ret = -EINVAL ; -QCryptoBlockCreateOptions *create_opts =NULL ; +int ret; +BlockBackend *blk;QCryptoBlock *crypto =NULL ; -struct BlockCryptoCreateData data = { - .size =ROUND_UP (qemu_opt_get_size_del (opts,BLOCK_OPT_SIZE ,0 ), -BDRV_SECTOR_SIZE ), - }; -QDict *cryptoopts; - -/* Parse options */ - cryptoopts =qemu_opts_to_qdict (opts,NULL ); +struct BlockCryptoCreateData data; - create_opts =block_crypto_create_opts_init (format, cryptoopts, errp); -if (!create_opts) { -return -1 ; - } + blk =blk_new (BLK_PERM_WRITE |BLK_PERM_RESIZE ,BLK_PERM_ALL ); -/* Create protocol layer */ - ret =bdrv_create_file (filename, opts, errp); + ret =blk_insert_bs (blk, bs, errp);if (ret <0 ) { -return ret; + goto cleanup; } - data.blk =blk_new_open (filename,NULL ,NULL , -BDRV_O_RDWR |BDRV_O_RESIZE |BDRV_O_PROTOCOL , - errp); -if (!data.blk) { -return -EINVAL ; - } + data = (struct BlockCryptoCreateData ) { + .blk = blk, + .size = size, + }; -/* Create format layer */ - crypto = qcrypto_block_create(create_opts, NULL, + crypto = qcrypto_block_create(opts, NULL, block_crypto_init_func, block_crypto_write_func, &data,
@@ -355,10 +341,8 @@ static int block_crypto_create_generic(QCryptoBlockFormat format, ret =0 ; cleanup: -QDECREF (cryptoopts);qcrypto_block_free (crypto); -blk_unref (data.blk); -qapi_free_QCryptoBlockCreateOptions (create_opts); +blk_unref (blk);return ret; }
@@ -563,8 +547,51 @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename, QemuOpts *opts, Error **errp) { -return block_crypto_create_generic (Q_CRYPTO_BLOCK_FORMAT_LUKS , - filename, opts, errp); +QCryptoBlockCreateOptions *create_opts =NULL ; +BlockDriverState *bs =NULL ; +QDict *cryptoopts; +int64_t size; +int ret; + +/* Parse options */ + size =qemu_opt_get_size_del (opts,BLOCK_OPT_SIZE ,0 ); + + cryptoopts =qemu_opts_to_qdict_filtered (opts,NULL , + &block_crypto_create_opts_luks, + true); + + create_opts =block_crypto_create_opts_init (Q_CRYPTO_BLOCK_FORMAT_LUKS , + cryptoopts, errp); +if (!create_opts) { + ret = -EINVAL ; + goto fail; + } + +/* Create protocol layer */ + ret =bdrv_create_file (filename, opts, errp); +if (ret <0 ) { +return ret; + } + + bs =bdrv_open (filename,NULL ,NULL , +BDRV_O_RDWR |BDRV_O_RESIZE |BDRV_O_PROTOCOL , errp); +if (!bs) { + ret = -EINVAL ; + goto fail; + } + +/* Create format layer */ + ret =block_crypto_co_create_generic (bs, size, create_opts, errp); +if (ret <0 ) { + goto fail; + } + + ret =0 ; +fail: +bdrv_unref (bs); +qapi_free_QCryptoBlockCreateOptions (create_opts); +QDECREF (cryptoopts); +return ret; } static int block_crypto_get_info_luks(BlockDriverState *bs,
Signature · 1 line
2.13.6
Re: [Qemu-devel] [PATCH 2/6] luks: Create block_crypto_co_create_generic()
Quoted 9 lines · Kevin Wolf
Everything that refers to the protocol layer or QemuOpts is moved out of
block_crypto_create_generic(), so that the remaining function is suitable to be called by a .bdrv_co_create implementation.LUKS is the only driver that actually implements the old interface, and we don't intend to use it in any new drivers, so put the moved out code directly into a LUKS function rather than creating a generic intermediate one.
Quoted 5 lines
Signed-off-by Kevin Wolf <[email protected]>--- block/crypto.c | 95 +++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 61 insertions(+), 34 deletions(-)
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 2/6] luks: Create block_crypto_co_create_generic()
In reply to Kevin WolfQuoted 9 lines · Kevin Wolf
Everything that refers to the protocol layer or QemuOpts is moved out of
block_crypto_create_generic(), so that the remaining function is suitable to be called by a .bdrv_co_create implementation.LUKS is the only driver that actually implements the old interface, and we don't intend to use it in any new drivers, so put the moved out code directly into a LUKS function rather than creating a generic intermediate one.
Quoted 4 lines
Signed-off-by Kevin Wolf <[email protected]>--- block/crypto.c | 95 +++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 61 insertions(+), 34 deletions(-)
Regards,
Daniel
Signature · 3 lines
|: 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 :|
PATCH 3/6 luks: Support .bdrv_co_create
In reply to Kevin WolfThis adds the .bdrv_co_create driver callback to luks, which enables image creation over QMP.
--- qapi/block-core.json | 17 ++++++++++++++++- block/crypto.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-)
qapi/block-core.json +16 -1
@@ -3452,6 +3452,21 @@ '*preallocation': 'PreallocMode' } } ## +# @BlockdevCreateOptionsLUKS: +# +# Driver specific image creation options for LUKS. +# +# @file Node to create the image format on +# @size Size of the virtual disk in bytes +# +# Since: 2.12 +## +{ 'struct': 'BlockdevCreateOptionsLUKS', + 'data': { 'file': 'BlockdevRef', + 'qcrypto': 'QCryptoBlockCreateOptionsLUKS', + 'size': 'size' } } + +## # @BlockdevCreateOptionsNfs: # # Driver specific image creation options for NFS.
@@ -3643,7 +3658,7 @@ 'http': 'BlockdevCreateNotSupported', 'https': 'BlockdevCreateNotSupported', 'iscsi': 'BlockdevCreateNotSupported', - 'luks': 'BlockdevCreateNotSupported', + 'luks': 'BlockdevCreateOptionsLUKS', 'nbd': 'BlockdevCreateNotSupported', 'nfs': 'BlockdevCreateOptionsNfs', 'null-aio': 'BlockdevCreateNotSupported',
block/crypto.c +34 -0
@@ -543,6 +543,39 @@ static int block_crypto_open_luks(BlockDriverState *bs, bs, options, flags, errp); } +static int coroutine_fn +block_crypto_co_create_luks (BlockdevCreateOptions *create_options,Error **errp) +{ +BlockdevCreateOptionsLUKS *luks_opts; +BlockDriverState *bs =NULL ; +QCryptoBlockCreateOptions create_opts; +int ret; + +assert (create_options->driver ==BLOCKDEV_DRIVER_LUKS ); + luks_opts = &create_options->u.luks; + + bs =bdrv_open_blockdev_ref (luks_opts->file, errp); +if (bs ==NULL ) { +return -EIO ; + } + + create_opts = (QCryptoBlockCreateOptions ) { + .format =Q_CRYPTO_BLOCK_FORMAT_LUKS , + .u.luks = *luks_opts->qcrypto, + }; + + ret =block_crypto_co_create_generic (bs, luks_opts->size, &create_opts, + errp); +if (ret <0 ) { + goto fail; + } + + ret =0 ; +fail: +bdrv_unref (bs); +return ret; +} + static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,QemuOpts *opts,Error **errp)
@@ -647,6 +680,7 @@ BlockDriver bdrv_crypto_luks = { .bdrv_open = block_crypto_open_luks, .bdrv_close = block_crypto_close, .bdrv_child_perm = bdrv_format_default_perms, + .bdrv_co_create = block_crypto_co_create_luks, .bdrv_co_create_opts = block_crypto_co_create_opts_luks, .bdrv_truncate = block_crypto_truncate, .create_opts = &block_crypto_create_opts_luks,
Signature · 1 line
2.13.6
Re: [Qemu-devel] [PATCH 3/6] luks: Support .bdrv_co_create
Quoted 15 lines · Kevin Wolf
This adds the .bdrv_co_create driver callback to luks, which enables image creation over QMP.
Signed-off-by Kevin Wolf <[email protected]>--- qapi/block-core.json | 17 ++++++++++++++++- block/crypto.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-)qapi/block-core.json +0
-0@@ -3452,6 +3452,21 @@ '*preallocation': 'PreallocMode' } }
qapi/block-core.json line 3462
Quoted 10 lines
qapi/block-core.json +8
-0## +# @BlockdevCreateOptionsLUKS: +# +# Driver specific image creation options for LUKS. +# +# @file Node to create the image format on +# @size Size of the virtual disk in bytes +# +# Since: 2.12
Well, as long as we make it by Tuesday :)
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 3/6] luks: Support .bdrv_co_create
In reply to Kevin WolfQuoted 20 lines · Kevin Wolf
This adds the .bdrv_co_create driver callback to luks, which enables image creation over QMP.
Signed-off-by Kevin Wolf <[email protected]>--- qapi/block-core.json | 17 ++++++++++++++++- block/crypto.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-)qapi/block-core.json +3
-0@@ -3452,6 +3452,21 @@ '*preallocation': 'PreallocMode' } } ## +# @BlockdevCreateOptionsLUKS: +# +# Driver specific image creation options for LUKS.
qapi/block-core.json line 3467
Quoted 10 lines
qapi/block-core.json +10
-0+# +# @file Node to create the image format on +# @size Size of the virtual disk in bytes +# +# Since: 2.12 +## +{ 'struct': 'BlockdevCreateOptionsLUKS', + 'data': { 'file': 'BlockdevRef', + 'qcrypto': 'QCryptoBlockCreateOptionsLUKS', + 'size': 'size' } }
s/qcrypto/crypto/ in this field.
I do wonder about whether instead of embedding QCryptoBlockCreateOptionsLUKS as a field, we could instead use QCryptoBlockCreateOptionsLUKS as the base struct and just inherit from it to add file/size.
I'm not really fussed, but I wonder if you/Eric have any thoughts on the pros or cons of inheritance vs embedding in this case.
Quoted 69 lines
qapi/block-core.json +3
-1+ +## # @BlockdevCreateOptionsNfs: # # Driver specific image creation options for NFS.@@ -3643,7 +3658,7 @@ 'http': 'BlockdevCreateNotSupported', 'https': 'BlockdevCreateNotSupported', 'iscsi': 'BlockdevCreateNotSupported', - 'luks': 'BlockdevCreateNotSupported', + 'luks': 'BlockdevCreateOptionsLUKS', 'nbd': 'BlockdevCreateNotSupported', 'nfs': 'BlockdevCreateOptionsNfs', 'null-aio': 'BlockdevCreateNotSupported',block/crypto.c +34
-0@@ -543,6 +543,39 @@ static int block_crypto_open_luks(BlockDriverState *bs, bs, options, flags, errp); } +static int coroutine_fn +block_crypto_co_create_luks (BlockdevCreateOptions *create_options,Error **errp) +{ +BlockdevCreateOptionsLUKS *luks_opts; +BlockDriverState *bs =NULL ; +QCryptoBlockCreateOptions create_opts; +int ret; + +assert (create_options->driver ==BLOCKDEV_DRIVER_LUKS ); + luks_opts = &create_options->u.luks; + + bs =bdrv_open_blockdev_ref (luks_opts->file, errp); +if (bs ==NULL ) { +return -EIO ; + } + + create_opts = (QCryptoBlockCreateOptions ) { + .format =Q_CRYPTO_BLOCK_FORMAT_LUKS , + .u.luks = *luks_opts->qcrypto, + }; + + ret =block_crypto_co_create_generic (bs, luks_opts->size, &create_opts, + errp); +if (ret <0 ) { + goto fail; + } + + ret =0 ; +fail: +bdrv_unref (bs); +return ret; +} + static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,QemuOpts *opts,Error **errp)@@ -647,6 +680,7 @@ BlockDriver bdrv_crypto_luks = { .bdrv_open = block_crypto_open_luks, .bdrv_close = block_crypto_close, .bdrv_child_perm = bdrv_format_default_perms, + .bdrv_co_create = block_crypto_co_create_luks, .bdrv_co_create_opts = block_crypto_co_create_opts_luks, .bdrv_truncate = block_crypto_truncate, .create_opts = &block_crypto_create_opts_luks,Signature · 1 line
2.13.6
Regards,
Daniel
Signature · 3 lines
|: 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 3/6] luks: Support .bdrv_co_create
Am 12.03.2018 um 12:40 hat Daniel P. Berrangé geschrieben:
Quoted 34 lines · Kevin Wolf
Quoted 20 lines
This adds the .bdrv_co_create driver callback to luks, which enables image creation over QMP.
Signed-off-by Kevin Wolf <[email protected]>--- qapi/block-core.json | 17 ++++++++++++++++- block/crypto.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-)qapi/block-core.json +3
-0@@ -3452,6 +3452,21 @@ '*preallocation': 'PreallocMode' } } ## +# @BlockdevCreateOptionsLUKS: +# +# Driver specific image creation options for LUKS.qapi/block-core.json line 3467
Quoted 10 lines
qapi/block-core.json +10
-0+# +# @file Node to create the image format on +# @size Size of the virtual disk in bytes +# +# Since: 2.12 +## +{ 'struct': 'BlockdevCreateOptionsLUKS', + 'data': { 'file': 'BlockdevRef', + 'qcrypto': 'QCryptoBlockCreateOptionsLUKS', + 'size': 'size' } }s/qcrypto/crypto/ in this field.
Quoted 6 lines
I do wonder about whether instead of embedding QCryptoBlockCreateOptionsLUKS as a field, we could instead use QCryptoBlockCreateOptionsLUKS as the base struct and just inherit from it to add file/size.
I'm not really fussed, but I wonder if you/Eric have any thoughts on the pros or cons of inheritance vs embedding in this case.
I don't think QAPI has a way to embed it in JSON, but still provide a QCryptoBlockCreateOptionsLUKS C object. Originally I wanted to use inheritance, but instead of having the struct type reused, you get all field definitions copied. I guess you could then manually cast to the base type and it would still work, but it didn't really feel clean.
Kevin
Re: [Qemu-devel] [PATCH 3/6] luks: Support .bdrv_co_create
Quoted 39 lines · Kevin Wolf
Am 12.03.2018 um 12:40 hat Daniel P. Berrangé geschrieben:
Quoted 38 lines · Kevin Wolf
Quoted 20 lines
This adds the .bdrv_co_create driver callback to luks, which enables image creation over QMP.
Signed-off-by Kevin Wolf <[email protected]>--- qapi/block-core.json | 17 ++++++++++++++++- block/crypto.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-)qapi/block-core.json +3
-0@@ -3452,6 +3452,21 @@ '*preallocation': 'PreallocMode' } } ## +# @BlockdevCreateOptionsLUKS: +# +# Driver specific image creation options for LUKS.qapi/block-core.json line 3467
Quoted 10 lines
qapi/block-core.json +10
-0+# +# @file Node to create the image format on +# @size Size of the virtual disk in bytes +# +# Since: 2.12 +## +{ 'struct': 'BlockdevCreateOptionsLUKS', + 'data': { 'file': 'BlockdevRef', + 'qcrypto': 'QCryptoBlockCreateOptionsLUKS', + 'size': 'size' } }s/qcrypto/crypto/ in this field.
I do wonder about whether instead of embedding QCryptoBlockCreateOptionsLUKS as a field, we could instead use QCryptoBlockCreateOptionsLUKS as the base struct and just inherit from it to add file/size.
Quoted 8 lines
Quoted 2 lines
I'm not really fussed, but I wonder if you/Eric have any thoughts on the pros or cons of inheritance vs embedding in this case.
I don't think QAPI has a way to embed it in JSON, but still provide a QCryptoBlockCreateOptionsLUKS C object. Originally I wanted to use inheritance, but instead of having the struct type reused, you get all field definitions copied. I guess you could then manually cast to the base type and it would still work, but it didn't really feel clean.
IIRC, QAPI generates a method
$BASETYPE *qapi_$TYPENAME_base(const $TYPENAME *obj)
which just does the blind cast to the base type.
Regards,
Daniel
Signature · 3 lines
|: 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 3/6] luks: Support .bdrv_co_create
Am 12.03.2018 um 12:50 hat Daniel P. Berrangé geschrieben:
Quoted 49 lines · Kevin Wolf
Quoted 39 lines
Am 12.03.2018 um 12:40 hat Daniel P. Berrangé geschrieben:
Quoted 38 lines · Kevin Wolf
Quoted 20 lines
This adds the .bdrv_co_create driver callback to luks, which enables image creation over QMP.
Signed-off-by Kevin Wolf <[email protected]>--- qapi/block-core.json | 17 ++++++++++++++++- block/crypto.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-)qapi/block-core.json +3
-0@@ -3452,6 +3452,21 @@ '*preallocation': 'PreallocMode' } } ## +# @BlockdevCreateOptionsLUKS: +# +# Driver specific image creation options for LUKS.qapi/block-core.json line 3467
Quoted 10 lines
qapi/block-core.json +10
-0+# +# @file Node to create the image format on +# @size Size of the virtual disk in bytes +# +# Since: 2.12 +## +{ 'struct': 'BlockdevCreateOptionsLUKS', + 'data': { 'file': 'BlockdevRef', + 'qcrypto': 'QCryptoBlockCreateOptionsLUKS', + 'size': 'size' } }s/qcrypto/crypto/ in this field.
I do wonder about whether instead of embedding QCryptoBlockCreateOptionsLUKS as a field, we could instead use QCryptoBlockCreateOptionsLUKS as the base struct and just inherit from it to add file/size.
Quoted 8 lines
Quoted 2 lines
I'm not really fussed, but I wonder if you/Eric have any thoughts on the pros or cons of inheritance vs embedding in this case.
I don't think QAPI has a way to embed it in JSON, but still provide a QCryptoBlockCreateOptionsLUKS C object. Originally I wanted to use inheritance, but instead of having the struct type reused, you get all field definitions copied. I guess you could then manually cast to the base type and it would still work, but it didn't really feel clean.
Quoted 5 lines
IIRC, QAPI generates a method
$BASETYPE *qapi_$TYPENAME_base(const $TYPENAME *obj)
which just does the blind cast to the base type.
Oh, thanks, I didn't know that one! That makes it an offical API that the QAPI generators must keep working, so it should be fine to use. I'll give it a try then.
Kevin
Re: [Qemu-devel] [PATCH 3/6] luks: Support .bdrv_co_create
In reply to Kevin WolfQuoted 12 lines · Kevin Wolf
Quoted 12 lines
qapi/block-core.json line 3467
Quoted 5 lines
qapi/block-core.json +5
-0+## +{ 'struct': 'BlockdevCreateOptionsLUKS', + 'data': { 'file': 'BlockdevRef', + 'qcrypto': 'QCryptoBlockCreateOptionsLUKS', + 'size': 'size' } }s/qcrypto/crypto/ in this field.
I do wonder about whether instead of embedding QCryptoBlockCreateOptionsLUKS as a field, we could instead use QCryptoBlockCreateOptionsLUKS as the base struct and just inherit from it to add file/size.
Quoted 8 lines
Quoted 2 lines
I'm not really fussed, but I wonder if you/Eric have any thoughts on the pros or cons of inheritance vs embedding in this case.
I don't think QAPI has a way to embed it in JSON, but still provide a QCryptoBlockCreateOptionsLUKS C object. Originally I wanted to use inheritance, but instead of having the struct type reused, you get all field definitions copied. I guess you could then manually cast to the base type and it would still work, but it didn't really feel clean.
The QAPI generators generate a function (qapi_NAME_base()) which will convert any derived type back to the parent class, which is cleaner than manually casting; if that makes the decision to use inheritance any easier.
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
PATCH 4/6 luks: Turn invalid assertion into check
In reply to Kevin WolfThe .bdrv_getlength implementation of the crypto block driver asserted that the payload offset isn't after EOF. This is an invalid assertion to make as the image file could be corrupted. Instead, check it and return -EIO if the file is too small for the payload offset.
Zero length images are fine, so trigger -EIO only on offset > len, not on offset >= len as the assertion did before.
--- block/crypto.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
block/crypto.c +4 -1
@@ -518,7 +518,10 @@ static int64_t block_crypto_getlength(BlockDriverState *bs)uint64_t offset =qcrypto_block_get_payload_offset (crypto->block);assert (offset <INT64_MAX ); -assert (offset < len); + +if (offset > len) { +return -EIO ; + } len -= offset;
Signature · 1 line
2.13.6
Re: [Qemu-devel] [PATCH 4/6] luks: Turn invalid assertion into check
Quoted 4 lines · Kevin Wolf
The .bdrv_getlength implementation of the crypto block driver asserted that the payload offset isn't after EOF. This is an invalid assertion to make as the image file could be corrupted. Instead, check it and return
-EIOif the file is too small for the payload offset.
Good catch. Probably not a CVE (unless someone can argue some way that causing a crash on an attempt to load a maliciously corrupted file can be used as a denial of service across a privilege boundary), but definitely needs fixing.
Quoted 14 lines
Zero length images are fine, so trigger
-EIOonly on offset > len, not on offset >= len as the assertion did before.Signed-off-by Kevin Wolf <[email protected]>--- block/crypto.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)block/crypto.c +0
-0
block/crypto.c static int64_t block_crypto_getlength(BlockDriverState *bs) · line 520
Quoted 3 lines
block/crypto.c +0
-0@@ -518,7 +518,10 @@ static int64_t block_crypto_getlength(BlockDriverState *bs)uint64_t offset =qcrypto_block_get_payload_offset (crypto->block);assert (offset <INT64_MAX );
Umm, if the file can be corrupted, what's to prevent someone from sticking in a negative size that fails this assertion?
Quoted 9 lines
block/crypto.c +4
-1-assert (offset < len); + +if (offset > len) { +return -EIO ; + } len -= offset;
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 4/6] luks: Turn invalid assertion into check
Am 09.03.2018 um 21:19 hat Eric Blake geschrieben:
Quoted 20 lines · Kevin Wolf
Quoted 4 lines
The .bdrv_getlength implementation of the crypto block driver asserted that the payload offset isn't after EOF. This is an invalid assertion to make as the image file could be corrupted. Instead, check it and return
-EIOif the file is too small for the payload offset.Good catch. Probably not a CVE (unless someone can argue some way that causing a crash on an attempt to load a maliciously corrupted file can be used as a denial of service across a privilege boundary), but definitely needs fixing.
Quoted 9 lines
Zero length images are fine, so trigger
-EIOonly on offset > len, not on offset >= len as the assertion did before.Signed-off-by Kevin Wolf <[email protected]>--- block/crypto.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
Quoted 10 lines
block/crypto.c static int64_t block_crypto_getlength(BlockDriverState *bs) · line 519
Quoted 7 lines
block/crypto.c +0
-0@@ -518,7 +518,10 @@ static int64_t block_crypto_getlength(BlockDriverState *bs)uint64_t offset =qcrypto_block_get_payload_offset (crypto->block);assert (offset <INT64_MAX );Umm, if the file can be corrupted, what's to prevent someone from sticking in a negative size that fails this assertion?
In qcrypto_block_luks_open():
block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
block->payload_offset = luks->header.payload_offset *
block->sector_sizeThe sector size is 512LL, and luks->header.payload_offset is 32 bit.
But I just saw that block_crypto_truncate() has another wrong assertion. Maybe I should fix that and write a test case for it. Not sure if I'll add it to this series or as a follow-up during the freeze.
Kevin
Re: [Qemu-devel] [PATCH 4/6] luks: Turn invalid assertion into check
In reply to Kevin WolfQuoted 18 lines · Kevin Wolf
The .bdrv_getlength implementation of the crypto block driver asserted that the payload offset isn't after EOF. This is an invalid assertion to make as the image file could be corrupted. Instead, check it and return
-EIOif the file is too small for the payload offset.Zero length images are fine, so trigger
-EIOonly on offset > len, not on offset >= len as the assertion did before.Signed-off-by Kevin Wolf <[email protected]>--- block/crypto.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)block/crypto.c +0
-0
block/crypto.c static int64_t block_crypto_getlength(BlockDriverState *bs) · line 526
Quoted 10 lines
block/crypto.c +4
-1@@ -518,7 +518,10 @@ static int64_t block_crypto_getlength(BlockDriverState *bs)uint64_t offset =qcrypto_block_get_payload_offset (crypto->block);assert (offset <INT64_MAX ); -assert (offset < len); + +if (offset > len) { +return -EIO ; + } len -= offset;
Regards,
Daniel
Signature · 3 lines
|: 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 :|
PATCH 5/6 luks: Catch integer overflow for huge sizes
In reply to Kevin WolfWhen you request an image size close to UINT64_MAX, the addition of the crypto header may cause an integer overflow. Catch it instead of silently truncating the image size.
--- block/crypto.c | 5 +++++ 1 file changed, 5 insertions(+)
block/crypto.c +5 -0
@@ -102,6 +102,11 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block, {struct BlockCryptoCreateData *data = opaque; +if (headerlen >UINT64_MAX - data->size) { +error_setg (errp,"The requested file size is too large" ); +return -EFBIG ; + } +/* User provided size should reflect amount of space made * available to the guest, so we must take account of that * which will be used by the crypto header
Signature · 1 line
2.13.6
Re: [Qemu-devel] [PATCH 5/6] luks: Catch integer overflow for huge sizes
Quoted 14 lines · Kevin Wolf
When you request an image size close to
UINT64_MAX, the addition of the crypto header may cause an integer overflow. Catch it instead of silently truncating the image size.Signed-off-by Kevin Wolf <[email protected]>--- block/crypto.c | 5 +++++ 1 file changed, 5 insertions(+)block/crypto.c +0
-0
block/crypto.c static ssize_t block_crypto_init_func(QCryptoBlock *block, · line 105
Quoted 4 lines
block/crypto.c +1
-0@@ -102,6 +102,11 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block, {struct BlockCryptoCreateData *data = opaque; +if (headerlen >UINT64_MAX - data->size) {
INT64_MAX, please. We are further bounded by having to fit within off_t (signed) rather than uint64_t.
Quoted 8 lines
block/crypto.c +4
-0+error_setg (errp,"The requested file size is too large" ); +return -EFBIG ; + } +/* User provided size should reflect amount of space made * available to the guest, so we must take account of that * which will be used by the crypto header
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 5/6] luks: Catch integer overflow for huge sizes
In reply to Kevin WolfQuoted 15 lines · Kevin Wolf
When you request an image size close to
UINT64_MAX, the addition of the crypto header may cause an integer overflow. Catch it instead of silently truncating the image size.Signed-off-by Kevin Wolf <[email protected]>--- block/crypto.c | 5 +++++ 1 file changed, 5 insertions(+)block/crypto.c +0
-0@@ -102,6 +102,11 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block, {
block/crypto.c static ssize_t block_crypto_init_func(QCryptoBlock *block, · line 112
Quoted 10 lines
block/crypto.c +5
-0struct BlockCryptoCreateData *data = opaque; +if (headerlen >UINT64_MAX - data->size) { +error_setg (errp,"The requested file size is too large" ); +return -EFBIG ; + } +/* User provided size should reflect amount of space made * available to the guest, so we must take account of that * which will be used by the crypto header
(if using INT64_MAX as Eric suggests)
Regards,
Daniel
Signature · 3 lines
|: 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 :|
PATCH 6/6 qemu-iotests: Test luks QMP image creation
In reply to Kevin Wolf--- tests/qemu-iotests/208 | 211 +++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/208.out | 136 ++++++++++++++++++++++++++++ tests/qemu-iotests/common.rc | 2 +- tests/qemu-iotests/group | 1 + 4 files changed, 349 insertions(+), 1 deletion(-) create mode 100755 tests/qemu-iotests/208 create mode 100644 tests/qemu-iotests/208.out
tests/qemu-iotests/208 new file +211 -0
@@ -0,0 +1,211 @@ +#!/bin/bash +# +# Test luks and file image creation +# +# Copyright (C) 2018 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +# creator [email protected] + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +status=1 # failure is the default! + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt luks +_supported_proto file +_supported_os Linux + +function do_run_qemu() +{ + echo Testing: "$@" + $QEMU -nographic -qmp stdio -serial none "$@" + echo +} + +function run_qemu() +{ + do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp \ + | _filter_qemu | _filter_imgfmt \ + | _filter_actual_image_size +} + +echo +echo "=== Successful image creation (defaults) ===" +echo + +size=$((128 * 1024 * 1024)) + +run_qemu -object secret,id=keysec0,data="foo" <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "file", + "filename": "$TEST_IMG_FILE", + "size": 0 + } +} +{ "execute": "blockdev-add", + "arguments": { + "driver": "file", + "node-name": "imgfile", + "filename": "$TEST_IMG_FILE" + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "imgfile", + "qcrypto": { "key-secret": "keysec0" }, + "size": $size + } +} +{ "execute": "quit" } +EOF + +_img_info --format-specific | _filter_img_info --format-specific + +echo +echo "=== Successful image creation (with non-default options) ===" +echo + +# Choose a different size to show that we got a new image +size=$((64 * 1024 * 1024)) + +run_qemu -object secret,id=keysec0,data="foo" <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "file", + "filename": "$TEST_IMG_FILE", + "size": 0 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": { + "driver": "file", + "filename": "$TEST_IMG_FILE" + }, + "size": $size, + "qcrypto": { + "key-secret": "keysec0", + "cipher-alg": "twofish-128", + "cipher-mode": "ctr", + "ivgen-alg": "plain64", + "ivgen-hash-alg": "md5", + "hash-alg": "sha1", + "iter-time": 10 + } + } +} +{ "execute": "quit" } +EOF + +_img_info --format-specific | _filter_img_info --format-specific + +echo +echo "=== Invalid BlockdevRef ===" +echo + +run_qemu <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "this doesn't exist", + "qcrypto": {}, + "size": $size + } +} +{ "execute": "quit" } +EOF + +echo +echo "=== Zero size ===" +echo + +run_qemu -blockdev driver=file,filename="$TEST_IMG_FILE",node-name=node0 \ + -object secret,id=keysec0,data="foo" <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "qcrypto": { "key-secret": "keysec0" }, + "size": 0 + } +} +{ "execute": "quit" } +EOF + +_img_info | _filter_img_info + + +echo +echo "=== Invalid sizes ===" +echo + +# TODO Negative image sizes aren't handled correctly, but this is a problem +# with QAPI's implementation of the 'size' type and affects other commands as +# well. Once this is fixed, we may want to add a test case here. + +# 1. 2^64 - 512 +# 2. 2^63 = 8 EB (qemu-img enforces image sizes less than this) +# 3. 2^63 - 512 (generally valid, but with the crypto header the file will +# exceed 63 bits) + +run_qemu -blockdev driver=file,filename="$TEST_IMG_FILE",node-name=node0 \ + -object secret,id=keysec0,data="foo" <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "qcrypto": { "key-secret": "keysec0" }, + "size": 18446744073709551104 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "qcrypto": { "key-secret": "keysec0" }, + "size": 9223372036854775808 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "qcrypto": { "key-secret": "keysec0" }, + "size": 9223372036854775296 + } +} +{ "execute": "quit" } +EOF + +# success, all done +echo "*** done" +rm -f $seq.full +status=0
tests/qemu-iotests/208.out new file +136 -0
136 lines not on this page · raw message
tests/qemu-iotests/common.rc +1 -1
@@ -332,7 +332,7 @@ _img_info() discard=0 regex_json_spec_start='^ *"format-specific": \{' - $QEMU_IMG info "$@" "$TEST_IMG" 2>&1 | \ + $QEMU_IMG info $QEMU_IMG_EXTRA_ARGS "$@" "$TEST_IMG" 2>&1 | \ sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \ -e "s#$TEST_DIR#TEST_DIR#g" \ -e "s#$IMGFMT#IMGFMT#g" \
tests/qemu-iotests/group +1 -0
@@ -204,3 +204,4 @@ 205 rw auto quick 206 rw auto 207 rw auto +208 rw auto
Signature · 1 line
2.13.6
Re: [Qemu-devel] [PATCH 6/6] qemu-iotests: Test luks QMP image creation
Quoted 9 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- tests/qemu-iotests/208 | 211 +++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/208.out | 136 ++++++++++++++++++++++++++++ tests/qemu-iotests/common.rc | 2 +- tests/qemu-iotests/group | 1 + 4 files changed, 349 insertions(+), 1 deletion(-)create mode 100755 tests/qemu-iotests/208
create mode 100644 tests/qemu-iotests/208.out
I've seen another patch using 208 - someone gets to renumber ;)
tests/qemu-iotests/208 line 28
Quoted 8 lines
tests/qemu-iotests/208 new file +8
-0+# creator [email protected] + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +status=1 # failure is the default!
Whatever happened to Jeff's efforts to reduce pointless boilerplate?
tests/qemu-iotests/208 line 41
Quoted 4 lines
tests/qemu-iotests/208 new file +4
-0+function do_run_qemu() +{ + echo Testing: "$@" + $QEMU -nographic -qmp stdio -serial none "$@"
Is -nodefaults better than -serial none?
At any rate, my comments are trivial, whether or not you do something about them, so
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 6/6] qemu-iotests: Test luks QMP image creation
In reply to Kevin WolfQuoted 33 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- tests/qemu-iotests/208 | 211 +++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/208.out | 136 ++++++++++++++++++++++++++++ tests/qemu-iotests/common.rc | 2 +- tests/qemu-iotests/group | 1 + 4 files changed, 349 insertions(+), 1 deletion(-) create mode 100755 tests/qemu-iotests/208 create mode 100644 tests/qemu-iotests/208.outtests/qemu-iotests/208 new file +16
-0+ +run_qemu -object secret,id=keysec0,data="foo" <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "file", + "filename": "$TEST_IMG_FILE", + "size": 0 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": { + "driver": "file", + "filename": "$TEST_IMG_FILE"
tests/qemu-iotests/208 line 118
Quoted 10 lines
tests/qemu-iotests/208 new file +10
-0+ }, + "size": $size, + "qcrypto": { + "key-secret": "keysec0", + "cipher-alg": "twofish-128", + "cipher-mode": "ctr", + "ivgen-alg": "plain64", + "ivgen-hash-alg": "md5", + "hash-alg": "sha1", + "iter-time": 10
I'd recommend using 'iter-time': 10, in every single x-blockdev-create command in this file, otherwise this test will be unneccessarily slow to execute as each x-blockdev-create will burn 2 seconds of time.
Regards,
Daniel
Signature · 3 lines
|: 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 :|