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 <berra...@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
signature.asc
Description: OpenPGP digital signature