From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48952) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCqeo-0000qZ-Jq for qemu-devel@nongnu.org; Tue, 14 Jun 2016 11:53:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bCqem-0001x5-4X for qemu-devel@nongnu.org; Tue, 14 Jun 2016 11:53:37 -0400 Date: Tue, 14 Jun 2016 16:53:24 +0100 From: "Daniel P. Berrange" Message-ID: <20160614155324.GW4310@redhat.com> Reply-To: "Daniel P. Berrange" References: <1465901813-510-1-git-send-email-berrange@redhat.com> <1465901813-510-3-git-send-email-berrange@redhat.com> <05bbe389-cb31-6084-7c94-662353e367d2@redhat.com> <20160614154714.GV4310@redhat.com> <6582240c-4703-d831-196b-35393d168d0f@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <6582240c-4703-d831-196b-35393d168d0f@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 2/2] block: export LUKS specific data to qemu-img info List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-devel@nongnu.org, Eric Blake , qemu-block@nongnu.org, 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 > >>> --- > >>> 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 :|