src/storage/storage_backend_disk.c | 49 ++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 20 deletions(-)
https://bugzilla.redhat.com/show_bug.cgi?id=1560946
Similar to the the Logical backend, use qemu-img on the created
disk partition device to set up for LUKS encryption. Secret mgmt
for the device can be complicated by a reboot possibly changing
the path to the device if the infrastructure changes.
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
Changes in v2: Don't alter the result 'endOffset' of the
DiskPartBoundaries, rather modify the input capacity value.
Alteration of the value for the error path won't matter
since the volume would be deleted anyway. The call to
virStorageBackendDiskPartBoundaries was also slightly
modified to check the return value vs -1 rather than != 0
so that the call is similar to other calls. Separating
that into it's own patch could have been done, but just
felt like too much "busy work" to be worth the trouble.
src/storage/storage_backend_disk.c | 49 ++++++++++++++++++++++----------------
1 file changed, 29 insertions(+), 20 deletions(-)
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index 2e3d1e04a4..e9578d01d6 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -879,28 +879,26 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool,
char *partFormat = NULL;
unsigned long long startOffset = 0, endOffset = 0;
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
+ virErrorPtr save_err;
virCommandPtr cmd = virCommandNewArgList(PARTED,
def->source.devices[0].path,
"mkpart",
"--script",
NULL);
- if (vol->target.encryption != NULL) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- "%s", _("storage pool does not support encrypted "
- "volumes"));
- goto cleanup;
- }
-
if (virStorageBackendDiskPartFormat(pool, vol, &partFormat) != 0)
goto cleanup;
virCommandAddArg(cmd, partFormat);
- if (virStorageBackendDiskPartBoundaries(pool, &startOffset,
- &endOffset,
- vol->target.capacity) != 0) {
+ /* 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)
+ vol->target.capacity += 2 * 1024 * 1024;
+
+ if (virStorageBackendDiskPartBoundaries(pool, &startOffset, &endOffset,
+ vol->target.capacity) < 0)
goto cleanup;
- }
virCommandAddArgFormat(cmd, "%lluB", startOffset);
virCommandAddArgFormat(cmd, "%lluB", endOffset);
@@ -919,15 +917,15 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool,
VIR_FREE(vol->target.path);
/* Fetch actual extent info, generate key */
- if (virStorageBackendDiskReadPartitions(pool, vol) < 0) {
- /* Best effort to remove the partition. Ignore any errors
- * since we could be calling this with vol->target.path == NULL
- */
- virErrorPtr save_err = virSaveLastError();
- ignore_value(virStorageBackendDiskDeleteVol(pool, vol, 0));
- virSetError(save_err);
- virFreeError(save_err);
- goto cleanup;
+ if (virStorageBackendDiskReadPartitions(pool, vol) < 0)
+ goto error;
+
+ if (vol->target.encryption) {
+ /* Adjust the sizes to account for the LUKS header */
+ vol->target.capacity -= 2 * 1024 * 1024;
+ vol->target.allocation -= 2 * 1024 * 1024;
+ if (virStorageBackendCreateVolUsingQemuImg(pool, vol, NULL, 0) < 0)
+ goto error;
}
res = 0;
@@ -936,8 +934,19 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool,
VIR_FREE(partFormat);
virCommandFree(cmd);
return res;
+
+ error:
+ /* Best effort to remove the partition. Ignore any errors
+ * since we could be calling this with vol->target.path == NULL
+ */
+ save_err = virSaveLastError();
+ ignore_value(virStorageBackendDiskDeleteVol(pool, vol, 0));
+ virSetError(save_err);
+ virFreeError(save_err);
+ goto cleanup;
}
+
static int
virStorageBackendDiskBuildVolFrom(virStoragePoolObjPtr pool,
virStorageVolDefPtr vol,
--
2.14.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, May 29, 2018 at 08:53:06 -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1560946
>
> Similar to the the Logical backend, use qemu-img on the created
> disk partition device to set up for LUKS encryption. Secret mgmt
> for the device can be complicated by a reboot possibly changing
> the path to the device if the infrastructure changes.
>
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
> Changes in v2: Don't alter the result 'endOffset' of the
> DiskPartBoundaries, rather modify the input capacity value.
> Alteration of the value for the error path won't matter
> since the volume would be deleted anyway. The call to
> virStorageBackendDiskPartBoundaries was also slightly
> modified to check the return value vs -1 rather than != 0
> so that the call is similar to other calls. Separating
> that into it's own patch could have been done, but just
> felt like too much "busy work" to be worth the trouble.
>
> src/storage/storage_backend_disk.c | 49 ++++++++++++++++++++++----------------
> 1 file changed, 29 insertions(+), 20 deletions(-)
>
> diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
> index 2e3d1e04a4..e9578d01d6 100644
> --- a/src/storage/storage_backend_disk.c
> +++ b/src/storage/storage_backend_disk.c
> @@ -879,28 +879,26 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool,
> char *partFormat = NULL;
> unsigned long long startOffset = 0, endOffset = 0;
> virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
> + virErrorPtr save_err;
> virCommandPtr cmd = virCommandNewArgList(PARTED,
> def->source.devices[0].path,
> "mkpart",
> "--script",
> NULL);
>
> - if (vol->target.encryption != NULL) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - "%s", _("storage pool does not support encrypted "
> - "volumes"));
> - goto cleanup;
> - }
> -
> if (virStorageBackendDiskPartFormat(pool, vol, &partFormat) != 0)
> goto cleanup;
> virCommandAddArg(cmd, partFormat);
>
> - if (virStorageBackendDiskPartBoundaries(pool, &startOffset,
> - &endOffset,
> - vol->target.capacity) != 0) {
> + /* 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)
> + vol->target.capacity += 2 * 1024 * 1024;
So this is done only for LUKS encryption ...
> +
> + if (virStorageBackendDiskPartBoundaries(pool, &startOffset, &endOffset,
> + vol->target.capacity) < 0)
> goto cleanup;
> - }
>
> virCommandAddArgFormat(cmd, "%lluB", startOffset);
> virCommandAddArgFormat(cmd, "%lluB", endOffset);
> @@ -919,15 +917,15 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool,
> VIR_FREE(vol->target.path);
>
> /* Fetch actual extent info, generate key */
> - if (virStorageBackendDiskReadPartitions(pool, vol) < 0) {
> - /* Best effort to remove the partition. Ignore any errors
> - * since we could be calling this with vol->target.path == NULL
> - */
> - virErrorPtr save_err = virSaveLastError();
> - ignore_value(virStorageBackendDiskDeleteVol(pool, vol, 0));
> - virSetError(save_err);
> - virFreeError(save_err);
> - goto cleanup;
> + if (virStorageBackendDiskReadPartitions(pool, vol) < 0)
> + goto error;
> +
> + if (vol->target.encryption) {
> + /* Adjust the sizes to account for the LUKS header */
> + vol->target.capacity -= 2 * 1024 * 1024;
> + vol->target.allocation -= 2 * 1024 * 1024;
but this is done for any encryption type, so if VIR_STORAGE_ENCRYPTION_FORMAT_QCOW
would be used it would rob off two megabytes of the volume size.
Honestly I think that VIR_STORAGE_ENCRYPTION_FORMAT_QCOW should be
disallowed for creation code right away, so that nobody creates the
broken format any more.
That way we don't have to deal with possibility that somebody with a old
qemu-img would run into problems here without adding much spurious code.
> + if (virStorageBackendCreateVolUsingQemuImg(pool, vol, NULL, 0) < 0)
> + goto error;
ACK if you reject VIR_STORAGE_ENCRYPTION_FORMAT_QCOW right at the
beginning.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 05/29/2018 09:34 AM, Peter Krempa wrote:
> On Tue, May 29, 2018 at 08:53:06 -0400, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1560946
>>
>> Similar to the the Logical backend, use qemu-img on the created
>> disk partition device to set up for LUKS encryption. Secret mgmt
>> for the device can be complicated by a reboot possibly changing
>> the path to the device if the infrastructure changes.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>> Changes in v2: Don't alter the result 'endOffset' of the
>> DiskPartBoundaries, rather modify the input capacity value.
>> Alteration of the value for the error path won't matter
>> since the volume would be deleted anyway. The call to
>> virStorageBackendDiskPartBoundaries was also slightly
>> modified to check the return value vs -1 rather than != 0
>> so that the call is similar to other calls. Separating
>> that into it's own patch could have been done, but just
>> felt like too much "busy work" to be worth the trouble.
>>
>> src/storage/storage_backend_disk.c | 49 ++++++++++++++++++++++----------------
>> 1 file changed, 29 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
>> index 2e3d1e04a4..e9578d01d6 100644
>> --- a/src/storage/storage_backend_disk.c
>> +++ b/src/storage/storage_backend_disk.c
>> @@ -879,28 +879,26 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool,
>> char *partFormat = NULL;
>> unsigned long long startOffset = 0, endOffset = 0;
>> virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
>> + virErrorPtr save_err;
>> virCommandPtr cmd = virCommandNewArgList(PARTED,
>> def->source.devices[0].path,
>> "mkpart",
>> "--script",
>> NULL);
>>
>> - if (vol->target.encryption != NULL) {
>> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> - "%s", _("storage pool does not support encrypted "
>> - "volumes"));
>> - goto cleanup;
>> - }
>> -
>> if (virStorageBackendDiskPartFormat(pool, vol, &partFormat) != 0)
>> goto cleanup;
>> virCommandAddArg(cmd, partFormat);
>>
>> - if (virStorageBackendDiskPartBoundaries(pool, &startOffset,
>> - &endOffset,
>> - vol->target.capacity) != 0) {
>> + /* 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)
>> + vol->target.capacity += 2 * 1024 * 1024;
>
> So this is done only for LUKS encryption ...
>
>> +
>> + if (virStorageBackendDiskPartBoundaries(pool, &startOffset, &endOffset,
>> + vol->target.capacity) < 0)
>> goto cleanup;
>> - }
>>
>> virCommandAddArgFormat(cmd, "%lluB", startOffset);
>> virCommandAddArgFormat(cmd, "%lluB", endOffset);
>> @@ -919,15 +917,15 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool,
>> VIR_FREE(vol->target.path);
>>
>> /* Fetch actual extent info, generate key */
>> - if (virStorageBackendDiskReadPartitions(pool, vol) < 0) {
>> - /* Best effort to remove the partition. Ignore any errors
>> - * since we could be calling this with vol->target.path == NULL
>> - */
>> - virErrorPtr save_err = virSaveLastError();
>> - ignore_value(virStorageBackendDiskDeleteVol(pool, vol, 0));
>> - virSetError(save_err);
>> - virFreeError(save_err);
>> - goto cleanup;
>> + if (virStorageBackendDiskReadPartitions(pool, vol) < 0)
>> + goto error;
>> +
>> + if (vol->target.encryption) {
>> + /* Adjust the sizes to account for the LUKS header */
>> + vol->target.capacity -= 2 * 1024 * 1024;
>> + vol->target.allocation -= 2 * 1024 * 1024;
>
> but this is done for any encryption type, so if VIR_STORAGE_ENCRYPTION_FORMAT_QCOW
> would be used it would rob off two megabytes of the volume size.
>
> Honestly I think that VIR_STORAGE_ENCRYPTION_FORMAT_QCOW should be
> disallowed for creation code right away, so that nobody creates the
> broken format any more.
>
> That way we don't have to deal with possibility that somebody with a old
> qemu-img would run into problems here without adding much spurious code.
>
>> + if (virStorageBackendCreateVolUsingQemuImg(pool, vol, NULL, 0) < 0)
>> + goto error;
>
> ACK if you reject VIR_STORAGE_ENCRYPTION_FORMAT_QCOW right at the
> beginning.
>
OK... I'll add this to avoid QCOW and/or DEFAULT
if (vol->target.encryption &&
vol->target.encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("storage pool only supports LUKS encrypted volumes"));
Tks,
John
probably need a similar adjustment for the _logical backend too....
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.