[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/2] block: export LUKS specific data to qemu
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/2] block: export LUKS specific data to qemu-img info |
Date: |
Tue, 14 Jun 2016 17:49:24 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
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 <address@hidden>
>>> ---
>>> 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