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 :-(
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 :|
