On Thu, Sep 17, 2020 at 1:06 AM Daniel P. Berrangé <berra...@redhat.com> wrote:
> The storage driver was wired up to support creating raw volumes in LUKS > format, but was never adapted to support LUKS-in-qcow2. This is trivial > as it merely requires the encryption properties to be prefixed with > the "encrypt." prefix, and "encrypt.format=luks" when creating the > volume. > > Signed-off-by: Daniel P. Berrangé <berra...@redhat.com> > --- > src/storage/storage_util.c | 70 +++++++++++++++++++++++++++++--------- > src/util/virqemu.c | 23 +++++++++---- > src/util/virqemu.h | 1 + > 3 files changed, 72 insertions(+), 22 deletions(-) > > diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c > index cf82ea0a87..e5e4fe428f 100644 > --- a/src/storage/storage_util.c > +++ b/src/storage/storage_util.c > @@ -707,7 +707,7 @@ > storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr encinfo, > > virStorageFileFormatTypeToString(info->backingFormat)); > > if (encinfo) > - virQEMUBuildQemuImgKeySecretOpts(&buf, encinfo, > info->secretAlias); > + virQEMUBuildQemuImgKeySecretOpts(&buf, info->format, encinfo, > info->secretAlias); > > if (info->preallocate) { > if (info->size_arg > info->allocation) > @@ -761,7 +761,8 @@ storageBackendCreateQemuImgCheckEncryption(int format, > { > virStorageEncryptionPtr enc = vol->target.encryption; > > - if (format == VIR_STORAGE_FILE_RAW) { > + if (format == VIR_STORAGE_FILE_RAW || > + format == VIR_STORAGE_FILE_QCOW2) { > if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > _("unsupported volume encryption format %d"), > @@ -927,21 +928,34 @@ > storageBackendCreateQemuImgSecretObject(virCommandPtr cmd, > } > > > -/* Add a --image-opts to the qemu-img resize command line: > +/* Add a --image-opts to the qemu-img resize command line for use > + * with encryption: > * --image-opts > driver=luks,file.filename=$volpath,key-secret=$secretAlias > + * or > + * --image-opts > driver=qcow2,file.filename=$volpath,encrypt.key-secret=$secretAlias > * > - * NB: format=raw is assumed > */ > static int > storageBackendResizeQemuImgImageOpts(virCommandPtr cmd, > + int format, > const char *path, > const char *secretAlias) > { > g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; > g_autofree char *commandStr = NULL; > + const char *encprefix; > + const char *driver; > > - virBufferAsprintf(&buf, "driver=luks,key-secret=%s,file.filename=", > - secretAlias); > + if (format == VIR_STORAGE_FILE_QCOW2) { > + driver = "qcow2"; > + encprefix = "encrypt."; > + } else { > + driver = "luks"; > + encprefix = ""; > + } > + > + virBufferAsprintf(&buf, "driver=%s,%skey-secret=%s,file.filename=", > + driver, encprefix, secretAlias); > virQEMUBuildBufferEscapeComma(&buf, path); > > commandStr = virBufferContentAndReset(&buf); > @@ -1006,6 +1020,16 @@ > virStorageBackendCreateQemuImgSetInfo(virStoragePoolObjPtr pool, > return -1; > } > } > storagevolxml2argvtest gets segment fault at this function: ➜ ~ abs_top_srcdir='/root/libvirt-6.7.0' LC_ALL='C' abs_top_builddir='/root/libvirt-6.7.0/build' abs_srcdir='/root/libvirt-6.7.0/tests' abs_builddir='/root/libvirt-6.7.0/build/tests' VIR_TEST_EXPENSIVE='0' LIBVIRT_AUTOSTART='0' /root/libvirt-6.7.0/build/tests/storagevolxml2argvtest TEST: storagevolxml2argvtest ...................![1] 916320 segmentation fault (core dumped) abs_top_srcdir='/root/libvirt-6.7.0' LC_ALL='C' abs_top_builddir= abs_srcdir= Backtrace: (gdb) bt #0 0x0000558b997ea149 in virStorageBackendCreateQemuImgSetInfo (info=<synthetic pointer>, convertStep=VIR_STORAGE_VOL_ENCRYPT_CREATE, inputvol=0x558b9b32e810, vol=0x558b9b32f840, pool=0x558b9b323b30) at ../src/storage/storage_util.c:1025 #1 0x0000558b997ea149 in virStorageBackendCreateQemuImgCmdFromVol (pool=0x558b9b323b30, vol=0x558b9b32f840, inputvol=0x558b9b32e810, flags=0, create_tool=0x558b997f6a00 <create_tool> "qemu-img", secretPath=0x558b997f6537 "/path/to/secretFile", inputSecretPath=0x558b997f654b "/path/to/inputSecretFile", convertStep=VIR_STORAGE_VOL_ENCRYPT_CREATE) at ../src/storage/storage_util.c:1103 #2 0x0000558b997e4b57 in testCompareXMLToArgvFiles (parse_flags=<optimized out>, flags=0, cmdline=0x558b9b325030 "/root/libvirt-6.7.0/tests/storagevolxml2argvdata/luks-convert-encrypt2fileraw.argv", inputvolxml=0x558b9b32f7f0 "/root/libvirt-6.7.0/tests/storagevolxml2xmlin/vol-encrypt2.xml", inputpoolxml=0x558b9b331730 "/root/libvirt-6.7.0/tests/storagepoolxml2xmlin/pool-dir.xml", volxml=0x558b9b3309f0 "/root/libvirt-6.7.0/tests/storagevolxml2xmlin/vol-file.xml", poolxml=0x558b9b32a530 "/root/libvirt-6.7.0/tests/storagepoolxml2xmlin/pool-dir.xml", shouldFail=false) at ../tests/storagevolxml2argvtest.c:92 #3 0x0000558b997e4b57 in testCompareXMLToArgvHelper (data=<optimized out>) at ../tests/storagevolxml2argvtest.c:174 #4 0x0000558b997e561a in virTestRun (title=0x558b997f6928 "Storage Vol XML-2-argv luks-convert-encrypt2fileraw", body=0x558b997e4930 <testCompareXMLToArgvHelper>, data=0x7ffea16a5870) at ../tests/testutils.c:142 #5 0x0000558b997e489a in mymain () at ../tests/storagevolxml2argvtest.c:271 #6 0x0000558b997e6512 in virTestMain (argc=1, argv=0x7ffea16a5aa8, func=0x558b997e40d0 <mymain>) at ../tests/testutils.c:841 #7 0x00007f2597f5b7b3 in __libc_start_main (main=0x558b997e3fd0 <main>, argc=1, argv=0x7ffea16a5aa8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffea16a5a98) at ../csu/libc-start.c:308 #8 0x0000558b997e400e in _start () at ../tests/storagevolxml2argvtest.c:282 > + if (inputvol && inputvol->target.format == VIR_STORAGE_FILE_RAW && > + inputvol->target.encryption) { > + if (vol->target.encryption->format == > VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { > + info->type = "luks"; > + } else { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Only luks encryption is supported for raw > files")); > + return -1; > + } > + } > > if (inputvol && > storageBackendCreateQemuImgSetInput(inputvol, convertStep, info) > < 0) > @@ -1056,6 +1080,8 @@ > virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, > virStorageEncryptionPtr inputenc = inputvol ? > inputvol->target.encryption : NULL; > virStorageEncryptionInfoDefPtr encinfo = NULL; > g_autofree char *inputSecretAlias = NULL; > + const char *encprefix; > + const char *inputencprefix; > > virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); > > @@ -1134,24 +1160,34 @@ > virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, > virCommandAddArgFormat(cmd, "%lluK", info.size_arg); > } else { > /* source */ > - if (inputenc) > + if (inputenc) { > + if (inputvol->target.format == VIR_STORAGE_FILE_QCOW2) > + inputencprefix = "encrypt."; > + else > + inputencprefix = ""; > virCommandAddArgFormat(cmd, > - > "driver=luks,file.filename=%s,key-secret=%s", > - info.inputPath, inputSecretAlias); > - else > + > "driver=%s,file.filename=%s,%skey-secret=%s", > + info.inputType, info.inputPath, > inputencprefix, inputSecretAlias); > + } else { > virCommandAddArgFormat(cmd, "driver=%s,file.filename=%s", > info.inputType ? info.inputType : > "raw", > info.inputPath); > + } > > /* dest */ > - if (enc) > + if (enc) { > + if (vol->target.format == VIR_STORAGE_FILE_QCOW2) > + encprefix = "encrypt."; > + else > + encprefix = ""; > + > virCommandAddArgFormat(cmd, > - > "driver=%s,file.filename=%s,key-secret=%s", > - info.type, info.path, > info.secretAlias); > - else > + > "driver=%s,file.filename=%s,%skey-secret=%s", > + info.type, info.path, encprefix, > info.secretAlias); > + } else { > virCommandAddArgFormat(cmd, "driver=%s,file.filename=%s", > info.type, info.path); > - > + } > } > VIR_FREE(info.secretAlias); > > @@ -2276,7 +2312,9 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr > pool, > secretAlias) < 0) > goto cleanup; > > - if (storageBackendResizeQemuImgImageOpts(cmd, vol->target.path, > + if (storageBackendResizeQemuImgImageOpts(cmd, > + vol->target.format, > + vol->target.path, > secretAlias) < 0) > goto cleanup; > } > diff --git a/src/util/virqemu.c b/src/util/virqemu.c > index 25d6fd35c5..bbb38eed75 100644 > --- a/src/util/virqemu.c > +++ b/src/util/virqemu.c > @@ -28,6 +28,7 @@ > #include "virqemu.h" > #include "virstring.h" > #include "viralloc.h" > +#include "virstoragefile.h" > > #define VIR_FROM_THIS VIR_FROM_NONE > > @@ -407,36 +408,46 @@ virQEMUBuildBufferEscapeComma(virBufferPtr buf, > const char *str) > */ > void > virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf, > + int format, > virStorageEncryptionInfoDefPtr encinfo, > const char *alias) > { > - virBufferAsprintf(buf, "key-secret=%s,", alias); > + const char *encprefix; > + > + if (format == VIR_STORAGE_FILE_QCOW2) { > + virBufferAsprintf(buf, "encrypt.format=luks,"); > syntax check is failed here: --- command --- 09:13:21 /usr/bin/make -C /root/libvirt-6.7.0/build/build-aux sc_prohibit_virBufferAsprintf_with_string_literal --- stdout --- make: Entering directory '/root/libvirt-6.7.0/build/build-aux' prohibit_virBufferAsprintf_with_string_literal /root/libvirt-6.7.0/src/util/virqemu.c:418: virBufferAsprintf(buf, "encrypt.format=luks,"); > + encprefix = "encrypt."; > + } else { > + encprefix = ""; > + } > + > + virBufferAsprintf(buf, "%skey-secret=%s,", encprefix, alias); > > if (!encinfo->cipher_name) > return; > > - virBufferAddLit(buf, "cipher-alg="); > + virBufferAsprintf(buf, "%scipher-alg=", encprefix); > virQEMUBuildBufferEscapeComma(buf, encinfo->cipher_name); > virBufferAsprintf(buf, "-%u,", encinfo->cipher_size); > if (encinfo->cipher_mode) { > - virBufferAddLit(buf, "cipher-mode="); > + virBufferAsprintf(buf, "%scipher-mode=", encprefix); > virQEMUBuildBufferEscapeComma(buf, encinfo->cipher_mode); > virBufferAddLit(buf, ","); > } > if (encinfo->cipher_hash) { > - virBufferAddLit(buf, "hash-alg="); > + virBufferAsprintf(buf, "%shash-alg=", encprefix); > virQEMUBuildBufferEscapeComma(buf, encinfo->cipher_hash); > virBufferAddLit(buf, ","); > } > if (!encinfo->ivgen_name) > return; > > - virBufferAddLit(buf, "ivgen-alg="); > + virBufferAsprintf(buf, "%sivgen-alg=", encprefix); > virQEMUBuildBufferEscapeComma(buf, encinfo->ivgen_name); > virBufferAddLit(buf, ","); > > if (encinfo->ivgen_hash) { > - virBufferAddLit(buf, "ivgen-hash-alg="); > + virBufferAsprintf(buf, "%sivgen-hash-alg=", encprefix); > virQEMUBuildBufferEscapeComma(buf, encinfo->ivgen_hash); > virBufferAddLit(buf, ","); > } > diff --git a/src/util/virqemu.h b/src/util/virqemu.h > index b1296cb657..be14c04d51 100644 > --- a/src/util/virqemu.h > +++ b/src/util/virqemu.h > @@ -60,6 +60,7 @@ char > *virQEMUBuildDriveCommandlineFromJSON(virJSONValuePtr src); > > void virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char *str); > void virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf, > + int format, > virStorageEncryptionInfoDefPtr enc, > const char *alias) > ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); > -- > 2.26.2 > > -- Reviewed-by: Han Han <h...@redhat.com>