[libvirt] [PATCH 0/5] Properly resize a local LUKS encrypted volume
The patches hopefully speak for themselves. John Ferlan (5): storage: Alter args to storageBackendResizeQemuImg storage: Add error path for virStorageBackendCreateQemuImgCmdFromVol storage: Alter storageBackendCreateQemuImgSecretObject args storage: Properly resize a local volume using LUKS docs: Add news article for bug fix docs/news.xml | 8 +++ src/storage/storage_util.c | 133 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 115 insertions(+), 26 deletions(-) -- 2.13.6
Rather than passing just the path, pass the virStorageVolDefPtr as we're going to need it shortly. Also fix the order of code and stack variables in the calling function virStorageBackendVolResizeLocal. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index a10e4590f3..50db358dc4 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -2287,7 +2287,7 @@ virStorageBackendVolRefreshLocal(virConnectPtr conn, static int -storageBackendResizeQemuImg(const char *path, +storageBackendResizeQemuImg(virStorageVolDefPtr vol, unsigned long long capacity) { int ret = -1; @@ -2306,7 +2306,7 @@ storageBackendResizeQemuImg(const char *path, capacity = VIR_ROUND_UP(capacity, 512); cmd = virCommandNew(img_tool); - virCommandAddArgList(cmd, "resize", path, NULL); + virCommandAddArgList(cmd, "resize", vol->target.path, NULL); virCommandAddArgFormat(cmd, "%llu", capacity); ret = virCommandRun(cmd, NULL); @@ -2328,11 +2328,11 @@ virStorageBackendVolResizeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long long capacity, unsigned int flags) { + bool pre_allocate = flags & VIR_STORAGE_VOL_RESIZE_ALLOCATE; + virCheckFlags(VIR_STORAGE_VOL_RESIZE_ALLOCATE | VIR_STORAGE_VOL_RESIZE_SHRINK, -1); - bool pre_allocate = flags & VIR_STORAGE_VOL_RESIZE_ALLOCATE; - if (vol->target.format == VIR_STORAGE_FILE_RAW) { return virStorageFileResize(vol->target.path, capacity, pre_allocate); } else if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { @@ -2345,7 +2345,7 @@ virStorageBackendVolResizeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } - return storageBackendResizeQemuImg(vol->target.path, capacity); + return storageBackendResizeQemuImg(vol, capacity); } } -- 2.13.6
Rather than inline the various free's and return NULL, just create an error label. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 50db358dc4..561899a3d0 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1261,19 +1261,13 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, if (info.format == VIR_STORAGE_FILE_RAW && vol->target.encryption != NULL && vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { - if (storageBackendCreateQemuImgSecretObject(cmd, vol, &info) < 0) { - VIR_FREE(info.secretAlias); - virCommandFree(cmd); - return NULL; - } + if (storageBackendCreateQemuImgSecretObject(cmd, vol, &info) < 0) + goto error; enc = &vol->target.encryption->encinfo; } - if (storageBackendCreateQemuImgSetOptions(cmd, imgformat, enc, info) < 0) { - VIR_FREE(info.secretAlias); - virCommandFree(cmd); - return NULL; - } + if (storageBackendCreateQemuImgSetOptions(cmd, imgformat, enc, info) < 0) + goto error; VIR_FREE(info.secretAlias); if (info.inputPath) @@ -1283,6 +1277,11 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, virCommandAddArgFormat(cmd, "%lluK", info.size_arg); return cmd; + + error: + VIR_FREE(info.secretAlias); + virCommandFree(cmd); + return NULL; } -- 2.13.6
Since all that was really needed was a couple of fields and building the object can be more generic, let's alter the args a bit. This will be useful shortly for adding the secret object for a volume resize operation on a luks volume that will need a secret object. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 561899a3d0..e6d2747e8d 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1120,17 +1120,14 @@ storageBackendCreateQemuImgSetOptions(virCommandPtr cmd, */ static int storageBackendCreateQemuImgSecretObject(virCommandPtr cmd, - virStorageVolDefPtr vol, - struct _virStorageBackendQemuImgInfo *info) + const char *secretPath, + const char *secretAlias) { virBuffer buf = VIR_BUFFER_INITIALIZER; char *commandStr = NULL; - if (virAsprintf(&info->secretAlias, "%s_luks0", vol->name) < 0) - return -1; - - virBufferAsprintf(&buf, "secret,id=%s,file=", info->secretAlias); - virQEMUBuildBufferEscapeComma(&buf, info->secretPath); + virBufferAsprintf(&buf, "secret,id=%s,file=", secretAlias); + virQEMUBuildBufferEscapeComma(&buf, secretPath); if (virBufferCheckError(&buf) < 0) { virBufferFreeAndReset(&buf); @@ -1261,7 +1258,10 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, if (info.format == VIR_STORAGE_FILE_RAW && vol->target.encryption != NULL && vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { - if (storageBackendCreateQemuImgSecretObject(cmd, vol, &info) < 0) + if (virAsprintf(&info.secretAlias, "%s_luks0", vol->name) < 0) + goto error; + if (storageBackendCreateQemuImgSecretObject(cmd, info.secretPath, + info.secretAlias) < 0) goto error; enc = &vol->target.encryption->encinfo; } -- 2.13.6
https://bugzilla.redhat.com/show_bug.cgi?id=1490279 Turns out the virStorageBackendVolResizeLocal did not differentiate whether the target volume was a LUKS volume or not and just blindly did the ftruncate() on the target volume. Follow the volume creation logic (in general) and create a qemu-img resize command to resize the target volume for LUKS ensuring that the --object secret is provided as well as the '--image-opts' used by the qemu-img resize logic to describe the path and secret ensuring that it's using the luks driver on the volume of course. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 98 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 90 insertions(+), 8 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index e6d2747e8d..e485fa5a95 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1143,6 +1143,37 @@ storageBackendCreateQemuImgSecretObject(virCommandPtr cmd, } +/* Add a --image-opts to the qemu-img resize command line: + * --image-opts driver=luks,file.filename=$volpath,key-secret=$secretAlias + * + * NB: format=raw is assumed + */ +static int +storageBackendResizeQemuImgImageOpts(virCommandPtr cmd, + const char *path, + const char *secretAlias) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *commandStr = NULL; + + virBufferAsprintf(&buf, "driver=luks,key-secret=%s,file.filename=", + secretAlias); + virQEMUBuildBufferEscapeComma(&buf, path); + + if (virBufferCheckError(&buf) < 0) { + virBufferFreeAndReset(&buf); + return -1; + } + + commandStr = virBufferContentAndReset(&buf); + + virCommandAddArgList(cmd, "--image-opts", commandStr, NULL); + + VIR_FREE(commandStr); + return 0; +} + + /* Create a qemu-img virCommand from the supplied binary path, * volume definitions and imgformat */ @@ -2286,12 +2317,17 @@ virStorageBackendVolRefreshLocal(virConnectPtr conn, static int -storageBackendResizeQemuImg(virStorageVolDefPtr vol, +storageBackendResizeQemuImg(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, unsigned long long capacity) { int ret = -1; - char *img_tool; + char *img_tool = NULL; virCommandPtr cmd = NULL; + const char *type; + char *secretPath = NULL; + char *secretAlias = NULL; img_tool = virFindFileInPath("qemu-img"); if (!img_tool) { @@ -2300,19 +2336,56 @@ storageBackendResizeQemuImg(virStorageVolDefPtr vol, return -1; } + if (vol->target.encryption) { + if (vol->target.format == VIR_STORAGE_FILE_RAW) + type = "luks"; + else + type = virStorageFileFormatTypeToString(vol->target.format); + + storageBackendLoadDefaultSecrets(conn, vol); + + if (storageBackendCreateQemuImgCheckEncryption(vol->target.format, + type, NULL, vol) < 0) + goto cleanup; + + if (!(secretPath = + storageBackendCreateQemuImgSecretPath(conn, pool, vol))) + goto cleanup; + + if (virAsprintf(&secretAlias, "%s_luks0", vol->name) < 0) + goto cleanup; + } + /* Round capacity as qemu-img resize errors out on sizes which are not * a multiple of 512 */ capacity = VIR_ROUND_UP(capacity, 512); cmd = virCommandNew(img_tool); - virCommandAddArgList(cmd, "resize", vol->target.path, NULL); + if (!vol->target.encryption) { + virCommandAddArgList(cmd, "resize", vol->target.path, NULL); + } else { + virCommandAddArgList(cmd, "resize", NULL); + + if (storageBackendCreateQemuImgSecretObject(cmd, secretPath, + secretAlias) < 0) + goto cleanup; + + if (storageBackendResizeQemuImgImageOpts(cmd, vol->target.path, + secretAlias) < 0) + goto cleanup; + } virCommandAddArgFormat(cmd, "%llu", capacity); ret = virCommandRun(cmd, NULL); + cleanup: VIR_FREE(img_tool); + if (secretPath) { + unlink(secretPath); + VIR_FREE(secretPath); + } + VIR_FREE(secretAlias); virCommandFree(cmd); - return ret; } @@ -2321,8 +2394,8 @@ storageBackendResizeQemuImg(virStorageVolDefPtr vol, * Resize a volume */ int -virStorageBackendVolResizeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +virStorageBackendVolResizeLocal(virConnectPtr conn, + virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned long long capacity, unsigned int flags) @@ -2332,8 +2405,17 @@ virStorageBackendVolResizeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, virCheckFlags(VIR_STORAGE_VOL_RESIZE_ALLOCATE | VIR_STORAGE_VOL_RESIZE_SHRINK, -1); - if (vol->target.format == VIR_STORAGE_FILE_RAW) { + if (vol->target.format == VIR_STORAGE_FILE_RAW && !vol->target.encryption) { return virStorageFileResize(vol->target.path, capacity, pre_allocate); + } else if (vol->target.format == VIR_STORAGE_FILE_RAW && vol->target.encryption) { + if (pre_allocate) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("preallocate is only supported for raw " + "type volume")); + return -1; + } + + return storageBackendResizeQemuImg(conn, pool, vol, capacity); } else if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { return storagePloopResize(vol, capacity); } else { @@ -2344,7 +2426,7 @@ virStorageBackendVolResizeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } - return storageBackendResizeQemuImg(vol, capacity); + return storageBackendResizeQemuImg(conn, pool, vol, capacity); } } -- 2.13.6
On 10/06/2017 08:13 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1490279
Turns out the virStorageBackendVolResizeLocal did not differentiate whether the target volume was a LUKS volume or not and just blindly did the ftruncate() on the target volume.
Follow the volume creation logic (in general) and create a qemu-img resize command to resize the target volume for LUKS ensuring that the --object secret is provided as well as the '--image-opts' used by the qemu-img resize logic to describe the path and secret ensuring that it's using the luks driver on the volume of course.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 98 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 90 insertions(+), 8 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index e6d2747e8d..e485fa5a95 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1143,6 +1143,37 @@ storageBackendCreateQemuImgSecretObject(virCommandPtr cmd, }
+/* Add a --image-opts to the qemu-img resize command line: + * --image-opts driver=luks,file.filename=$volpath,key-secret=$secretAlias + * + * NB: format=raw is assumed + */ +static int +storageBackendResizeQemuImgImageOpts(virCommandPtr cmd, + const char *path, + const char *secretAlias) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *commandStr = NULL; + + virBufferAsprintf(&buf, "driver=luks,key-secret=%s,file.filename=", + secretAlias); + virQEMUBuildBufferEscapeComma(&buf, path); + + if (virBufferCheckError(&buf) < 0) { + virBufferFreeAndReset(&buf); + return -1; + } + + commandStr = virBufferContentAndReset(&buf); + + virCommandAddArgList(cmd, "--image-opts", commandStr, NULL); + + VIR_FREE(commandStr); + return 0; +} + + /* Create a qemu-img virCommand from the supplied binary path, * volume definitions and imgformat */ @@ -2286,12 +2317,17 @@ virStorageBackendVolRefreshLocal(virConnectPtr conn,
static int -storageBackendResizeQemuImg(virStorageVolDefPtr vol, +storageBackendResizeQemuImg(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, unsigned long long capacity) { int ret = -1; - char *img_tool; + char *img_tool = NULL; virCommandPtr cmd = NULL; + const char *type; + char *secretPath = NULL; + char *secretAlias = NULL;
img_tool = virFindFileInPath("qemu-img"); if (!img_tool) { @@ -2300,19 +2336,56 @@ storageBackendResizeQemuImg(virStorageVolDefPtr vol, return -1; }
+ if (vol->target.encryption) { + if (vol->target.format == VIR_STORAGE_FILE_RAW) + type = "luks"; + else + type = virStorageFileFormatTypeToString(vol->target.format); + + storageBackendLoadDefaultSecrets(conn, vol); + + if (storageBackendCreateQemuImgCheckEncryption(vol->target.format, + type, NULL, vol) < 0) + goto cleanup; + + if (!(secretPath = + storageBackendCreateQemuImgSecretPath(conn, pool, vol))) + goto cleanup; + + if (virAsprintf(&secretAlias, "%s_luks0", vol->name) < 0) + goto cleanup; + } + /* Round capacity as qemu-img resize errors out on sizes which are not * a multiple of 512 */ capacity = VIR_ROUND_UP(capacity, 512);
cmd = virCommandNew(img_tool); - virCommandAddArgList(cmd, "resize", vol->target.path, NULL); + if (!vol->target.encryption) { + virCommandAddArgList(cmd, "resize", vol->target.path, NULL); + } else { + virCommandAddArgList(cmd, "resize", NULL);
Or just virCommandAddArd(cmd, "resize");
+ + if (storageBackendCreateQemuImgSecretObject(cmd, secretPath, + secretAlias) < 0) + goto cleanup; + + if (storageBackendResizeQemuImgImageOpts(cmd, vol->target.path, + secretAlias) < 0) + goto cleanup; + }
Michal
Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/news.xml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index ff36c800a4..116f4a76c9 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -55,6 +55,14 @@ way. </description> </change> + <change> + Properly resize local LUKS encrypted volume + </change> + <description> + Resizing of a local LUKS encrypted volume will now use qemu-img + to resize the volume. This will require configuring a secret for + the LUKS encrypted volume. + </description> </section> </release> <release version="v3.8.0" date="2017-10-04"> -- 2.13.6
On 10/06/2017 02:13 PM, John Ferlan wrote:
Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/news.xml | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index ff36c800a4..116f4a76c9 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -55,6 +55,14 @@ way. </description> </change> + <change> + Properly resize local LUKS encrypted volume + </change> + <description> + Resizing of a local LUKS encrypted volume will now use qemu-img + to resize the volume. This will require configuring a secret for + the LUKS encrypted volume. + </description> </section> </release> <release version="v3.8.0" date="2017-10-04">
Oh I messed this one up - so much for not running make CHECK after writing news articles ;-)... This should have been: + <change> + <summary> + Properly resize local LUKS encrypted volume + </summary> + <description> + Resizing of a local LUKS encrypted volume will now use qemu-img + to resize the volume. This will require configuring a secret for + the LUKS encrypted volume. + </description> + </change> John
On 10/06/2017 08:13 PM, John Ferlan wrote:
Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/news.xml | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index ff36c800a4..116f4a76c9 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -55,6 +55,14 @@ way. </description> </change> + <change> + Properly resize local LUKS encrypted volume + </change> + <description> + Resizing of a local LUKS encrypted volume will now use qemu-img + to resize the volume. This will require configuring a secret for + the LUKS encrypted volume. + </description> </section> </release> <release version="v3.8.0" date="2017-10-04">
Almost. <change> <summary> ... </summary> <description> ... </description> </change> Michal
ping? Thanks John On 10/06/2017 02:13 PM, John Ferlan wrote:
The patches hopefully speak for themselves.
John Ferlan (5): storage: Alter args to storageBackendResizeQemuImg storage: Add error path for virStorageBackendCreateQemuImgCmdFromVol storage: Alter storageBackendCreateQemuImgSecretObject args storage: Properly resize a local volume using LUKS docs: Add news article for bug fix
docs/news.xml | 8 +++ src/storage/storage_util.c | 133 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 115 insertions(+), 26 deletions(-)
ping^2 ? Tks, John On 10/17/2017 06:34 PM, John Ferlan wrote:
ping?
Thanks
John
On 10/06/2017 02:13 PM, John Ferlan wrote:
The patches hopefully speak for themselves.
John Ferlan (5): storage: Alter args to storageBackendResizeQemuImg storage: Add error path for virStorageBackendCreateQemuImgCmdFromVol storage: Alter storageBackendCreateQemuImgSecretObject args storage: Properly resize a local volume using LUKS docs: Add news article for bug fix
docs/news.xml | 8 +++ src/storage/storage_util.c | 133 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 115 insertions(+), 26 deletions(-)
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 10/06/2017 08:13 PM, John Ferlan wrote:
The patches hopefully speak for themselves.
John Ferlan (5): storage: Alter args to storageBackendResizeQemuImg storage: Add error path for virStorageBackendCreateQemuImgCmdFromVol storage: Alter storageBackendCreateQemuImgSecretObject args storage: Properly resize a local volume using LUKS docs: Add news article for bug fix
docs/news.xml | 8 +++ src/storage/storage_util.c | 133 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 115 insertions(+), 26 deletions(-)
ACK series. Michal
participants (2)
-
John Ferlan -
Michal Privoznik