On Tue, Apr 17, 2018 at 03:23:33PM -0400, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1526382 > > As of QEMU 2.9, qemu-img has enforced using the "key-secret" for > creation of encrypted volumes. That is, LUKS encryption is now > required and the old (awful) qcow[2] encryption methodolgy is > no longer supported. Not quite right actually. The 'key-secret' approach can be used to create both LUKS and the old qcow[2] encryption. We only forbid qcow[2] encryption with the system emulators, still have full support in qemu-img for sake of interoperability. The only break there was the command line syntax > > In order to check for this, we scan the qemu-img -o help options > looking for "key-secret" and if set, we enforce during the create > volume processing that the about to be encrypted volume doesn't > attempt to use the old crufty encryption mechanism. > > Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> > --- > src/storage/storage_util.c | 32 ++++++++++++++++++++++++++++++-- > 1 file changed, 30 insertions(+), 2 deletions(-) > > diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c > index 7df52239c2..d2e02a57ca 100644 > --- a/src/storage/storage_util.c > +++ b/src/storage/storage_util.c > @@ -797,6 +797,7 @@ storagePloopResize(virStorageVolDefPtr vol, > enum { > QEMU_IMG_BACKING_FORMAT_OPTIONS = 0, > QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT, > + QEMU_IMG_BACKING_FORMAT_OPTIONS_KEY_SECRET, > }; > > static char * > @@ -824,6 +825,14 @@ virStorageBackendQemuImgSupportsCompat(const char *output) > return strstr(output, "\ncompat "); > } > > + > +static bool > +virStorageBackendQemuImgRequiresKeySecret(const char *output) > +{ > + return strstr(output, "key-secret"); > +} > + > + > static int > virStorageBackendQEMUImgBackingFormat(const char *qemuimg) > { > @@ -843,6 +852,11 @@ virStorageBackendQEMUImgBackingFormat(const char *qemuimg) > if (virStorageBackendQemuImgSupportsCompat(output)) > ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT; > > + /* QEMU 2.9 enforced that qemu-img creation of an encrypted volume > + * uses LUKS encryption. */ > + if (virStorageBackendQemuImgRequiresKeySecret(output)) > + ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_KEY_SECRET; > + > cleanup: > VIR_FREE(output); > return ret; > @@ -934,6 +948,7 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc, > > /* storageBackendCreateQemuImgCheckEncryption: > * @format: format of file found > + * @imgformat: image format capability > * @conn: pointer to connection > * @vol: pointer to volume def > * > @@ -943,6 +958,7 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc, > */ > static int > storageBackendCreateQemuImgCheckEncryption(int format, > + int imgformat, > const char *type, > virStorageVolDefPtr vol) > { > @@ -956,6 +972,12 @@ storageBackendCreateQemuImgCheckEncryption(int format, > vol->target.encryption->format); > return -1; > } > + if (imgformat >= QEMU_IMG_BACKING_FORMAT_OPTIONS_KEY_SECRET) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("qemu-img no longer supports qcow encryption, " > + "use LUKS encryption instead")); > + return -1; > + } Why is imgformat being compared against QEMU_IMG_BACKING_FORMAT_OPTIONS_KEY_SECRET ? Aren't those two sides of the expression from completely different enum types. > if (enc->nsecrets > 1) { > virReportError(VIR_ERR_XML_ERROR, "%s", > _("too many secrets for qcow encryption")); > @@ -1264,8 +1286,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, > return NULL; > > if (info.encryption && > - storageBackendCreateQemuImgCheckEncryption(info.format, type, > - vol) < 0) > + storageBackendCreateQemuImgCheckEncryption(info.format, imgformat, > + type, vol) < 0) > return NULL; > > > @@ -2359,6 +2381,7 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool, > { > int ret = -1; > char *img_tool = NULL; > + int imgformat; > virCommandPtr cmd = NULL; > const char *type; > char *secretPath = NULL; > @@ -2371,6 +2394,10 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool, > return -1; > } > > + imgformat = virStorageBackendQEMUImgBackingFormat("qemu-img"); > + if (imgformat < 0) > + goto cleanup; > + > if (vol->target.encryption) { > if (vol->target.format == VIR_STORAGE_FILE_RAW) > type = "luks"; > @@ -2380,6 +2407,7 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool, > storageBackendLoadDefaultSecrets(vol); > > if (storageBackendCreateQemuImgCheckEncryption(vol->target.format, > + imgformat, > type, vol) < 0) > goto cleanup; > > -- > 2.13.6 > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
