Re: [libvirt] [PATCH 2/2] storage: remove "luks" storage volume type

The current LUKS support has a "luks" volume type which has a "luks" encryption format. This partially makes sense if you consider the QEMU shorthand syntax only requires you to specify a format=luks, and it'll automagically uses "raw" as the next level driver. QEMU will however let you override the "raw" with any other driver it supports (vmdk, qcow, rbd, iscsi, etc, etc) IOW the intention though is that the "luks" encryption format is applied to all disk formats (whether raw, qcow2, rbd, gluster or whatever). As such it doesn't make much sense for libvirt to say the volume type is "luks" - we should be saying that it is a "raw" file, but with "luks" encryption applied. IOW, when creating a storage volume we should use this XML <volume> <name>demo.raw</name> <capacity>5368709120</capacity> <target> <format type='raw'/> <encryption format='luks'> <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccd2f80d6f'/> </encryption> </target> </volume> and when configuring a guest disk we should use <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/home/berrange/VirtualMachines/demo.raw'/> <target dev='sda' bus='scsi'/> <encryption format='luks'> <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccd2f80d6f'/> </encryption> </disk> This commit thus removes the "luks" storage volume type added in commit 318ebb36f1027b3357a32d6f781bd77d7a9043fd Author: John Ferlan <jferlan(a)redhat.com&gt; Date: Tue Jun 21 12:59:54 2016 -0400 util: Add 'luks' to the FileTypeInfo The storage file probing code is modified so that it can probe the actual encryption formats explicitly, rather than merely probing existance of encryption and letting the storage driver guess the format. The rest of the code is then adapted to deal with VIR_STORAGE_FILE_RAW w/ VIR_STORAGE_ENCRYPTION_FORMAT_LUKS instead of just VIR_STORAGE_FILE_LUKS. The commit mentioned above was included in libvirt v2.0.0. So when querying volume XML this will be a change in behaviour vs the 2.0.0 release - it'll report 'raw' instead of 'luks' for the volume format, but still report 'luks' for encryption format. I think this change is OK because the storage driver did not include any support for creating volumes, nor starting guets with luks volumes in v2.0.0 - that only since then. Clearly if we change this we must do it before v2.1.0 though. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/qemu/qemu_command.c | 10 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_hotplug.c | 2 +- src/storage/storage_backend.c | 41 +++-- src/storage/storage_backend_fs.c | 17 +- src/storage/storage_backend_gluster.c | 5 - src/util/virstoragefile.c | 202 ++++++++++++++++----- src/util/virstoragefile.h | 1 - tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml | 4 +- tests/storagevolxml2xmlin/vol-luks-cipher.xml | 2 +- tests/storagevolxml2xmlin/vol-luks.xml | 2 +- tests/storagevolxml2xmlout/vol-luks-cipher.xml | 2 +- tests/storagevolxml2xmlout/vol-luks.xml | 2 +- 13 files changed, 198 insertions(+), 94 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 862fb29..5ef536a 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1116,9 +1111,9 @@ virStorageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool, int accessRetCode = -1; char *absolutePath = NULL; - if (info->format == VIR_STORAGE_FILE_LUKS) { + if (info->format == VIR_STORAGE_FILE_RAW) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("cannot set backing store for luks volume")); + _("cannot set backing store for raw volume")); return -1; }

@@ -1283,7 +1278,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, _("format features only available with qcow2")); return NULL; } - if (info.format == VIR_STORAGE_FILE_LUKS) { + if (info.format == VIR_STORAGE_FILE_RAW && + vol->target.encryption != NULL) { if (inputvol) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("cannot use inputvol with luks volume"));

@@ -1484,13 +1490,16 @@ virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, if (!inputvol) return NULL; - /* If either volume is a non-raw file vol, we need to use an external - * tool for converting + VIR_WARN("BUild vol from func");

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 2834baa..c264041 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -808,6 +882,45 @@ qcow2GetFeatures(virBitmapPtr *features, } +static bool +virStorageFileHasEncryptionFormat(const struct FileEncryptionInfo *info, + char *buf, + size_t len) +{ + if (!info->magic && info->modeOffset == -1) + return 0; /* Shouldn't happen - expect at least one */ + + if (info->magic) { + if (!virStorageFileMatchesMagic(info->magicOffset, + info->magic, + buf, len)) + return false; + + if (info->versionOffset != -1 && + !virStorageFileMatchesVersion(info->versionOffset, + info->versionSize, + info->versionNumbers, + info->endian, + buf, len)) + return false; + + return true; + } else if (info->modeOffset != -1) { + if (info->modeOffset >= len) + return false; + + if (buf[info->modeOffset] != info->modeValue) + return false; + + return true; + } else { + return false; + } +} + + + +

@@ -820,6 +933,7 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta, int *backingFormat) { int ret = -1; + size_t j;