* [Qemu-devel] [PATCH v2 0/2] Report format specific info for LUKS block driver
@ 2016-06-14 10:56 Daniel P. Berrange
2016-06-14 10:56 ` [Qemu-devel] [PATCH v2 1/2] crypto: add support for querying parameters for block encryption Daniel P. Berrange
2016-06-14 10:56 ` [Qemu-devel] [PATCH v2 2/2] block: export LUKS specific data to qemu-img info Daniel P. Berrange
0 siblings, 2 replies; 8+ messages in thread
From: Daniel P. Berrange @ 2016-06-14 10:56 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, qemu-block, Kevin Wolf, Max Reitz, Daniel P. Berrange
This is a followup to:
v1: https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg01723.html
The 'qemu-img info' tool has ability to print format specific
information, eg with qcow2 it reports two extra items:
$ qemu-img info ~/VirtualMachines/demo.qcow2
image: /home/berrange/VirtualMachines/demo.qcow2
file format: qcow2
virtual size: 3.0G (3221225472 bytes)
disk size: 140K
cluster_size: 65536
Format specific information:
compat: 0.10
refcount bits: 16
This is not currently wired up for the LUKS driver. This patch
series adds that support so that we can report useful data about
the LUKS volume such as the crypto algorithm choices, key slot
usage and other volume metadata.
The first patch extends the crypto API to allow querying of the
format specific metadata
The second patches extends the block API to allow the LUKS driver
to report the format specific metadata.
$ qemu-img info ~/VirtualMachines/demo.luks
image: /home/berrange/VirtualMachines/demo.luks
file format: luks
virtual size: 98M (102760448 bytes)
disk size: 100M
encrypted: yes
Format specific information:
ivgen alg: plain64
hash alg: sha1
cipher alg: aes-128
uuid: 6ddee74b-3a22-408c-8909-6789d4fa2594
cipher mode: xts
slots:
[0]:
active: true
iters: 572706
key offset: 4096
stripes: 4000
[1]:
active: false
key offset: 135168
[2]:
active: false
key offset: 266240
[3]:
active: false
key offset: 397312
[4]:
active: false
key offset: 528384
[5]:
active: false
key offset: 659456
[6]:
active: false
key offset: 790528
[7]:
active: false
key offset: 921600
payload offset: 2097152
master key iters: 142375
Technically most of the code changes here are in the crypto
layer, rather than the block layer. I'm fine with both patches
going through the block maintainer tree, or can submit a both
patches myself as, for sake of simplicity of merge.
Changed in v2:
- Drop patches related to creating a text output visitor to
format the ImageInfoSpecific data. This will be continued
in a separate patch series
- Fix key offset to be in bytes instead of sectors
- Drop the duplicated ImageInfoSpecificLUKS type and just
directly use QCryptoBlockInfoLUKS type in block layer
- Skip reporting stripes/iters if keyslot is inactive
- Add missing QAPI schema docs
Daniel P. Berrange (2):
crypto: add support for querying parameters for block encryption
block: export LUKS specific data to qemu-img info
block/crypto.c | 59 +++++++++++++++++++++++++++++++++++++++
crypto/block-luks.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
crypto/block.c | 17 +++++++++++
crypto/blockpriv.h | 4 +++
include/crypto/block.h | 16 +++++++++++
qapi/block-core.json | 7 ++++-
qapi/crypto.json | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++
7 files changed, 248 insertions(+), 1 deletion(-)
--
2.5.5
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] crypto: add support for querying parameters for block encryption
2016-06-14 10:56 [Qemu-devel] [PATCH v2 0/2] Report format specific info for LUKS block driver Daniel P. Berrange
@ 2016-06-14 10:56 ` Daniel P. Berrange
2016-06-14 15:30 ` Max Reitz
2016-06-14 10:56 ` [Qemu-devel] [PATCH v2 2/2] block: export LUKS specific data to qemu-img info Daniel P. Berrange
1 sibling, 1 reply; 8+ messages in thread
From: Daniel P. Berrange @ 2016-06-14 10:56 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, qemu-block, Kevin Wolf, Max Reitz, Daniel P. Berrange
When creating new block encryption volumes, we accept a list of
parameters to control the formatting process. It is useful to
be able to query what those parameters were for existing block
devices. Add a qcrypto_block_get_info() method which returns a
QCryptoBlockInfo instance to report this data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
crypto/block-luks.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
crypto/block.c | 17 +++++++++++
crypto/blockpriv.h | 4 +++
include/crypto/block.h | 16 +++++++++++
qapi/crypto.json | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 183 insertions(+)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 63649f1..6a6ef07 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -201,6 +201,15 @@ QEMU_BUILD_BUG_ON(sizeof(struct QCryptoBlockLUKSHeader) != 592);
struct QCryptoBlockLUKS {
QCryptoBlockLUKSHeader header;
+
+ /* Cache parsed versions of what's in header fields,
+ * as we can't rely on QCryptoBlock.cipher being
+ * non-NULL */
+ QCryptoCipherAlgorithm cipher_alg;
+ QCryptoCipherMode cipher_mode;
+ QCryptoIVGenAlgorithm ivgen_alg;
+ QCryptoHashAlgorithm ivgen_hash_alg;
+ QCryptoHashAlgorithm hash_alg;
};
@@ -835,6 +844,12 @@ qcrypto_block_luks_open(QCryptoBlock *block,
block->payload_offset = luks->header.payload_offset *
QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
+ luks->cipher_alg = cipheralg;
+ luks->cipher_mode = ciphermode;
+ luks->ivgen_alg = ivalg;
+ luks->ivgen_hash_alg = ivhash;
+ luks->hash_alg = hash;
+
g_free(masterkey);
g_free(password);
@@ -1250,6 +1265,12 @@ qcrypto_block_luks_create(QCryptoBlock *block,
goto error;
}
+ luks->cipher_alg = luks_opts.cipher_alg;
+ luks->cipher_mode = luks_opts.cipher_mode;
+ luks->ivgen_alg = luks_opts.ivgen_alg;
+ luks->ivgen_hash_alg = luks_opts.ivgen_hash_alg;
+ luks->hash_alg = luks_opts.hash_alg;
+
memset(masterkey, 0, luks->header.key_bytes);
g_free(masterkey);
memset(slotkey, 0, luks->header.key_bytes);
@@ -1284,6 +1305,54 @@ qcrypto_block_luks_create(QCryptoBlock *block,
}
+static int qcrypto_block_luks_get_info(QCryptoBlock *block,
+ QCryptoBlockInfo *info,
+ Error **errp)
+{
+ QCryptoBlockLUKS *luks = block->opaque;
+ QCryptoBlockInfoLUKSSlot *slot;
+ QCryptoBlockInfoLUKSSlotList *slots = NULL, *prev = NULL;
+ size_t i;
+
+ info->u.luks.cipher_alg = luks->cipher_alg;
+ info->u.luks.cipher_mode = luks->cipher_mode;
+ info->u.luks.ivgen_alg = luks->ivgen_alg;
+ if (info->u.luks.ivgen_alg == QCRYPTO_IVGEN_ALG_ESSIV) {
+ info->u.luks.has_ivgen_hash_alg = true;
+ info->u.luks.ivgen_hash_alg = luks->ivgen_hash_alg;
+ }
+ info->u.luks.hash_alg = luks->hash_alg;
+ info->u.luks.payload_offset = block->payload_offset;
+ info->u.luks.master_key_iters = luks->header.master_key_iterations;
+ info->u.luks.uuid = g_strdup((const char *)luks->header.uuid);
+
+ for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+ slots = g_new0(QCryptoBlockInfoLUKSSlotList, 1);
+ if (i == 0) {
+ info->u.luks.slots = slots;
+ } else {
+ prev->next = slots;
+ }
+
+ slots->value = slot = g_new0(QCryptoBlockInfoLUKSSlot, 1);
+ slot->active = luks->header.key_slots[i].active ==
+ QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED;
+ slot->key_offset = luks->header.key_slots[i].key_offset
+ * QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
+ if (slot->active) {
+ slot->has_iters = true;
+ slot->iters = luks->header.key_slots[i].iterations;
+ slot->has_stripes = true;
+ slot->stripes = luks->header.key_slots[i].stripes;
+ }
+
+ prev = slots;
+ }
+
+ return 0;
+}
+
+
static void qcrypto_block_luks_cleanup(QCryptoBlock *block)
{
g_free(block->opaque);
@@ -1321,6 +1390,7 @@ qcrypto_block_luks_encrypt(QCryptoBlock *block,
const QCryptoBlockDriver qcrypto_block_driver_luks = {
.open = qcrypto_block_luks_open,
.create = qcrypto_block_luks_create,
+ .get_info = qcrypto_block_luks_get_info,
.cleanup = qcrypto_block_luks_cleanup,
.decrypt = qcrypto_block_luks_decrypt,
.encrypt = qcrypto_block_luks_encrypt,
diff --git a/crypto/block.c b/crypto/block.c
index da60eba..be823ee 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -105,6 +105,23 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
}
+QCryptoBlockInfo *qcrypto_block_get_info(QCryptoBlock *block,
+ Error **errp)
+{
+ QCryptoBlockInfo *info = g_new0(QCryptoBlockInfo, 1);
+
+ info->format = block->format;
+
+ if (block->driver->get_info &&
+ block->driver->get_info(block, info, errp) < 0) {
+ g_free(info);
+ return NULL;
+ }
+
+ return info;
+}
+
+
int qcrypto_block_decrypt(QCryptoBlock *block,
uint64_t startsector,
uint8_t *buf,
diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
index 6297085..35217cd 100644
--- a/crypto/blockpriv.h
+++ b/crypto/blockpriv.h
@@ -53,6 +53,10 @@ struct QCryptoBlockDriver {
void *opaque,
Error **errp);
+ int (*get_info)(QCryptoBlock *block,
+ QCryptoBlockInfo *info,
+ Error **errp);
+
void (*cleanup)(QCryptoBlock *block);
int (*encrypt)(QCryptoBlock *block,
diff --git a/include/crypto/block.h b/include/crypto/block.h
index a21e11f..6256f64 100644
--- a/include/crypto/block.h
+++ b/include/crypto/block.h
@@ -138,6 +138,22 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
void *opaque,
Error **errp);
+
+/**
+ * qcrypto_block_get_info:
+ * block: the block encryption object
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Get information about the configuration options for the
+ * block encryption object. This includes details such as
+ * the cipher algorithms, modes, and initialization vector
+ * generators.
+ *
+ * Returns: a block encryption info object, or NULL on error
+ */
+QCryptoBlockInfo *qcrypto_block_get_info(QCryptoBlock *block,
+ Error **errp);
+
/**
* @qcrypto_block_decrypt:
* @block: the block encryption object
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 760d0c0..2c7465a 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -220,3 +220,79 @@
'discriminator': 'format',
'data': { 'qcow': 'QCryptoBlockOptionsQCow',
'luks': 'QCryptoBlockCreateOptionsLUKS' } }
+
+
+##
+# QCryptoBlockInfoBase:
+#
+# The common information that applies to all full disk
+# encryption formats
+#
+# @format: the encryption format
+#
+# Since: 2.7
+##
+{ 'struct': 'QCryptoBlockInfoBase',
+ 'data': { 'format': 'QCryptoBlockFormat' }}
+
+
+##
+# QCryptoBlockInfoLUKSSlot:
+#
+# Information about the LUKS block encryption key
+# slot options
+#
+# @active: whether the key slot is currently in use
+# @key-offset: offset to the key material in bytes
+# @iters: #optional number of PBKDF2 iterations for key material
+# @stripes: #optional number of stripes for splitting key material
+#
+# Since: 2.7
+##
+{ 'struct': 'QCryptoBlockInfoLUKSSlot',
+ 'data': {'active': 'bool',
+ '*iters': 'int',
+ '*stripes': 'int',
+ 'key-offset': 'int' } }
+
+
+##
+# QCryptoBlockInfoLUKS:
+#
+# Information about the LUKS block encryption options
+#
+# @cipher-alg: the cipher algorithm for data encryption
+# @cipher-mode: the cipher mode for data encryption
+# @ivgen-alg: the initialization vector generator
+# @ivgen-hash-alg: #optional the initialization vector generator hash
+# @hash-alg: the master key hash algorithm
+# @payload-offset: offset to the payload data in bytes
+# @master-key-iters: number of PBKDF2 iterations for key material
+# @uuid: unique identifier for the volume
+# @slots: information about each key slot
+#
+# Since: 2.7
+##
+{ 'struct': 'QCryptoBlockInfoLUKS',
+ 'data': {'cipher-alg': 'QCryptoCipherAlgorithm',
+ 'cipher-mode': 'QCryptoCipherMode',
+ 'ivgen-alg': 'QCryptoIVGenAlgorithm',
+ '*ivgen-hash-alg': 'QCryptoHashAlgorithm',
+ 'hash-alg': 'QCryptoHashAlgorithm',
+ 'payload-offset': 'int',
+ 'master-key-iters': 'int',
+ 'uuid': 'str',
+ 'slots': [ 'QCryptoBlockInfoLUKSSlot' ] }}
+
+
+##
+# QCryptoBlockInfo:
+#
+# Information about the block encryption options
+#
+# Since: 2.7
+##
+{ 'union': 'QCryptoBlockInfo',
+ 'base': 'QCryptoBlockInfoBase',
+ 'discriminator': 'format',
+ 'data': { 'luks': 'QCryptoBlockInfoLUKS' } }
--
2.5.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] block: export LUKS specific data to qemu-img info
2016-06-14 10:56 [Qemu-devel] [PATCH v2 0/2] Report format specific info for LUKS block driver Daniel P. Berrange
2016-06-14 10:56 ` [Qemu-devel] [PATCH v2 1/2] crypto: add support for querying parameters for block encryption Daniel P. Berrange
@ 2016-06-14 10:56 ` Daniel P. Berrange
2016-06-14 15:38 ` Max Reitz
1 sibling, 1 reply; 8+ messages in thread
From: Daniel P. Berrange @ 2016-06-14 10:56 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, qemu-block, Kevin Wolf, Max Reitz, Daniel P. Berrange
The qemu-img info command has the ability to expose format
specific metadata about volumes. Wire up this facility for
the LUKS driver to report on cipher configuration and key
slot usage.
$ qemu-img info ~/VirtualMachines/demo.luks
image: /home/berrange/VirtualMachines/demo.luks
file format: luks
virtual size: 98M (102760448 bytes)
disk size: 100M
encrypted: yes
Format specific information:
ivgen alg: plain64
hash alg: sha1
cipher alg: aes-128
uuid: 6ddee74b-3a22-408c-8909-6789d4fa2594
cipher mode: xts
slots:
[0]:
active: true
iters: 572706
key offset: 4096
stripes: 4000
[1]:
active: false
key offset: 135168
[2]:
active: false
key offset: 266240
[3]:
active: false
key offset: 397312
[4]:
active: false
key offset: 528384
[5]:
active: false
key offset: 659456
[6]:
active: false
key offset: 790528
[7]:
active: false
key offset: 921600
payload offset: 2097152
master key iters: 142375
One somewhat undesirable artifact is that the data fields are
printed out in (apparently) random order. This will be addressed
later by changing the way the block layer pretty-prints the
image specific data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
block/crypto.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
qapi/block-core.json | 7 ++++++-
2 files changed, 65 insertions(+), 1 deletion(-)
diff --git a/block/crypto.c b/block/crypto.c
index 758e14e..823572f 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -565,6 +565,63 @@ static int block_crypto_create_luks(const char *filename,
filename, opts, errp);
}
+static int block_crypto_get_info_luks(BlockDriverState *bs,
+ BlockDriverInfo *bdi)
+{
+ BlockDriverInfo subbdi;
+ int ret;
+
+ ret = bdrv_get_info(bs->file->bs, &subbdi);
+ if (ret != 0) {
+ return ret;
+ }
+
+ bdi->unallocated_blocks_are_zero = false;
+ bdi->can_write_zeroes_with_unmap = false;
+ bdi->cluster_size = subbdi.cluster_size;
+
+ return 0;
+}
+
+static ImageInfoSpecific *
+block_crypto_get_specific_info_luks(BlockDriverState *bs)
+{
+ BlockCrypto *crypto = bs->opaque;
+ ImageInfoSpecific *spec_info;
+ QCryptoBlockInfo *info;
+
+ info = qcrypto_block_get_info(crypto->block, NULL);
+ if (!info) {
+ return NULL;
+ }
+ if (info->format != Q_CRYPTO_BLOCK_FORMAT_LUKS) {
+ qapi_free_QCryptoBlockInfo(info);
+ return NULL;
+ }
+
+ spec_info = g_new(ImageInfoSpecific, 1);
+ spec_info->type = IMAGE_INFO_SPECIFIC_KIND_LUKS;
+ spec_info->u.luks.data = g_new(QCryptoBlockInfoLUKS, 1);
+ spec_info->u.luks.data->cipher_alg = info->u.luks.cipher_alg;
+ spec_info->u.luks.data->cipher_mode = info->u.luks.cipher_mode;
+ spec_info->u.luks.data->ivgen_alg = info->u.luks.ivgen_alg;
+ spec_info->u.luks.data->has_ivgen_hash_alg =
+ info->u.luks.has_ivgen_hash_alg;
+ spec_info->u.luks.data->ivgen_hash_alg = info->u.luks.ivgen_hash_alg;
+ spec_info->u.luks.data->hash_alg = info->u.luks.hash_alg;
+ spec_info->u.luks.data->payload_offset = info->u.luks.payload_offset;
+ spec_info->u.luks.data->master_key_iters = info->u.luks.master_key_iters;
+ spec_info->u.luks.data->uuid = g_strdup(info->u.luks.uuid);
+
+ /* Steal the pointer instead of bothering to copy */
+ spec_info->u.luks.data->slots = info->u.luks.slots;
+ info->u.luks.slots = NULL;
+
+ qapi_free_QCryptoBlockInfo(info);
+
+ return spec_info;
+}
+
BlockDriver bdrv_crypto_luks = {
.format_name = "luks",
.instance_size = sizeof(BlockCrypto),
@@ -578,6 +635,8 @@ BlockDriver bdrv_crypto_luks = {
.bdrv_co_readv = block_crypto_co_readv,
.bdrv_co_writev = block_crypto_co_writev,
.bdrv_getlength = block_crypto_getlength,
+ .bdrv_get_info = block_crypto_get_info_luks,
+ .bdrv_get_specific_info = block_crypto_get_specific_info_luks,
};
static void block_crypto_init(void)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 98a20d2..959dd8b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -74,6 +74,7 @@
'extents': ['ImageInfo']
} }
+
##
# @ImageInfoSpecific:
#
@@ -85,7 +86,11 @@
{ 'union': 'ImageInfoSpecific',
'data': {
'qcow2': 'ImageInfoSpecificQCow2',
- 'vmdk': 'ImageInfoSpecificVmdk'
+ 'vmdk': 'ImageInfoSpecificVmdk',
+ # If we need to add block driver specific parameters for
+ # LUKS in future, then we'll subclass QCryptoBlockInfoLUKS
+ # to define a ImageInfoSpecificLUKS
+ 'luks': 'QCryptoBlockInfoLUKS'
} }
##
--
2.5.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] crypto: add support for querying parameters for block encryption
2016-06-14 10:56 ` [Qemu-devel] [PATCH v2 1/2] crypto: add support for querying parameters for block encryption Daniel P. Berrange
@ 2016-06-14 15:30 ` Max Reitz
0 siblings, 0 replies; 8+ messages in thread
From: Max Reitz @ 2016-06-14 15:30 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel; +Cc: Eric Blake, qemu-block, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 4354 bytes --]
On 14.06.2016 12:56, Daniel P. Berrange wrote:
> When creating new block encryption volumes, we accept a list of
> parameters to control the formatting process. It is useful to
> be able to query what those parameters were for existing block
> devices. Add a qcrypto_block_get_info() method which returns a
> QCryptoBlockInfo instance to report this data.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> crypto/block-luks.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
> crypto/block.c | 17 +++++++++++
> crypto/blockpriv.h | 4 +++
> include/crypto/block.h | 16 +++++++++++
> qapi/crypto.json | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 183 insertions(+)
>
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index 63649f1..6a6ef07 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
[...]
> @@ -1284,6 +1305,54 @@ qcrypto_block_luks_create(QCryptoBlock *block,
> }
>
>
> +static int qcrypto_block_luks_get_info(QCryptoBlock *block,
> + QCryptoBlockInfo *info,
> + Error **errp)
> +{
> + QCryptoBlockLUKS *luks = block->opaque;
> + QCryptoBlockInfoLUKSSlot *slot;
> + QCryptoBlockInfoLUKSSlotList *slots = NULL, *prev = NULL;
You could make prev a double pointer, initializing it to
&info->u.luks.slots... [1]
> + size_t i;
> +
> + info->u.luks.cipher_alg = luks->cipher_alg;
> + info->u.luks.cipher_mode = luks->cipher_mode;
> + info->u.luks.ivgen_alg = luks->ivgen_alg;
> + if (info->u.luks.ivgen_alg == QCRYPTO_IVGEN_ALG_ESSIV) {
> + info->u.luks.has_ivgen_hash_alg = true;
> + info->u.luks.ivgen_hash_alg = luks->ivgen_hash_alg;
> + }
> + info->u.luks.hash_alg = luks->hash_alg;
> + info->u.luks.payload_offset = block->payload_offset;
> + info->u.luks.master_key_iters = luks->header.master_key_iterations;
> + info->u.luks.uuid = g_strdup((const char *)luks->header.uuid);
Wouldn't g_strndup() with sizeof(luks->header.uuid) be a better choice?
> +
> + for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
> + slots = g_new0(QCryptoBlockInfoLUKSSlotList, 1);
> + if (i == 0) {
> + info->u.luks.slots = slots;
> + } else {
> + prev->next = slots;
> + }
[1] ...then unconditionally use *prev = slots here...
> +
> + slots->value = slot = g_new0(QCryptoBlockInfoLUKSSlot, 1);
> + slot->active = luks->header.key_slots[i].active ==
> + QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED;
> + slot->key_offset = luks->header.key_slots[i].key_offset
> + * QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
> + if (slot->active) {
> + slot->has_iters = true;
> + slot->iters = luks->header.key_slots[i].iterations;
> + slot->has_stripes = true;
> + slot->stripes = luks->header.key_slots[i].stripes;
> + }
> +
> + prev = slots;
[1] ...and prev = &slots->next here.
> + }
> +
> + return 0;
> +}
> +
> +
> static void qcrypto_block_luks_cleanup(QCryptoBlock *block)
> {
> g_free(block->opaque);
[...]
> diff --git a/include/crypto/block.h b/include/crypto/block.h
> index a21e11f..6256f64 100644
> --- a/include/crypto/block.h
> +++ b/include/crypto/block.h
> @@ -138,6 +138,22 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
> void *opaque,
> Error **errp);
>
> +
> +/**
> + * qcrypto_block_get_info:
> + * block: the block encryption object
I think this is missing an @.
Max
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Get information about the configuration options for the
> + * block encryption object. This includes details such as
> + * the cipher algorithms, modes, and initialization vector
> + * generators.
> + *
> + * Returns: a block encryption info object, or NULL on error
> + */
> +QCryptoBlockInfo *qcrypto_block_get_info(QCryptoBlock *block,
> + Error **errp);
> +
> /**
> * @qcrypto_block_decrypt:
> * @block: the block encryption object
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] block: export LUKS specific data to qemu-img info
2016-06-14 10:56 ` [Qemu-devel] [PATCH v2 2/2] block: export LUKS specific data to qemu-img info Daniel P. Berrange
@ 2016-06-14 15:38 ` Max Reitz
2016-06-14 15:47 ` Daniel P. Berrange
0 siblings, 1 reply; 8+ messages in thread
From: Max Reitz @ 2016-06-14 15:38 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel; +Cc: Eric Blake, qemu-block, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 5906 bytes --]
On 14.06.2016 12:56, Daniel P. Berrange wrote:
> The qemu-img info command has the ability to expose format
> specific metadata about volumes. Wire up this facility for
> the LUKS driver to report on cipher configuration and key
> slot usage.
>
> $ qemu-img info ~/VirtualMachines/demo.luks
> image: /home/berrange/VirtualMachines/demo.luks
> file format: luks
> virtual size: 98M (102760448 bytes)
> disk size: 100M
> encrypted: yes
> Format specific information:
> ivgen alg: plain64
> hash alg: sha1
> cipher alg: aes-128
> uuid: 6ddee74b-3a22-408c-8909-6789d4fa2594
> cipher mode: xts
> slots:
> [0]:
> active: true
> iters: 572706
> key offset: 4096
> stripes: 4000
> [1]:
> active: false
> key offset: 135168
> [2]:
> active: false
> key offset: 266240
> [3]:
> active: false
> key offset: 397312
> [4]:
> active: false
> key offset: 528384
> [5]:
> active: false
> key offset: 659456
> [6]:
> active: false
> key offset: 790528
> [7]:
> active: false
> key offset: 921600
> payload offset: 2097152
> master key iters: 142375
>
> One somewhat undesirable artifact is that the data fields are
> printed out in (apparently) random order. This will be addressed
> later by changing the way the block layer pretty-prints the
> image specific data.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> block/crypto.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> qapi/block-core.json | 7 ++++++-
> 2 files changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/block/crypto.c b/block/crypto.c
> index 758e14e..823572f 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -565,6 +565,63 @@ static int block_crypto_create_luks(const char *filename,
> filename, opts, errp);
> }
>
> +static int block_crypto_get_info_luks(BlockDriverState *bs,
> + BlockDriverInfo *bdi)
> +{
> + BlockDriverInfo subbdi;
> + int ret;
> +
> + ret = bdrv_get_info(bs->file->bs, &subbdi);
> + if (ret != 0) {
> + return ret;
> + }
> +
> + bdi->unallocated_blocks_are_zero = false;
> + bdi->can_write_zeroes_with_unmap = false;
> + bdi->cluster_size = subbdi.cluster_size;
> +
> + return 0;
> +}
> +
> +static ImageInfoSpecific *
> +block_crypto_get_specific_info_luks(BlockDriverState *bs)
> +{
> + BlockCrypto *crypto = bs->opaque;
> + ImageInfoSpecific *spec_info;
> + QCryptoBlockInfo *info;
> +
> + info = qcrypto_block_get_info(crypto->block, NULL);
> + if (!info) {
> + return NULL;
> + }
> + if (info->format != Q_CRYPTO_BLOCK_FORMAT_LUKS) {
> + qapi_free_QCryptoBlockInfo(info);
> + return NULL;
> + }
> +
> + spec_info = g_new(ImageInfoSpecific, 1);
> + spec_info->type = IMAGE_INFO_SPECIFIC_KIND_LUKS;
One space too many.
> + spec_info->u.luks.data = g_new(QCryptoBlockInfoLUKS, 1);
> + spec_info->u.luks.data->cipher_alg = info->u.luks.cipher_alg;
> + spec_info->u.luks.data->cipher_mode = info->u.luks.cipher_mode;
> + spec_info->u.luks.data->ivgen_alg = info->u.luks.ivgen_alg;
> + spec_info->u.luks.data->has_ivgen_hash_alg =
> + info->u.luks.has_ivgen_hash_alg;
> + spec_info->u.luks.data->ivgen_hash_alg = info->u.luks.ivgen_hash_alg;
> + spec_info->u.luks.data->hash_alg = info->u.luks.hash_alg;
> + spec_info->u.luks.data->payload_offset = info->u.luks.payload_offset;
> + spec_info->u.luks.data->master_key_iters = info->u.luks.master_key_iters;
> + spec_info->u.luks.data->uuid = g_strdup(info->u.luks.uuid);
> +
> + /* Steal the pointer instead of bothering to copy */
> + spec_info->u.luks.data->slots = info->u.luks.slots;
> + info->u.luks.slots = NULL;
Why not just steal everything by *spec_info->u.luks.data = info->u.luks
and then making sure the qapi_free() call below won't free anything in
there with a memset(&info->u.luks, 0, sizeof(info->u.luks))?
> +
> + qapi_free_QCryptoBlockInfo(info);
> +
> + return spec_info;
> +}
> +
> BlockDriver bdrv_crypto_luks = {
> .format_name = "luks",
> .instance_size = sizeof(BlockCrypto),
> @@ -578,6 +635,8 @@ BlockDriver bdrv_crypto_luks = {
> .bdrv_co_readv = block_crypto_co_readv,
> .bdrv_co_writev = block_crypto_co_writev,
> .bdrv_getlength = block_crypto_getlength,
> + .bdrv_get_info = block_crypto_get_info_luks,
> + .bdrv_get_specific_info = block_crypto_get_specific_info_luks,
> };
>
> static void block_crypto_init(void)
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 98a20d2..959dd8b 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -74,6 +74,7 @@
> 'extents': ['ImageInfo']
> } }
>
> +
> ##
> # @ImageInfoSpecific:
> #
This hunk looks unnecessary.
Max
> @@ -85,7 +86,11 @@
> { 'union': 'ImageInfoSpecific',
> 'data': {
> 'qcow2': 'ImageInfoSpecificQCow2',
> - 'vmdk': 'ImageInfoSpecificVmdk'
> + 'vmdk': 'ImageInfoSpecificVmdk',
> + # If we need to add block driver specific parameters for
> + # LUKS in future, then we'll subclass QCryptoBlockInfoLUKS
> + # to define a ImageInfoSpecificLUKS
> + 'luks': 'QCryptoBlockInfoLUKS'
> } }
>
> ##
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] block: export LUKS specific data to qemu-img info
2016-06-14 15:38 ` Max Reitz
@ 2016-06-14 15:47 ` Daniel P. Berrange
2016-06-14 15:49 ` Max Reitz
0 siblings, 1 reply; 8+ messages in thread
From: Daniel P. Berrange @ 2016-06-14 15:47 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, Eric Blake, qemu-block, Kevin Wolf
On Tue, Jun 14, 2016 at 05:38:36PM +0200, Max Reitz wrote:
> On 14.06.2016 12:56, Daniel P. Berrange wrote:
> > The qemu-img info command has the ability to expose format
> > specific metadata about volumes. Wire up this facility for
> > the LUKS driver to report on cipher configuration and key
> > slot usage.
> >
> > $ qemu-img info ~/VirtualMachines/demo.luks
> > image: /home/berrange/VirtualMachines/demo.luks
> > file format: luks
> > virtual size: 98M (102760448 bytes)
> > disk size: 100M
> > encrypted: yes
> > Format specific information:
> > ivgen alg: plain64
> > hash alg: sha1
> > cipher alg: aes-128
> > uuid: 6ddee74b-3a22-408c-8909-6789d4fa2594
> > cipher mode: xts
> > slots:
> > [0]:
> > active: true
> > iters: 572706
> > key offset: 4096
> > stripes: 4000
> > [1]:
> > active: false
> > key offset: 135168
> > [2]:
> > active: false
> > key offset: 266240
> > [3]:
> > active: false
> > key offset: 397312
> > [4]:
> > active: false
> > key offset: 528384
> > [5]:
> > active: false
> > key offset: 659456
> > [6]:
> > active: false
> > key offset: 790528
> > [7]:
> > active: false
> > key offset: 921600
> > payload offset: 2097152
> > master key iters: 142375
> >
> > One somewhat undesirable artifact is that the data fields are
> > printed out in (apparently) random order. This will be addressed
> > later by changing the way the block layer pretty-prints the
> > image specific data.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > block/crypto.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > qapi/block-core.json | 7 ++++++-
> > 2 files changed, 65 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/crypto.c b/block/crypto.c
> > index 758e14e..823572f 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -565,6 +565,63 @@ static int block_crypto_create_luks(const char *filename,
> > filename, opts, errp);
> > }
> >
> > +static int block_crypto_get_info_luks(BlockDriverState *bs,
> > + BlockDriverInfo *bdi)
> > +{
> > + BlockDriverInfo subbdi;
> > + int ret;
> > +
> > + ret = bdrv_get_info(bs->file->bs, &subbdi);
> > + if (ret != 0) {
> > + return ret;
> > + }
> > +
> > + bdi->unallocated_blocks_are_zero = false;
> > + bdi->can_write_zeroes_with_unmap = false;
> > + bdi->cluster_size = subbdi.cluster_size;
> > +
> > + return 0;
> > +}
> > +
> > +static ImageInfoSpecific *
> > +block_crypto_get_specific_info_luks(BlockDriverState *bs)
> > +{
> > + BlockCrypto *crypto = bs->opaque;
> > + ImageInfoSpecific *spec_info;
> > + QCryptoBlockInfo *info;
> > +
> > + info = qcrypto_block_get_info(crypto->block, NULL);
> > + if (!info) {
> > + return NULL;
> > + }
> > + if (info->format != Q_CRYPTO_BLOCK_FORMAT_LUKS) {
> > + qapi_free_QCryptoBlockInfo(info);
> > + return NULL;
> > + }
> > +
> > + spec_info = g_new(ImageInfoSpecific, 1);
> > + spec_info->type = IMAGE_INFO_SPECIFIC_KIND_LUKS;
>
> One space too many.
>
> > + spec_info->u.luks.data = g_new(QCryptoBlockInfoLUKS, 1);
> > + spec_info->u.luks.data->cipher_alg = info->u.luks.cipher_alg;
> > + spec_info->u.luks.data->cipher_mode = info->u.luks.cipher_mode;
> > + spec_info->u.luks.data->ivgen_alg = info->u.luks.ivgen_alg;
> > + spec_info->u.luks.data->has_ivgen_hash_alg =
> > + info->u.luks.has_ivgen_hash_alg;
> > + spec_info->u.luks.data->ivgen_hash_alg = info->u.luks.ivgen_hash_alg;
> > + spec_info->u.luks.data->hash_alg = info->u.luks.hash_alg;
> > + spec_info->u.luks.data->payload_offset = info->u.luks.payload_offset;
> > + spec_info->u.luks.data->master_key_iters = info->u.luks.master_key_iters;
> > + spec_info->u.luks.data->uuid = g_strdup(info->u.luks.uuid);
> > +
> > + /* Steal the pointer instead of bothering to copy */
> > + spec_info->u.luks.data->slots = info->u.luks.slots;
> > + info->u.luks.slots = NULL;
>
> Why not just steal everything by *spec_info->u.luks.data = info->u.luks
> and then making sure the qapi_free() call below won't free anything in
> there with a memset(&info->u.luks, 0, sizeof(info->u.luks))?
I wish we could, but info->u.luks is an embedded QCryptoBlockInfoLUKS,
not merely a pointer to an independently allocated QCryptoBlockInfoLUKS :-(
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] block: export LUKS specific data to qemu-img info
2016-06-14 15:47 ` Daniel P. Berrange
@ 2016-06-14 15:49 ` Max Reitz
2016-06-14 15:53 ` Daniel P. Berrange
0 siblings, 1 reply; 8+ messages in thread
From: Max Reitz @ 2016-06-14 15:49 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: qemu-devel, Eric Blake, qemu-block, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 5232 bytes --]
On 14.06.2016 17:47, Daniel P. Berrange wrote:
> On Tue, Jun 14, 2016 at 05:38:36PM +0200, Max Reitz wrote:
>> On 14.06.2016 12:56, Daniel P. Berrange wrote:
>>> The qemu-img info command has the ability to expose format
>>> specific metadata about volumes. Wire up this facility for
>>> the LUKS driver to report on cipher configuration and key
>>> slot usage.
>>>
>>> $ qemu-img info ~/VirtualMachines/demo.luks
>>> image: /home/berrange/VirtualMachines/demo.luks
>>> file format: luks
>>> virtual size: 98M (102760448 bytes)
>>> disk size: 100M
>>> encrypted: yes
>>> Format specific information:
>>> ivgen alg: plain64
>>> hash alg: sha1
>>> cipher alg: aes-128
>>> uuid: 6ddee74b-3a22-408c-8909-6789d4fa2594
>>> cipher mode: xts
>>> slots:
>>> [0]:
>>> active: true
>>> iters: 572706
>>> key offset: 4096
>>> stripes: 4000
>>> [1]:
>>> active: false
>>> key offset: 135168
>>> [2]:
>>> active: false
>>> key offset: 266240
>>> [3]:
>>> active: false
>>> key offset: 397312
>>> [4]:
>>> active: false
>>> key offset: 528384
>>> [5]:
>>> active: false
>>> key offset: 659456
>>> [6]:
>>> active: false
>>> key offset: 790528
>>> [7]:
>>> active: false
>>> key offset: 921600
>>> payload offset: 2097152
>>> master key iters: 142375
>>>
>>> One somewhat undesirable artifact is that the data fields are
>>> printed out in (apparently) random order. This will be addressed
>>> later by changing the way the block layer pretty-prints the
>>> image specific data.
>>>
>>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>>> ---
>>> block/crypto.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> qapi/block-core.json | 7 ++++++-
>>> 2 files changed, 65 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/block/crypto.c b/block/crypto.c
>>> index 758e14e..823572f 100644
>>> --- a/block/crypto.c
>>> +++ b/block/crypto.c
>>> @@ -565,6 +565,63 @@ static int block_crypto_create_luks(const char *filename,
>>> filename, opts, errp);
>>> }
>>>
>>> +static int block_crypto_get_info_luks(BlockDriverState *bs,
>>> + BlockDriverInfo *bdi)
>>> +{
>>> + BlockDriverInfo subbdi;
>>> + int ret;
>>> +
>>> + ret = bdrv_get_info(bs->file->bs, &subbdi);
>>> + if (ret != 0) {
>>> + return ret;
>>> + }
>>> +
>>> + bdi->unallocated_blocks_are_zero = false;
>>> + bdi->can_write_zeroes_with_unmap = false;
>>> + bdi->cluster_size = subbdi.cluster_size;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static ImageInfoSpecific *
>>> +block_crypto_get_specific_info_luks(BlockDriverState *bs)
>>> +{
>>> + BlockCrypto *crypto = bs->opaque;
>>> + ImageInfoSpecific *spec_info;
>>> + QCryptoBlockInfo *info;
>>> +
>>> + info = qcrypto_block_get_info(crypto->block, NULL);
>>> + if (!info) {
>>> + return NULL;
>>> + }
>>> + if (info->format != Q_CRYPTO_BLOCK_FORMAT_LUKS) {
>>> + qapi_free_QCryptoBlockInfo(info);
>>> + return NULL;
>>> + }
>>> +
>>> + spec_info = g_new(ImageInfoSpecific, 1);
>>> + spec_info->type = IMAGE_INFO_SPECIFIC_KIND_LUKS;
>>
>> One space too many.
>>
>>> + spec_info->u.luks.data = g_new(QCryptoBlockInfoLUKS, 1);
>>> + spec_info->u.luks.data->cipher_alg = info->u.luks.cipher_alg;
>>> + spec_info->u.luks.data->cipher_mode = info->u.luks.cipher_mode;
>>> + spec_info->u.luks.data->ivgen_alg = info->u.luks.ivgen_alg;
>>> + spec_info->u.luks.data->has_ivgen_hash_alg =
>>> + info->u.luks.has_ivgen_hash_alg;
>>> + spec_info->u.luks.data->ivgen_hash_alg = info->u.luks.ivgen_hash_alg;
>>> + spec_info->u.luks.data->hash_alg = info->u.luks.hash_alg;
>>> + spec_info->u.luks.data->payload_offset = info->u.luks.payload_offset;
>>> + spec_info->u.luks.data->master_key_iters = info->u.luks.master_key_iters;
>>> + spec_info->u.luks.data->uuid = g_strdup(info->u.luks.uuid);
>>> +
>>> + /* Steal the pointer instead of bothering to copy */
>>> + spec_info->u.luks.data->slots = info->u.luks.slots;
>>> + info->u.luks.slots = NULL;
>>
>> Why not just steal everything by *spec_info->u.luks.data = info->u.luks
>> and then making sure the qapi_free() call below won't free anything in
>> there with a memset(&info->u.luks, 0, sizeof(info->u.luks))?
>
> I wish we could, but info->u.luks is an embedded QCryptoBlockInfoLUKS,
> not merely a pointer to an independently allocated QCryptoBlockInfoLUKS :-(
Yes, but note the asterisk I put there. Leave the g_new() and just make it:
spec_info->u.luks.data = g_new(QCryptoBlockInfoLUKS, 1);
*spec_info->u.luks.data = info->u.luks;
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] block: export LUKS specific data to qemu-img info
2016-06-14 15:49 ` Max Reitz
@ 2016-06-14 15:53 ` Daniel P. Berrange
0 siblings, 0 replies; 8+ messages in thread
From: Daniel P. Berrange @ 2016-06-14 15:53 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, Eric Blake, qemu-block, Kevin Wolf
On Tue, Jun 14, 2016 at 05:49:24PM +0200, Max Reitz wrote:
> On 14.06.2016 17:47, Daniel P. Berrange wrote:
> > On Tue, Jun 14, 2016 at 05:38:36PM +0200, Max Reitz wrote:
> >> On 14.06.2016 12:56, Daniel P. Berrange wrote:
> >>> The qemu-img info command has the ability to expose format
> >>> specific metadata about volumes. Wire up this facility for
> >>> the LUKS driver to report on cipher configuration and key
> >>> slot usage.
> >>>
> >>> $ qemu-img info ~/VirtualMachines/demo.luks
> >>> image: /home/berrange/VirtualMachines/demo.luks
> >>> file format: luks
> >>> virtual size: 98M (102760448 bytes)
> >>> disk size: 100M
> >>> encrypted: yes
> >>> Format specific information:
> >>> ivgen alg: plain64
> >>> hash alg: sha1
> >>> cipher alg: aes-128
> >>> uuid: 6ddee74b-3a22-408c-8909-6789d4fa2594
> >>> cipher mode: xts
> >>> slots:
> >>> [0]:
> >>> active: true
> >>> iters: 572706
> >>> key offset: 4096
> >>> stripes: 4000
> >>> [1]:
> >>> active: false
> >>> key offset: 135168
> >>> [2]:
> >>> active: false
> >>> key offset: 266240
> >>> [3]:
> >>> active: false
> >>> key offset: 397312
> >>> [4]:
> >>> active: false
> >>> key offset: 528384
> >>> [5]:
> >>> active: false
> >>> key offset: 659456
> >>> [6]:
> >>> active: false
> >>> key offset: 790528
> >>> [7]:
> >>> active: false
> >>> key offset: 921600
> >>> payload offset: 2097152
> >>> master key iters: 142375
> >>>
> >>> One somewhat undesirable artifact is that the data fields are
> >>> printed out in (apparently) random order. This will be addressed
> >>> later by changing the way the block layer pretty-prints the
> >>> image specific data.
> >>>
> >>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> >>> ---
> >>> block/crypto.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>> qapi/block-core.json | 7 ++++++-
> >>> 2 files changed, 65 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/block/crypto.c b/block/crypto.c
> >>> index 758e14e..823572f 100644
> >>> --- a/block/crypto.c
> >>> +++ b/block/crypto.c
> >>> @@ -565,6 +565,63 @@ static int block_crypto_create_luks(const char *filename,
> >>> filename, opts, errp);
> >>> }
> >>>
> >>> +static int block_crypto_get_info_luks(BlockDriverState *bs,
> >>> + BlockDriverInfo *bdi)
> >>> +{
> >>> + BlockDriverInfo subbdi;
> >>> + int ret;
> >>> +
> >>> + ret = bdrv_get_info(bs->file->bs, &subbdi);
> >>> + if (ret != 0) {
> >>> + return ret;
> >>> + }
> >>> +
> >>> + bdi->unallocated_blocks_are_zero = false;
> >>> + bdi->can_write_zeroes_with_unmap = false;
> >>> + bdi->cluster_size = subbdi.cluster_size;
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static ImageInfoSpecific *
> >>> +block_crypto_get_specific_info_luks(BlockDriverState *bs)
> >>> +{
> >>> + BlockCrypto *crypto = bs->opaque;
> >>> + ImageInfoSpecific *spec_info;
> >>> + QCryptoBlockInfo *info;
> >>> +
> >>> + info = qcrypto_block_get_info(crypto->block, NULL);
> >>> + if (!info) {
> >>> + return NULL;
> >>> + }
> >>> + if (info->format != Q_CRYPTO_BLOCK_FORMAT_LUKS) {
> >>> + qapi_free_QCryptoBlockInfo(info);
> >>> + return NULL;
> >>> + }
> >>> +
> >>> + spec_info = g_new(ImageInfoSpecific, 1);
> >>> + spec_info->type = IMAGE_INFO_SPECIFIC_KIND_LUKS;
> >>
> >> One space too many.
> >>
> >>> + spec_info->u.luks.data = g_new(QCryptoBlockInfoLUKS, 1);
> >>> + spec_info->u.luks.data->cipher_alg = info->u.luks.cipher_alg;
> >>> + spec_info->u.luks.data->cipher_mode = info->u.luks.cipher_mode;
> >>> + spec_info->u.luks.data->ivgen_alg = info->u.luks.ivgen_alg;
> >>> + spec_info->u.luks.data->has_ivgen_hash_alg =
> >>> + info->u.luks.has_ivgen_hash_alg;
> >>> + spec_info->u.luks.data->ivgen_hash_alg = info->u.luks.ivgen_hash_alg;
> >>> + spec_info->u.luks.data->hash_alg = info->u.luks.hash_alg;
> >>> + spec_info->u.luks.data->payload_offset = info->u.luks.payload_offset;
> >>> + spec_info->u.luks.data->master_key_iters = info->u.luks.master_key_iters;
> >>> + spec_info->u.luks.data->uuid = g_strdup(info->u.luks.uuid);
> >>> +
> >>> + /* Steal the pointer instead of bothering to copy */
> >>> + spec_info->u.luks.data->slots = info->u.luks.slots;
> >>> + info->u.luks.slots = NULL;
> >>
> >> Why not just steal everything by *spec_info->u.luks.data = info->u.luks
> >> and then making sure the qapi_free() call below won't free anything in
> >> there with a memset(&info->u.luks, 0, sizeof(info->u.luks))?
> >
> > I wish we could, but info->u.luks is an embedded QCryptoBlockInfoLUKS,
> > not merely a pointer to an independently allocated QCryptoBlockInfoLUKS :-(
>
> Yes, but note the asterisk I put there. Leave the g_new() and just make it:
>
> spec_info->u.luks.data = g_new(QCryptoBlockInfoLUKS, 1);
> *spec_info->u.luks.data = info->u.luks;
Oh whoops, I mis-read your original reply. I see what you mean now.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-06-14 15:53 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-14 10:56 [Qemu-devel] [PATCH v2 0/2] Report format specific info for LUKS block driver Daniel P. Berrange
2016-06-14 10:56 ` [Qemu-devel] [PATCH v2 1/2] crypto: add support for querying parameters for block encryption Daniel P. Berrange
2016-06-14 15:30 ` Max Reitz
2016-06-14 10:56 ` [Qemu-devel] [PATCH v2 2/2] block: export LUKS specific data to qemu-img info Daniel P. Berrange
2016-06-14 15:38 ` Max Reitz
2016-06-14 15:47 ` Daniel P. Berrange
2016-06-14 15:49 ` Max Reitz
2016-06-14 15:53 ` Daniel P. Berrange
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).