* [Qemu-devel] [PATCH v3] LUKS: support preallocation
@ 2019-07-11 15:09 Maxim Levitsky
2019-07-11 16:27 ` [Qemu-devel] [Qemu-block] " Stefano Garzarella
2019-07-12 14:45 ` [Qemu-devel] " no-reply
0 siblings, 2 replies; 7+ messages in thread
From: Maxim Levitsky @ 2019-07-11 15:09 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Maxim Levitsky, Max Reitz
preallocation=off and preallocation=metadata
both allocate luks header only, and preallocation=falloc/full
is passed to underlying file.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534951
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
Note that QMP support was only compile tested, since I am still learning
on how to use it.
If there is some library/script/etc which makes it more high level,
I would more that glad to hear about it. So far I used the qmp-shell
Also can I use qmp's blockdev-create outside a vm running?
block/crypto.c | 29 ++++++++++++++++++++++++++---
qapi/block-core.json | 5 ++++-
2 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/block/crypto.c b/block/crypto.c
index 8237424ae6..034a645652 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -74,6 +74,7 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,
struct BlockCryptoCreateData {
BlockBackend *blk;
uint64_t size;
+ PreallocMode prealloc;
};
@@ -112,7 +113,7 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
* available to the guest, so we must take account of that
* which will be used by the crypto header
*/
- return blk_truncate(data->blk, data->size + headerlen, PREALLOC_MODE_OFF,
+ return blk_truncate(data->blk, data->size + headerlen, data->prealloc,
errp);
}
@@ -251,6 +252,7 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
static int block_crypto_co_create_generic(BlockDriverState *bs,
int64_t size,
QCryptoBlockCreateOptions *opts,
+ PreallocMode prealloc,
Error **errp)
{
int ret;
@@ -266,9 +268,14 @@ static int block_crypto_co_create_generic(BlockDriverState *bs,
goto cleanup;
}
+ if (prealloc == PREALLOC_MODE_METADATA) {
+ prealloc = PREALLOC_MODE_OFF;
+ }
+
data = (struct BlockCryptoCreateData) {
.blk = blk,
.size = size,
+ .prealloc = prealloc,
};
crypto = qcrypto_block_create(opts, NULL,
@@ -500,6 +507,7 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
BlockdevCreateOptionsLUKS *luks_opts;
BlockDriverState *bs = NULL;
QCryptoBlockCreateOptions create_opts;
+ PreallocMode preallocation = PREALLOC_MODE_OFF;
int ret;
assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
@@ -515,8 +523,11 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
.u.luks = *qapi_BlockdevCreateOptionsLUKS_base(luks_opts),
};
+ if (luks_opts->has_preallocation)
+ preallocation = luks_opts->preallocation;
+
ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
- errp);
+ preallocation, errp);
if (ret < 0) {
goto fail;
}
@@ -534,12 +545,24 @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
QCryptoBlockCreateOptions *create_opts = NULL;
BlockDriverState *bs = NULL;
QDict *cryptoopts;
+ PreallocMode prealloc;
+ char *buf = NULL;
int64_t size;
int ret;
+ Error *local_err = NULL;
/* Parse options */
size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+ buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
+ prealloc = qapi_enum_parse(&PreallocMode_lookup, buf,
+ PREALLOC_MODE_OFF, &local_err);
+ g_free(buf);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return -EINVAL;
+ }
+
cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
&block_crypto_create_opts_luks,
true);
@@ -565,7 +588,7 @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
}
/* Create format layer */
- ret = block_crypto_co_create_generic(bs, size, create_opts, errp);
+ ret = block_crypto_co_create_generic(bs, size, create_opts, prealloc, errp);
if (ret < 0) {
goto fail;
}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0d43d4f37c..ebcfc9f903 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4205,13 +4205,16 @@
#
# @file Node to create the image format on
# @size Size of the virtual disk in bytes
+# @preallocation Preallocation mode for the new image (default: off;
+# allowed values: off/falloc/full
#
# Since: 2.12
##
{ 'struct': 'BlockdevCreateOptionsLUKS',
'base': 'QCryptoBlockCreateOptionsLUKS',
'data': { 'file': 'BlockdevRef',
- 'size': 'size' } }
+ 'size': 'size',
+ '*preallocation': 'PreallocMode' } }
##
# @BlockdevCreateOptionsNfs:
--
2.17.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3] LUKS: support preallocation
2019-07-11 15:09 [Qemu-devel] [PATCH v3] LUKS: support preallocation Maxim Levitsky
@ 2019-07-11 16:27 ` Stefano Garzarella
2019-07-14 14:51 ` Maxim Levitsky
2019-07-12 14:45 ` [Qemu-devel] " no-reply
1 sibling, 1 reply; 7+ messages in thread
From: Stefano Garzarella @ 2019-07-11 16:27 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, Max Reitz, qemu-devel, qemu-block, Markus Armbruster
On Thu, Jul 11, 2019 at 06:09:40PM +0300, Maxim Levitsky wrote:
> preallocation=off and preallocation=metadata
> both allocate luks header only, and preallocation=falloc/full
> is passed to underlying file.
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534951
>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
>
> ---
>
> Note that QMP support was only compile tested, since I am still learning
> on how to use it.
>
> If there is some library/script/etc which makes it more high level,
> I would more that glad to hear about it. So far I used the qmp-shell
>
> Also can I use qmp's blockdev-create outside a vm running?
>
> block/crypto.c | 29 ++++++++++++++++++++++++++---
> qapi/block-core.json | 5 ++++-
> 2 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/block/crypto.c b/block/crypto.c
> index 8237424ae6..034a645652 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -74,6 +74,7 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,
> struct BlockCryptoCreateData {
> BlockBackend *blk;
> uint64_t size;
> + PreallocMode prealloc;
> };
>
>
> @@ -112,7 +113,7 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
> * available to the guest, so we must take account of that
> * which will be used by the crypto header
> */
> - return blk_truncate(data->blk, data->size + headerlen, PREALLOC_MODE_OFF,
> + return blk_truncate(data->blk, data->size + headerlen, data->prealloc,
> errp);
> }
>
> @@ -251,6 +252,7 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
> static int block_crypto_co_create_generic(BlockDriverState *bs,
> int64_t size,
> QCryptoBlockCreateOptions *opts,
> + PreallocMode prealloc,
> Error **errp)
> {
> int ret;
> @@ -266,9 +268,14 @@ static int block_crypto_co_create_generic(BlockDriverState *bs,
> goto cleanup;
> }
>
> + if (prealloc == PREALLOC_MODE_METADATA) {
> + prealloc = PREALLOC_MODE_OFF;
> + }
> +
> data = (struct BlockCryptoCreateData) {
> .blk = blk,
> .size = size,
> + .prealloc = prealloc,
> };
>
> crypto = qcrypto_block_create(opts, NULL,
> @@ -500,6 +507,7 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
> BlockdevCreateOptionsLUKS *luks_opts;
> BlockDriverState *bs = NULL;
> QCryptoBlockCreateOptions create_opts;
> + PreallocMode preallocation = PREALLOC_MODE_OFF;
> int ret;
>
> assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
> @@ -515,8 +523,11 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
> .u.luks = *qapi_BlockdevCreateOptionsLUKS_base(luks_opts),
> };
>
> + if (luks_opts->has_preallocation)
> + preallocation = luks_opts->preallocation;
> +
> ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
> - errp);
> + preallocation, errp);
> if (ret < 0) {
> goto fail;
> }
> @@ -534,12 +545,24 @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
> QCryptoBlockCreateOptions *create_opts = NULL;
> BlockDriverState *bs = NULL;
> QDict *cryptoopts;
> + PreallocMode prealloc;
> + char *buf = NULL;
> int64_t size;
> int ret;
> + Error *local_err = NULL;
>
> /* Parse options */
> size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
>
> + buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> + prealloc = qapi_enum_parse(&PreallocMode_lookup, buf,
> + PREALLOC_MODE_OFF, &local_err);
> + g_free(buf);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return -EINVAL;
> + }
> +
> cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
> &block_crypto_create_opts_luks,
> true);
> @@ -565,7 +588,7 @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
> }
>
> /* Create format layer */
> - ret = block_crypto_co_create_generic(bs, size, create_opts, errp);
> + ret = block_crypto_co_create_generic(bs, size, create_opts, prealloc, errp);
> if (ret < 0) {
> goto fail;
> }
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 0d43d4f37c..ebcfc9f903 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -4205,13 +4205,16 @@
> #
> # @file Node to create the image format on
> # @size Size of the virtual disk in bytes
> +# @preallocation Preallocation mode for the new image (default: off;
> +# allowed values: off/falloc/full
Should we add also "metadata" to allowed values? and "since: 4.2"?
I'd like to have (just to have similar documentation with others
preallocation parameters):
# @preallocation Preallocation mode for the new image (default: off;
# allowed values: off, falloc, full, metadata; since: 4.2)
Thanks,
Stefano
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3] LUKS: support preallocation
2019-07-11 15:09 [Qemu-devel] [PATCH v3] LUKS: support preallocation Maxim Levitsky
2019-07-11 16:27 ` [Qemu-devel] [Qemu-block] " Stefano Garzarella
@ 2019-07-12 14:45 ` no-reply
1 sibling, 0 replies; 7+ messages in thread
From: no-reply @ 2019-07-12 14:45 UTC (permalink / raw)
To: mlevitsk; +Cc: kwolf, qemu-block, armbru, qemu-devel, mlevitsk, mreitz
Patchew URL: https://patchew.org/QEMU/20190711150940.17483-1-mlevitsk@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20190711150940.17483-1-mlevitsk@redhat.com
Type: series
Subject: [Qemu-devel] [PATCH v3] LUKS: support preallocation
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Switched to a new branch 'test'
f1b0906cf3 LUKS: support preallocation
=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#73: FILE: block/crypto.c:526:
+ if (luks_opts->has_preallocation)
[...]
total: 1 errors, 0 warnings, 104 lines checked
Commit f1b0906cf3c9 (LUKS: support preallocation) has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190711150940.17483-1-mlevitsk@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3] LUKS: support preallocation
2019-07-11 16:27 ` [Qemu-devel] [Qemu-block] " Stefano Garzarella
@ 2019-07-14 14:51 ` Maxim Levitsky
2019-07-15 8:30 ` Stefano Garzarella
0 siblings, 1 reply; 7+ messages in thread
From: Maxim Levitsky @ 2019-07-14 14:51 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Kevin Wolf, Max Reitz, qemu-devel, qemu-block, Markus Armbruster
On Thu, 2019-07-11 at 18:27 +0200, Stefano Garzarella wrote:
> On Thu, Jul 11, 2019 at 06:09:40PM +0300, Maxim Levitsky wrote:
> > preallocation=off and preallocation=metadata
> > both allocate luks header only, and preallocation=falloc/full
> > is passed to underlying file.
> >
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534951
> >
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> >
> > ---
> >
> > Note that QMP support was only compile tested, since I am still learning
> > on how to use it.
> >
> > If there is some library/script/etc which makes it more high level,
> > I would more that glad to hear about it. So far I used the qmp-shell
> >
> > Also can I use qmp's blockdev-create outside a vm running?
> >
> > block/crypto.c | 29 ++++++++++++++++++++++++++---
> > qapi/block-core.json | 5 ++++-
> > 2 files changed, 30 insertions(+), 4 deletions(-)
> >
> > diff --git a/block/crypto.c b/block/crypto.c
> > index 8237424ae6..034a645652 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -74,6 +74,7 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,
> > struct BlockCryptoCreateData {
> > BlockBackend *blk;
> > uint64_t size;
> > + PreallocMode prealloc;
> > };
> >
> >
> > @@ -112,7 +113,7 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
> > * available to the guest, so we must take account of that
> > * which will be used by the crypto header
> > */
> > - return blk_truncate(data->blk, data->size + headerlen, PREALLOC_MODE_OFF,
> > + return blk_truncate(data->blk, data->size + headerlen, data->prealloc,
> > errp);
> > }
> >
> > @@ -251,6 +252,7 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
> > static int block_crypto_co_create_generic(BlockDriverState *bs,
> > int64_t size,
> > QCryptoBlockCreateOptions *opts,
> > + PreallocMode prealloc,
> > Error **errp)
> > {
> > int ret;
> > @@ -266,9 +268,14 @@ static int block_crypto_co_create_generic(BlockDriverState *bs,
> > goto cleanup;
> > }
> >
> > + if (prealloc == PREALLOC_MODE_METADATA) {
> > + prealloc = PREALLOC_MODE_OFF;
> > + }
> > +
> > data = (struct BlockCryptoCreateData) {
> > .blk = blk,
> > .size = size,
> > + .prealloc = prealloc,
> > };
> >
> > crypto = qcrypto_block_create(opts, NULL,
> > @@ -500,6 +507,7 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
> > BlockdevCreateOptionsLUKS *luks_opts;
> > BlockDriverState *bs = NULL;
> > QCryptoBlockCreateOptions create_opts;
> > + PreallocMode preallocation = PREALLOC_MODE_OFF;
> > int ret;
> >
> > assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
> > @@ -515,8 +523,11 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
> > .u.luks = *qapi_BlockdevCreateOptionsLUKS_base(luks_opts),
> > };
> >
> > + if (luks_opts->has_preallocation)
> > + preallocation = luks_opts->preallocation;
> > +
> > ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
> > - errp);
> > + preallocation, errp);
> > if (ret < 0) {
> > goto fail;
> > }
> > @@ -534,12 +545,24 @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
> > QCryptoBlockCreateOptions *create_opts = NULL;
> > BlockDriverState *bs = NULL;
> > QDict *cryptoopts;
> > + PreallocMode prealloc;
> > + char *buf = NULL;
> > int64_t size;
> > int ret;
> > + Error *local_err = NULL;
> >
> > /* Parse options */
> > size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> >
> > + buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> > + prealloc = qapi_enum_parse(&PreallocMode_lookup, buf,
> > + PREALLOC_MODE_OFF, &local_err);
> > + g_free(buf);
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + return -EINVAL;
> > + }
> > +
> > cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
> > &block_crypto_create_opts_luks,
> > true);
> > @@ -565,7 +588,7 @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
> > }
> >
> > /* Create format layer */
> > - ret = block_crypto_co_create_generic(bs, size, create_opts, errp);
> > + ret = block_crypto_co_create_generic(bs, size, create_opts, prealloc, errp);
> > if (ret < 0) {
> > goto fail;
> > }
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 0d43d4f37c..ebcfc9f903 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -4205,13 +4205,16 @@
> > #
> > # @file Node to create the image format on
> > # @size Size of the virtual disk in bytes
> > +# @preallocation Preallocation mode for the new image (default: off;
> > +# allowed values: off/falloc/full
>
> Should we add also "metadata" to allowed values? and "since: 4.2"?
> I'd like to have (just to have similar documentation with others
> preallocation parameters):
It it support but preallocation=off is the same as preallocation=metadata in luks,
as luks only metadata is the header which is created anyway.
In some sense I should throw a error for preallocation=off, but I suspect that
that will break userspace api.
What do you think?
I forgot to add 'since: 4.2', will do in the next revision of that patch series.
>
> # @preallocation Preallocation mode for the new image (default: off;
> # allowed values: off, falloc, full, metadata; since: 4.2)
>
>
> Thanks,
> Stefano
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3] LUKS: support preallocation
2019-07-14 14:51 ` Maxim Levitsky
@ 2019-07-15 8:30 ` Stefano Garzarella
2019-07-15 8:36 ` Maxim Levitsky
2019-07-15 8:38 ` Max Reitz
0 siblings, 2 replies; 7+ messages in thread
From: Stefano Garzarella @ 2019-07-15 8:30 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, Max Reitz, qemu-devel, qemu-block, Markus Armbruster
On Sun, Jul 14, 2019 at 05:51:51PM +0300, Maxim Levitsky wrote:
> On Thu, 2019-07-11 at 18:27 +0200, Stefano Garzarella wrote:
> > On Thu, Jul 11, 2019 at 06:09:40PM +0300, Maxim Levitsky wrote:
> > > preallocation=off and preallocation=metadata
> > > both allocate luks header only, and preallocation=falloc/full
> > > is passed to underlying file.
> > >
> > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534951
> > >
> > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > >
> > > ---
> > >
> > > Note that QMP support was only compile tested, since I am still learning
> > > on how to use it.
> > >
> > > If there is some library/script/etc which makes it more high level,
> > > I would more that glad to hear about it. So far I used the qmp-shell
> > >
> > > Also can I use qmp's blockdev-create outside a vm running?
> > >
> > > block/crypto.c | 29 ++++++++++++++++++++++++++---
> > > qapi/block-core.json | 5 ++++-
> > > 2 files changed, 30 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/block/crypto.c b/block/crypto.c
> > > index 8237424ae6..034a645652 100644
> > > --- a/block/crypto.c
> > > +++ b/block/crypto.c
> > > @@ -74,6 +74,7 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,
> > > struct BlockCryptoCreateData {
> > > BlockBackend *blk;
> > > uint64_t size;
> > > + PreallocMode prealloc;
> > > };
> > >
> > >
> > > @@ -112,7 +113,7 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
> > > * available to the guest, so we must take account of that
> > > * which will be used by the crypto header
> > > */
> > > - return blk_truncate(data->blk, data->size + headerlen, PREALLOC_MODE_OFF,
> > > + return blk_truncate(data->blk, data->size + headerlen, data->prealloc,
> > > errp);
> > > }
> > >
> > > @@ -251,6 +252,7 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
> > > static int block_crypto_co_create_generic(BlockDriverState *bs,
> > > int64_t size,
> > > QCryptoBlockCreateOptions *opts,
> > > + PreallocMode prealloc,
> > > Error **errp)
> > > {
> > > int ret;
> > > @@ -266,9 +268,14 @@ static int block_crypto_co_create_generic(BlockDriverState *bs,
> > > goto cleanup;
> > > }
> > >
> > > + if (prealloc == PREALLOC_MODE_METADATA) {
> > > + prealloc = PREALLOC_MODE_OFF;
> > > + }
> > > +
> > > data = (struct BlockCryptoCreateData) {
> > > .blk = blk,
> > > .size = size,
> > > + .prealloc = prealloc,
> > > };
> > >
> > > crypto = qcrypto_block_create(opts, NULL,
> > > @@ -500,6 +507,7 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
> > > BlockdevCreateOptionsLUKS *luks_opts;
> > > BlockDriverState *bs = NULL;
> > > QCryptoBlockCreateOptions create_opts;
> > > + PreallocMode preallocation = PREALLOC_MODE_OFF;
> > > int ret;
> > >
> > > assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
> > > @@ -515,8 +523,11 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
> > > .u.luks = *qapi_BlockdevCreateOptionsLUKS_base(luks_opts),
> > > };
> > >
> > > + if (luks_opts->has_preallocation)
> > > + preallocation = luks_opts->preallocation;
> > > +
> > > ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
> > > - errp);
> > > + preallocation, errp);
> > > if (ret < 0) {
> > > goto fail;
> > > }
> > > @@ -534,12 +545,24 @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
> > > QCryptoBlockCreateOptions *create_opts = NULL;
> > > BlockDriverState *bs = NULL;
> > > QDict *cryptoopts;
> > > + PreallocMode prealloc;
> > > + char *buf = NULL;
> > > int64_t size;
> > > int ret;
> > > + Error *local_err = NULL;
> > >
> > > /* Parse options */
> > > size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> > >
> > > + buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> > > + prealloc = qapi_enum_parse(&PreallocMode_lookup, buf,
> > > + PREALLOC_MODE_OFF, &local_err);
> > > + g_free(buf);
> > > + if (local_err) {
> > > + error_propagate(errp, local_err);
> > > + return -EINVAL;
> > > + }
> > > +
> > > cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
> > > &block_crypto_create_opts_luks,
> > > true);
> > > @@ -565,7 +588,7 @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
> > > }
> > >
> > > /* Create format layer */
> > > - ret = block_crypto_co_create_generic(bs, size, create_opts, errp);
> > > + ret = block_crypto_co_create_generic(bs, size, create_opts, prealloc, errp);
> > > if (ret < 0) {
> > > goto fail;
> > > }
> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > index 0d43d4f37c..ebcfc9f903 100644
> > > --- a/qapi/block-core.json
> > > +++ b/qapi/block-core.json
> > > @@ -4205,13 +4205,16 @@
> > > #
> > > # @file Node to create the image format on
> > > # @size Size of the virtual disk in bytes
> > > +# @preallocation Preallocation mode for the new image (default: off;
> > > +# allowed values: off/falloc/full
> >
> > Should we add also "metadata" to allowed values? and "since: 4.2"?
> > I'd like to have (just to have similar documentation with others
> > preallocation parameters):
>
> It it support but preallocation=off is the same as preallocation=metadata in luks,
> as luks only metadata is the header which is created anyway.
> In some sense I should throw a error for preallocation=off, but I suspect that
> that will break userspace api.
> What do you think?
I don't know very well the details, but I agree with you that some
user APIs could expect that preallocation=off is always available.
Maybe we can write something in the comment (explaining that off and metadata
are the same) and make the preallocation=metadata the default choice.
Thanks,
Stefano
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3] LUKS: support preallocation
2019-07-15 8:30 ` Stefano Garzarella
@ 2019-07-15 8:36 ` Maxim Levitsky
2019-07-15 8:38 ` Max Reitz
1 sibling, 0 replies; 7+ messages in thread
From: Maxim Levitsky @ 2019-07-15 8:36 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Kevin Wolf, Max Reitz, qemu-devel, qemu-block, Markus Armbruster
On Mon, 2019-07-15 at 10:30 +0200, Stefano Garzarella wrote:
> On Sun, Jul 14, 2019 at 05:51:51PM +0300, Maxim Levitsky wrote:
> > On Thu, 2019-07-11 at 18:27 +0200, Stefano Garzarella wrote:
> > > On Thu, Jul 11, 2019 at 06:09:40PM +0300, Maxim Levitsky wrote:
> > > > preallocation=off and preallocation=metadata
> > > > both allocate luks header only, and preallocation=falloc/full
> > > > is passed to underlying file.
> > > >
> > > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534951
> > > >
> > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > > >
> > > > ---
> > > >
> > > > Note that QMP support was only compile tested, since I am still learning
> > > > on how to use it.
> > > >
> > > > If there is some library/script/etc which makes it more high level,
> > > > I would more that glad to hear about it. So far I used the qmp-shell
> > > >
> > > > Also can I use qmp's blockdev-create outside a vm running?
> > > >
> > > > block/crypto.c | 29 ++++++++++++++++++++++++++---
> > > > qapi/block-core.json | 5 ++++-
> > > > 2 files changed, 30 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/block/crypto.c b/block/crypto.c
> > > > index 8237424ae6..034a645652 100644
> > > > --- a/block/crypto.c
> > > > +++ b/block/crypto.c
> > > > @@ -74,6 +74,7 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,
> > > > struct BlockCryptoCreateData {
> > > > BlockBackend *blk;
> > > > uint64_t size;
> > > > + PreallocMode prealloc;
> > > > };
> > > >
> > > >
> > > > @@ -112,7 +113,7 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
> > > > * available to the guest, so we must take account of that
> > > > * which will be used by the crypto header
> > > > */
> > > > - return blk_truncate(data->blk, data->size + headerlen, PREALLOC_MODE_OFF,
> > > > + return blk_truncate(data->blk, data->size + headerlen, data->prealloc,
> > > > errp);
> > > > }
> > > >
> > > > @@ -251,6 +252,7 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
> > > > static int block_crypto_co_create_generic(BlockDriverState *bs,
> > > > int64_t size,
> > > > QCryptoBlockCreateOptions *opts,
> > > > + PreallocMode prealloc,
> > > > Error **errp)
> > > > {
> > > > int ret;
> > > > @@ -266,9 +268,14 @@ static int block_crypto_co_create_generic(BlockDriverState *bs,
> > > > goto cleanup;
> > > > }
> > > >
> > > > + if (prealloc == PREALLOC_MODE_METADATA) {
> > > > + prealloc = PREALLOC_MODE_OFF;
> > > > + }
> > > > +
> > > > data = (struct BlockCryptoCreateData) {
> > > > .blk = blk,
> > > > .size = size,
> > > > + .prealloc = prealloc,
> > > > };
> > > >
> > > > crypto = qcrypto_block_create(opts, NULL,
> > > > @@ -500,6 +507,7 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
> > > > BlockdevCreateOptionsLUKS *luks_opts;
> > > > BlockDriverState *bs = NULL;
> > > > QCryptoBlockCreateOptions create_opts;
> > > > + PreallocMode preallocation = PREALLOC_MODE_OFF;
> > > > int ret;
> > > >
> > > > assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
> > > > @@ -515,8 +523,11 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
> > > > .u.luks = *qapi_BlockdevCreateOptionsLUKS_base(luks_opts),
> > > > };
> > > >
> > > > + if (luks_opts->has_preallocation)
> > > > + preallocation = luks_opts->preallocation;
> > > > +
> > > > ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
> > > > - errp);
> > > > + preallocation, errp);
> > > > if (ret < 0) {
> > > > goto fail;
> > > > }
> > > > @@ -534,12 +545,24 @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
> > > > QCryptoBlockCreateOptions *create_opts = NULL;
> > > > BlockDriverState *bs = NULL;
> > > > QDict *cryptoopts;
> > > > + PreallocMode prealloc;
> > > > + char *buf = NULL;
> > > > int64_t size;
> > > > int ret;
> > > > + Error *local_err = NULL;
> > > >
> > > > /* Parse options */
> > > > size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> > > >
> > > > + buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> > > > + prealloc = qapi_enum_parse(&PreallocMode_lookup, buf,
> > > > + PREALLOC_MODE_OFF, &local_err);
> > > > + g_free(buf);
> > > > + if (local_err) {
> > > > + error_propagate(errp, local_err);
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
> > > > &block_crypto_create_opts_luks,
> > > > true);
> > > > @@ -565,7 +588,7 @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
> > > > }
> > > >
> > > > /* Create format layer */
> > > > - ret = block_crypto_co_create_generic(bs, size, create_opts, errp);
> > > > + ret = block_crypto_co_create_generic(bs, size, create_opts, prealloc, errp);
> > > > if (ret < 0) {
> > > > goto fail;
> > > > }
> > > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > > index 0d43d4f37c..ebcfc9f903 100644
> > > > --- a/qapi/block-core.json
> > > > +++ b/qapi/block-core.json
> > > > @@ -4205,13 +4205,16 @@
> > > > #
> > > > # @file Node to create the image format on
> > > > # @size Size of the virtual disk in bytes
> > > > +# @preallocation Preallocation mode for the new image (default: off;
> > > > +# allowed values: off/falloc/full
> > >
> > > Should we add also "metadata" to allowed values? and "since: 4.2"?
> > > I'd like to have (just to have similar documentation with others
> > > preallocation parameters):
> >
> > It it support but preallocation=off is the same as preallocation=metadata in luks,
> > as luks only metadata is the header which is created anyway.
> > In some sense I should throw a error for preallocation=off, but I suspect that
> > that will break userspace api.
> > What do you think?
>
> I don't know very well the details, but I agree with you that some
> user APIs could expect that preallocation=off is always available.
>
> Maybe we can write something in the comment (explaining that off and metadata
> are the same) and make the preallocation=metadata the default choice.
>
> Thanks,
> Stefano
All right!
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3] LUKS: support preallocation
2019-07-15 8:30 ` Stefano Garzarella
2019-07-15 8:36 ` Maxim Levitsky
@ 2019-07-15 8:38 ` Max Reitz
1 sibling, 0 replies; 7+ messages in thread
From: Max Reitz @ 2019-07-15 8:38 UTC (permalink / raw)
To: Stefano Garzarella, Maxim Levitsky
Cc: Kevin Wolf, qemu-devel, qemu-block, Markus Armbruster
[-- Attachment #1.1: Type: text/plain, Size: 7104 bytes --]
On 15.07.19 10:30, Stefano Garzarella wrote:
> On Sun, Jul 14, 2019 at 05:51:51PM +0300, Maxim Levitsky wrote:
>> On Thu, 2019-07-11 at 18:27 +0200, Stefano Garzarella wrote:
>>> On Thu, Jul 11, 2019 at 06:09:40PM +0300, Maxim Levitsky wrote:
>>>> preallocation=off and preallocation=metadata
>>>> both allocate luks header only, and preallocation=falloc/full
>>>> is passed to underlying file.
>>>>
>>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534951
>>>>
>>>> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
>>>>
>>>> ---
>>>>
>>>> Note that QMP support was only compile tested, since I am still learning
>>>> on how to use it.
>>>>
>>>> If there is some library/script/etc which makes it more high level,
>>>> I would more that glad to hear about it. So far I used the qmp-shell
>>>>
>>>> Also can I use qmp's blockdev-create outside a vm running?
>>>>
>>>> block/crypto.c | 29 ++++++++++++++++++++++++++---
>>>> qapi/block-core.json | 5 ++++-
>>>> 2 files changed, 30 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/block/crypto.c b/block/crypto.c
>>>> index 8237424ae6..034a645652 100644
>>>> --- a/block/crypto.c
>>>> +++ b/block/crypto.c
>>>> @@ -74,6 +74,7 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,
>>>> struct BlockCryptoCreateData {
>>>> BlockBackend *blk;
>>>> uint64_t size;
>>>> + PreallocMode prealloc;
>>>> };
>>>>
>>>>
>>>> @@ -112,7 +113,7 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
>>>> * available to the guest, so we must take account of that
>>>> * which will be used by the crypto header
>>>> */
>>>> - return blk_truncate(data->blk, data->size + headerlen, PREALLOC_MODE_OFF,
>>>> + return blk_truncate(data->blk, data->size + headerlen, data->prealloc,
>>>> errp);
>>>> }
>>>>
>>>> @@ -251,6 +252,7 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
>>>> static int block_crypto_co_create_generic(BlockDriverState *bs,
>>>> int64_t size,
>>>> QCryptoBlockCreateOptions *opts,
>>>> + PreallocMode prealloc,
>>>> Error **errp)
>>>> {
>>>> int ret;
>>>> @@ -266,9 +268,14 @@ static int block_crypto_co_create_generic(BlockDriverState *bs,
>>>> goto cleanup;
>>>> }
>>>>
>>>> + if (prealloc == PREALLOC_MODE_METADATA) {
>>>> + prealloc = PREALLOC_MODE_OFF;
>>>> + }
>>>> +
>>>> data = (struct BlockCryptoCreateData) {
>>>> .blk = blk,
>>>> .size = size,
>>>> + .prealloc = prealloc,
>>>> };
>>>>
>>>> crypto = qcrypto_block_create(opts, NULL,
>>>> @@ -500,6 +507,7 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
>>>> BlockdevCreateOptionsLUKS *luks_opts;
>>>> BlockDriverState *bs = NULL;
>>>> QCryptoBlockCreateOptions create_opts;
>>>> + PreallocMode preallocation = PREALLOC_MODE_OFF;
>>>> int ret;
>>>>
>>>> assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
>>>> @@ -515,8 +523,11 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
>>>> .u.luks = *qapi_BlockdevCreateOptionsLUKS_base(luks_opts),
>>>> };
>>>>
>>>> + if (luks_opts->has_preallocation)
>>>> + preallocation = luks_opts->preallocation;
>>>> +
>>>> ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
>>>> - errp);
>>>> + preallocation, errp);
>>>> if (ret < 0) {
>>>> goto fail;
>>>> }
>>>> @@ -534,12 +545,24 @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
>>>> QCryptoBlockCreateOptions *create_opts = NULL;
>>>> BlockDriverState *bs = NULL;
>>>> QDict *cryptoopts;
>>>> + PreallocMode prealloc;
>>>> + char *buf = NULL;
>>>> int64_t size;
>>>> int ret;
>>>> + Error *local_err = NULL;
>>>>
>>>> /* Parse options */
>>>> size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
>>>>
>>>> + buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
>>>> + prealloc = qapi_enum_parse(&PreallocMode_lookup, buf,
>>>> + PREALLOC_MODE_OFF, &local_err);
>>>> + g_free(buf);
>>>> + if (local_err) {
>>>> + error_propagate(errp, local_err);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
>>>> &block_crypto_create_opts_luks,
>>>> true);
>>>> @@ -565,7 +588,7 @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
>>>> }
>>>>
>>>> /* Create format layer */
>>>> - ret = block_crypto_co_create_generic(bs, size, create_opts, errp);
>>>> + ret = block_crypto_co_create_generic(bs, size, create_opts, prealloc, errp);
>>>> if (ret < 0) {
>>>> goto fail;
>>>> }
>>>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>>>> index 0d43d4f37c..ebcfc9f903 100644
>>>> --- a/qapi/block-core.json
>>>> +++ b/qapi/block-core.json
>>>> @@ -4205,13 +4205,16 @@
>>>> #
>>>> # @file Node to create the image format on
>>>> # @size Size of the virtual disk in bytes
>>>> +# @preallocation Preallocation mode for the new image (default: off;
>>>> +# allowed values: off/falloc/full
>>>
>>> Should we add also "metadata" to allowed values? and "since: 4.2"?
>>> I'd like to have (just to have similar documentation with others
>>> preallocation parameters):
>>
>> It it support but preallocation=off is the same as preallocation=metadata in luks,
>> as luks only metadata is the header which is created anyway.
>> In some sense I should throw a error for preallocation=off, but I suspect that
>> that will break userspace api.
>> What do you think?
>
> I don't know very well the details, but I agree with you that some
> user APIs could expect that preallocation=off is always available.
>
> Maybe we can write something in the comment (explaining that off and metadata
> are the same) and make the preallocation=metadata the default choice.
preallocation=off does not mean “Do not allocate any metadata”. For
example, qcow2 still creates the qcow2 header and a minimal refcount
structure. It just means not to allocate any unnecessary metadata.
For LUKS, all metadata is necessary, so preallocation=off and
preallocation=metadata are the same. I don‘t think that means we only
need to support one or the other. I think just making
preallocation=metadata an alias for preallocation=off here is fine – and
preallocation=off should stay the default, because it’s the default for
everything else.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-07-15 8:38 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-11 15:09 [Qemu-devel] [PATCH v3] LUKS: support preallocation Maxim Levitsky
2019-07-11 16:27 ` [Qemu-devel] [Qemu-block] " Stefano Garzarella
2019-07-14 14:51 ` Maxim Levitsky
2019-07-15 8:30 ` Stefano Garzarella
2019-07-15 8:36 ` Maxim Levitsky
2019-07-15 8:38 ` Max Reitz
2019-07-12 14:45 ` [Qemu-devel] " no-reply
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.