[libvirt] [REPOST PATCH 0/4] Add the ability to LUKS encrypt during LV creation

Repost to address merge conflict from commit id '0a294a8e2' which used if (virStorageSourceHasBacking(&vol->target)) instead of if (vol->target.backingStore). Original series: https://www.redhat.com/archives/libvir-list/2017-October/msg00340.html John Ferlan (4): storage: Extract out the LVCREATE storage: Introduce virStorageBackendCreateVolUsingQemuImg storage: Allow creation of a LUKS using logical volume docs: Add news article docs/news.xml | 13 ++++++ src/storage/storage_backend_logical.c | 75 ++++++++++++++++++++--------------- src/storage/storage_util.c | 42 ++++++++++++++++++++ src/storage/storage_util.h | 8 ++++ 4 files changed, 105 insertions(+), 33 deletions(-) -- 2.13.6

Refactor to extract out the LVCREATE command. This also removes the need for the local @created since the error path can now only be reached after the creation of the logical volume. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_logical.c | 65 +++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index a872a2f881..93f4e0a595 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -938,30 +938,11 @@ virStorageBackendLogicalDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendLogicalCreateVol(virConnectPtr conn, - virStoragePoolObjPtr pool, - virStorageVolDefPtr vol) +virStorageBackendLogicalLVCreate(virStorageVolDefPtr vol, + virStoragePoolDefPtr def) { - int fd = -1; - virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + int ret; virCommandPtr cmd = NULL; - virErrorPtr err; - struct stat sb; - bool created = false; - - if (vol->target.encryption != NULL) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("storage pool does not support encrypted " - "volumes")); - return -1; - } - - vol->type = VIR_STORAGE_VOL_BLOCK; - - VIR_FREE(vol->target.path); - if (virAsprintf(&vol->target.path, "%s/%s", - def->target.path, vol->name) < 0) - return -1; cmd = virCommandNewArgList(LVCREATE, "--name", vol->name, @@ -982,12 +963,38 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, else virCommandAddArg(cmd, def->source.name); - if (virCommandRun(cmd, NULL) < 0) - goto error; - - created = true; + ret = virCommandRun(cmd, NULL); virCommandFree(cmd); - cmd = NULL; + return ret; +} + + +static int +virStorageBackendLogicalCreateVol(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol) +{ + int fd = -1; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + virErrorPtr err; + struct stat sb; + + if (vol->target.encryption != NULL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("storage pool does not support encrypted " + "volumes")); + return -1; + } + + vol->type = VIR_STORAGE_VOL_BLOCK; + + VIR_FREE(vol->target.path); + if (virAsprintf(&vol->target.path, "%s/%s", + def->target.path, vol->name) < 0) + return -1; + + if (virStorageBackendLogicalLVCreate(vol, def) < 0) + return -1; if ((fd = virStorageBackendVolOpen(vol->target.path, &sb, VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) @@ -1031,9 +1038,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, error: err = virSaveLastError(); VIR_FORCE_CLOSE(fd); - if (created) - virStorageBackendLogicalDeleteVol(conn, pool, vol, 0); - virCommandFree(cmd); + virStorageBackendLogicalDeleteVol(conn, pool, vol, 0); virSetError(err); virFreeError(err); return -1; -- 2.13.6

Create a shim that will allow other backends to make use of qemu-img functionality to create or possibly modify the volume. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/storage/storage_util.h | 8 ++++++++ 2 files changed, 50 insertions(+) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 5252e429fd..e9bafd3b15 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1409,6 +1409,48 @@ storageBackendCreateQemuImg(virConnectPtr conn, return ret; } + +/** + * virStorageBackendCreateVolUsingQemuImg + * @conn: Connection pointer + * @pool: Storage Pool Object + * @vol: Volume definition + * @inputvol: Volume to use for creation + * @flags: Flags for creation options + * + * A shim to storageBackendCreateQemuImg to allow other backends to + * utilize qemu-img processing in order to create or alter the volume. + * + * NB: If a volume target format is not supplied (per usual for some + * backends), temporarily adjust the format to be RAW. Once completed, + * reset the format back to NONE. + * + * Returns: 0 on success, -1 on failure. + */ +int +virStorageBackendCreateVolUsingQemuImg(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags) +{ + int ret = -1; + bool changeFormat = false; + + if (vol->target.format == VIR_STORAGE_FILE_NONE) { + vol->target.format = VIR_STORAGE_FILE_RAW; + changeFormat = true; + } + + ret = storageBackendCreateQemuImg(conn, pool, vol, inputvol, flags); + + if (changeFormat) + vol->target.format = VIR_STORAGE_FILE_NONE; + + return ret; +} + + virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, virStorageVolDefPtr inputvol) diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h index 00793ff3a4..dc7e62517b 100644 --- a/src/storage/storage_util.h +++ b/src/storage/storage_util.h @@ -27,6 +27,14 @@ # include "storage_backend.h" /* File creation/cloning functions used for cloning between backends */ + +int +virStorageBackendCreateVolUsingQemuImg(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags); + virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, virStorageVolDefPtr inputvol); -- 2.13.6

https://bugzilla.redhat.com/show_bug.cgi?id=1427049 Use virStorageBackendCreateVolUsingQemuImg to apply the LUKS information to the logical volume just created. As part of the processing of the lvcreate command add 2MB to the capacity to account for the LUKS header when it's determined that the volume desires to use encryption. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_logical.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 93f4e0a595..5df30de29d 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -942,13 +942,14 @@ virStorageBackendLogicalLVCreate(virStorageVolDefPtr vol, virStoragePoolDefPtr def) { int ret; + unsigned long long capacity = vol->target.capacity; virCommandPtr cmd = NULL; cmd = virCommandNewArgList(LVCREATE, "--name", vol->name, NULL); virCommandAddArg(cmd, "-L"); - if (vol->target.capacity != vol->target.allocation) { + if (capacity != vol->target.allocation) { virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->target.allocation ? vol->target.allocation : 1, 1024)); @@ -956,8 +957,14 @@ virStorageBackendLogicalLVCreate(virStorageVolDefPtr vol, virCommandAddArg(cmd, "--virtualsize"); vol->target.sparse = true; } - virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->target.capacity, - 1024)); + + /* If we're going to encrypt using LUKS, then we could need up to + * an extra 2MB for the LUKS header - so account for that now */ + if (vol->target.encryption && + vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) + capacity += 2 * 1024 * 1024; + virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(capacity, 1024)); + if (virStorageSourceHasBacking(&vol->target)) virCommandAddArgList(cmd, "-s", vol->target.backingStore->path, NULL); else @@ -979,13 +986,6 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, virErrorPtr err; struct stat sb; - if (vol->target.encryption != NULL) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("storage pool does not support encrypted " - "volumes")); - return -1; - } - vol->type = VIR_STORAGE_VOL_BLOCK; VIR_FREE(vol->target.path); @@ -996,6 +996,10 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, if (virStorageBackendLogicalLVCreate(vol, def) < 0) return -1; + if (vol->target.encryption && + virStorageBackendCreateVolUsingQemuImg(conn, pool, vol, NULL, 0) < 0) + goto error; + if ((fd = virStorageBackendVolOpen(vol->target.path, &sb, VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) goto error; -- 2.13.6

Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/news.xml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index ff36c800a4..cb59359d76 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -42,6 +42,19 @@ </change> </section> <section title="Improvements"> + <change> + <summary> + Allow a logical volume to be create using LUKS + </summary> + <description> + A logical volume may be created using an <code>encryption</code> + element using "luks" format. This does require a previously created + <code>secret</code> to store the passphrase used to encrypt the + volume Adding the volume to a domain can then either provide the + secret or allow the consumer in the guest to provide the passphrase + in order to decrypt the volume. + </description> + </change> </section> <section title="Bug fixes"> <change> -- 2.13.6

ping? tks John On 10/19/2017 09:56 AM, John Ferlan wrote:
Repost to address merge conflict from commit id '0a294a8e2' which used if (virStorageSourceHasBacking(&vol->target)) instead of if (vol->target.backingStore).
Original series: https://www.redhat.com/archives/libvir-list/2017-October/msg00340.html
John Ferlan (4): storage: Extract out the LVCREATE storage: Introduce virStorageBackendCreateVolUsingQemuImg storage: Allow creation of a LUKS using logical volume docs: Add news article
docs/news.xml | 13 ++++++ src/storage/storage_backend_logical.c | 75 ++++++++++++++++++++--------------- src/storage/storage_util.c | 42 ++++++++++++++++++++ src/storage/storage_util.h | 8 ++++ 4 files changed, 105 insertions(+), 33 deletions(-)

On 10/19/2017 03:56 PM, John Ferlan wrote:
Repost to address merge conflict from commit id '0a294a8e2' which used if (virStorageSourceHasBacking(&vol->target)) instead of if (vol->target.backingStore).
Original series: https://www.redhat.com/archives/libvir-list/2017-October/msg00340.html
John Ferlan (4): storage: Extract out the LVCREATE storage: Introduce virStorageBackendCreateVolUsingQemuImg storage: Allow creation of a LUKS using logical volume docs: Add news article
docs/news.xml | 13 ++++++ src/storage/storage_backend_logical.c | 75 ++++++++++++++++++++--------------- src/storage/storage_util.c | 42 ++++++++++++++++++++ src/storage/storage_util.h | 8 ++++ 4 files changed, 105 insertions(+), 33 deletions(-)
ACK series. Michal
participants (2)
-
John Ferlan
-
Michal Privoznik