On Thu, May 25, 2017 at 02:52:30PM -0500, Eric Blake wrote: > On 05/25/2017 11:38 AM, Daniel P. Berrange wrote: > > Currently 'qemu-img info' reports a simple "encrypted: yes" > > field. This is not very useful now that qcow2 can support > > multiple encryption formats. Users want to know which format > > is in use and some data related to it. > > > > Wire up usage of the qcrypto_block_get_info() method so that > > 'qemu-img info' can report about the encryption format > > and parameters in use > > > > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > > --- > > block/qcow2.c | 35 ++++++++++++++++++++++++++++++++++- > > qapi/block-core.json | 24 +++++++++++++++++++++++- > > 2 files changed, 57 insertions(+), 2 deletions(-) > > > > diff --git a/block/qcow2.c b/block/qcow2.c > > index 38f9eb5..cb321a2 100644 > > --- a/block/qcow2.c > > +++ b/block/qcow2.c > > @@ -3196,8 +3196,17 @@ static int qcow2_get_info(BlockDriverState *bs, > > BlockDriverInfo *bdi) > > static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs) > > { > > BDRVQcow2State *s = bs->opaque; > > - ImageInfoSpecific *spec_info = g_new(ImageInfoSpecific, 1); > > + ImageInfoSpecific *spec_info; > > + QCryptoBlockInfo *encrypt_info = NULL; > > > > + if (s->crypto != NULL) { > > + encrypt_info = qcrypto_block_get_info(s->crypto, NULL); > > Worth using &error_abort instead of silently ignoring the error? Is an > error even possible in our output visitor [adding Markus for reference]?
In fact the qcrypto_block_get_info() will never return an error right now as implemented. So I guess we could even just remove the Error **errp parameter from that call > > > + if (!encrypt_info) { > > + return NULL; > > + } > > If you use &error_abort, this is a dead check. > > > + } > > + > > + spec_info = g_new(ImageInfoSpecific, 1); > > + if (encrypt_info) { > > + ImageInfoSpecificQCow2Encryption *qencrypt = > > + g_new(ImageInfoSpecificQCow2Encryption, 1); > > + switch (encrypt_info->format) { > > + case Q_CRYPTO_BLOCK_FORMAT_QCOW: > > + qencrypt->format = BLOCKDEV_QCOW2_ENCRYPTION_FORMAT_AES; > > + qencrypt->u.aes = encrypt_info->u.qcow; > > + break; > > + case Q_CRYPTO_BLOCK_FORMAT_LUKS: > > + qencrypt->format = BLOCKDEV_QCOW2_ENCRYPTION_FORMAT_LUKS; > > + qencrypt->u.luks = encrypt_info->u.luks; > > + break; > > + default: > > + assert(false); > > + } > > + /* Since we did shallow copy above, erase any pointers > > + * in the original info */ > > + memset(&encrypt_info->u, 0, sizeof(encrypt_info->u)); > > + qapi_free_QCryptoBlockInfo(encrypt_info); > > Does QAPI_CLONE_MEMBERS (commit 4626a19c) make this code any easier to > write? Then again, malloc'ing duplicates to then free the unchanged > original is a bit slower than stealing references from the original then > making sure the original can be freed without problem. I don't think CLONE would make a signijficant difference to the size of the code, since we have to remap the enum constants regardless. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|