Perform some code cleanup in areas that are about to be altered. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_util.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index bc048e3dff..4fddbf3f9e 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -874,7 +874,7 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc, /* storageBackendCreateQemuImgCheckEncryption: * @format: format of file found - * @conn: pointer to connection + * @type: TypeToString of format.type * @vol: pointer to volume def * * Ensure the proper setup for encryption. @@ -1199,11 +1199,9 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, return NULL; if (info.encryption && - storageBackendCreateQemuImgCheckEncryption(info.format, type, - vol) < 0) + storageBackendCreateQemuImgCheckEncryption(info.format, type, vol) < 0) return NULL; - /* Size in KB */ info.size_arg = VIR_DIV_UP(vol->target.capacity, 1024); -- 2.14.3

Remove the != NULL checks, use !! for setting info.encryption. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_util.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 4fddbf3f9e..cc1f6e7086 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -423,7 +423,7 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool, reflink_copy = true; - if (vol->target.encryption != NULL) { + if (vol->target.encryption) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("storage pool does not support encrypted volumes")); goto cleanup; @@ -707,7 +707,7 @@ storageBackendCreatePloop(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, return -1; } - if (vol->target.encryption != NULL) { + if (vol->target.encryption) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("encrypted ploop volumes are not supported with " "ploop init")); @@ -1128,7 +1128,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, .format = vol->target.format, .path = vol->target.path, .allocation = vol->target.allocation, - .encryption = vol->target.encryption != NULL, + .encryption = !!vol->target.encryption, .preallocate = !!(flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA), .compat = vol->target.compat, .features = vol->target.features, @@ -1169,8 +1169,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, _("format features only available with qcow2")); return NULL; } - if (info.format == VIR_STORAGE_FILE_RAW && - vol->target.encryption != NULL) { + if (info.format == VIR_STORAGE_FILE_RAW && vol->target.encryption) { if (inputvol) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("cannot use inputvol with encrypted raw volume")); @@ -1220,8 +1219,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, if (info.backingPath) virCommandAddArgList(cmd, "-b", info.backingPath, NULL); - if (info.format == VIR_STORAGE_FILE_RAW && - vol->target.encryption != NULL && + if (info.format == VIR_STORAGE_FILE_RAW && vol->target.encryption && vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { if (virAsprintf(&info.secretAlias, "%s_luks0", vol->name) < 0) goto error; @@ -1415,10 +1413,10 @@ virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, */ if ((vol->type == VIR_STORAGE_VOL_FILE && (vol->target.format != VIR_STORAGE_FILE_RAW || - vol->target.encryption != NULL)) || + vol->target.encryption)) || (inputvol->type == VIR_STORAGE_VOL_FILE && (inputvol->target.format != VIR_STORAGE_FILE_RAW || - inputvol->target.encryption != NULL))) { + inputvol->target.encryption))) { return storageBackendCreateQemuImg; } @@ -2105,7 +2103,7 @@ storageBackendVolBuildLocal(virStoragePoolObjPtr pool, virStorageBackendBuildVolFrom create_func; if (inputvol) { - if (vol->target.encryption != NULL) { + if (vol->target.encryption) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("storage pool does not support " "building encrypted volumes from " -- 2.14.3

Commit id 'a48c71411' altered the logic a bit and didn't remove an unnecessary check as info.encryption is true when vol->target.encryption != NULL, so if we enter the if segment with info.format == VIR_STORAGE_FILE_RAW && vol->target.encryption != NULL, then there's no way info.encryption could be false. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_util.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index cc1f6e7086..4dd73f2734 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1175,11 +1175,6 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, _("cannot use inputvol with encrypted raw volume")); return NULL; } - if (!info.encryption) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing encryption description")); - return NULL; - } if (vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { type = "luks"; } else { -- 2.14.3

Rename to storageBackendCreateQemuImgOpts - which is what it's doing. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/libvirt_private.syms | 2 +- src/storage/storage_util.c | 2 +- src/util/virqemu.c | 8 ++++---- src/util/virqemu.h | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f8883dc50d..20352ffe99 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2593,8 +2593,8 @@ virQEMUBuildCommandLineJSON; virQEMUBuildCommandLineJSONArrayBitmap; virQEMUBuildCommandLineJSONArrayNumbered; virQEMUBuildDriveCommandlineFromJSON; -virQEMUBuildLuksOpts; virQEMUBuildObjectCommandlineFromJSON; +virQEMUBuildQemuImgKeySecretOpts; # util/virrandom.h diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 4dd73f2734..37a649d17b 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -823,7 +823,7 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc, virBuffer buf = VIR_BUFFER_INITIALIZER; if (info.format == VIR_STORAGE_FILE_RAW && enc) { - virQEMUBuildLuksOpts(&buf, enc, info.secretAlias); + virQEMUBuildQemuImgKeySecretOpts(&buf, enc, info.secretAlias); } else { if (info.backingPath) virBufferAsprintf(&buf, "backing_fmt=%s,", diff --git a/src/util/virqemu.c b/src/util/virqemu.c index d6652262fe..04cd71605e 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -303,7 +303,7 @@ virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char *str) /** - * virQEMUBuildLuksOpts: + * virQEMUBuildQemuImgKeySecretOpts: * @buf: buffer to build the string into * @enc: pointer to encryption info * @alias: alias to use @@ -323,9 +323,9 @@ virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char *str) * */ void -virQEMUBuildLuksOpts(virBufferPtr buf, - virStorageEncryptionInfoDefPtr enc, - const char *alias) +virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf, + virStorageEncryptionInfoDefPtr enc, + const char *alias) { virBufferAsprintf(buf, "key-secret=%s,", alias); diff --git a/src/util/virqemu.h b/src/util/virqemu.h index 539d62ab14..2599481753 100644 --- a/src/util/virqemu.h +++ b/src/util/virqemu.h @@ -50,9 +50,9 @@ char *virQEMUBuildObjectCommandlineFromJSON(const char *type, char *virQEMUBuildDriveCommandlineFromJSON(virJSONValuePtr src); void virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char *str); -void virQEMUBuildLuksOpts(virBufferPtr buf, - virStorageEncryptionInfoDefPtr enc, - const char *alias) +void virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf, + virStorageEncryptionInfoDefPtr enc, + const char *alias) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); #endif /* __VIR_QEMU_H_ */ -- 2.14.3

Rather than having storageBackendCreateQemuImgCheckEncryption perform the virStorageGenerateQcowEncryption, let's just do that earlier during storageBackendCreateQemuImg so that the check helper is just a check helper rather doing something different based on whether the format is qcow[2] or raw based encryption. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_util.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 37a649d17b..64d4d1d7d2 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -901,10 +901,10 @@ storageBackendCreateQemuImgCheckEncryption(int format, _("too many secrets for qcow encryption")); return -1; } - if (enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT || - enc->nsecrets == 0) { - if (virStorageGenerateQcowEncryption(vol) < 0) - return -1; + if (enc->nsecrets == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("no secret provided for qcow encryption")); + return -1; } } else if (format == VIR_STORAGE_FILE_RAW) { if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { @@ -1309,6 +1309,26 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, } +static int +storageBackendGenerateSecretData(virStorageVolDefPtr vol) +{ + virStorageEncryptionPtr enc = vol->target.encryption; + + if (!enc) + return 0; + + if ((vol->target.format == VIR_STORAGE_FILE_QCOW || + vol->target.format == VIR_STORAGE_FILE_QCOW2) && + (enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT || + enc->nsecrets == 0)) { + if (virStorageGenerateQcowEncryption(vol) < 0) + return -1; + } + + return 0; +} + + static int storageBackendCreateQemuImg(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, @@ -1330,6 +1350,9 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool, return -1; } + if (storageBackendGenerateSecretData(vol) < 0) + goto cleanup; + if (vol->target.format == VIR_STORAGE_FILE_RAW && vol->target.encryption && vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { -- 2.14.3

On 05/15/2018 10:12 AM, Peter Krempa wrote: [turned off wrapping to avoid nasty looking cut-n-paste from code] Hmmm... let's see... storageBackendResizeQemuImg() { ... if (vol->target.encryption) { ... storageBackendLoadDefaultSecrets(vol); if (storageBackendCreateQemuImgCheckEncryption(vol->target.format, type, vol) < 0) goto cleanup; ... Leading us to: storageBackendLoadDefaultSecrets() { ... if (!vol->target.encryption || vol->target.encryption->nsecrets != 0) return 0; ... otherwise we fill in nsecrets/secrets with the secret for the volume (if found), meaning when we leave we'd have nsecrets == 1. Because nsecrets == 1 that means the CheckEncryption will not attempt to create a secret. If a secret for the volume is not found, then yes we leave with nsecrets == 0 and seemingly would/could have a regression. But let's consider the ramifications and that we created the volume with a specific secret, but we could not find that secret later on when someone went to resize the volume. Currently if this were a luks volume, then the resize would fail in the CheckEncryption because there is no secret. However, for a qcow volume we'd create a new secret! With the new code we'd generate the same failure that luks has but with a qcow specific error message instead of regenerating a new secret for resize that wasn't used for create. So is the new model a regression or a fix? Tks - John

Move generation of secretPath to storageBackendGenerateSecretData and simplify a bit since we know vol->target.encryption is set plus we have a local @enc. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_util.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 64d4d1d7d2..d20a109306 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1310,7 +1310,9 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, static int -storageBackendGenerateSecretData(virStorageVolDefPtr vol) +storageBackendGenerateSecretData(virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + char **secretPath) { virStorageEncryptionPtr enc = vol->target.encryption; @@ -1325,6 +1327,12 @@ storageBackendGenerateSecretData(virStorageVolDefPtr vol) return -1; } + if (vol->target.format == VIR_STORAGE_FILE_RAW && + enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { + if (!(*secretPath = storageBackendCreateQemuImgSecretPath(pool, vol))) + return -1; + } + return 0; } @@ -1350,17 +1358,9 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool, return -1; } - if (storageBackendGenerateSecretData(vol) < 0) + if (storageBackendGenerateSecretData(pool, vol, &secretPath) < 0) goto cleanup; - if (vol->target.format == VIR_STORAGE_FILE_RAW && - vol->target.encryption && - vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { - if (!(secretPath = - storageBackendCreateQemuImgSecretPath(pool, vol))) - goto cleanup; - } - cmd = virStorageBackendCreateQemuImgCmdFromVol(pool, vol, inputvol, flags, create_tool, secretPath); -- 2.14.3

Remove the "luks" distinction as the code is about to become more generic and be able to support qcow encryption as well. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- 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 d20a109306..c72fd47024 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1216,7 +1216,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, if (info.format == VIR_STORAGE_FILE_RAW && vol->target.encryption && vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { - if (virAsprintf(&info.secretAlias, "%s_luks0", vol->name) < 0) + if (virAsprintf(&info.secretAlias, "%s_encrypt0", vol->name) < 0) goto error; if (storageBackendCreateQemuImgSecretObject(cmd, info.secretPath, info.secretAlias) < 0) @@ -1269,7 +1269,7 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, if ((fd = mkostemp(secretPath, O_CLOEXEC)) < 0) { virReportSystemError(errno, "%s", - _("failed to open luks secret file for write")); + _("failed to open secret file for write")); goto error; } @@ -1280,7 +1280,7 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, if (safewrite(fd, secret, secretlen) < 0) { virReportSystemError(errno, "%s", - _("failed to write luks secret file")); + _("failed to write secret file")); goto error; } VIR_FORCE_CLOSE(fd); @@ -1290,7 +1290,7 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, if (chown(secretPath, vol->target.perms->uid, vol->target.perms->gid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to chown luks secret file")); + _("failed to chown secret file")); goto error; } } @@ -2331,7 +2331,7 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool, storageBackendCreateQemuImgSecretPath(pool, vol))) goto cleanup; - if (virAsprintf(&secretAlias, "%s_luks0", vol->name) < 0) + if (virAsprintf(&secretAlias, "%s_encrypt0", vol->name) < 0) goto cleanup; } -- 2.14.3

The only way backing_fmts could be set is if the info->format was not RAW (see storageBackendCreateQemuImgSetBacking), so let's just extract it from the if/else surrounding the application of the encryption options. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_util.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index c72fd47024..cedec10403 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -822,12 +822,13 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc, { virBuffer buf = VIR_BUFFER_INITIALIZER; + if (info.backingPath) + virBufferAsprintf(&buf, "backing_fmt=%s,", + virStorageFileFormatTypeToString(info.backingFormat)); + if (info.format == VIR_STORAGE_FILE_RAW && enc) { virQEMUBuildQemuImgKeySecretOpts(&buf, enc, info.secretAlias); } else { - if (info.backingPath) - virBufferAsprintf(&buf, "backing_fmt=%s,", - virStorageFileFormatTypeToString(info.backingFormat)); if (info.encryption) virBufferAddLit(&buf, "encryption=on,"); if (info.preallocate) { -- 2.14.3

The only way preallocate could be set is if the info->format was not RAW (see storageBackendCreateQemuImgSetBacking), so let's just extract it from the if/else surrounding the application of the encryption options. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_util.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index cedec10403..bd8fb7ca92 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -831,12 +831,13 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc, } else { if (info.encryption) virBufferAddLit(&buf, "encryption=on,"); - if (info.preallocate) { - if (info.size_arg > info.allocation) - virBufferAddLit(&buf, "preallocation=metadata,"); - else - virBufferAddLit(&buf, "preallocation=falloc,"); - } + } + + if (info.preallocate) { + if (info.size_arg > info.allocation) + virBufferAddLit(&buf, "preallocation=metadata,"); + else + virBufferAddLit(&buf, "preallocation=falloc,"); } if (info.nocow) -- 2.14.3

We're about to split up the code a bit more, so we'll need this to be in the local struct. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_util.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index bd8fb7ca92..c28f427a1a 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -794,6 +794,7 @@ storagePloopResize(virStorageVolDefPtr vol, */ struct _virStorageBackendQemuImgInfo { int format; + const char *type; const char *path; unsigned long long size_arg; unsigned long long allocation; @@ -1125,9 +1126,9 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, const char *secretPath) { virCommandPtr cmd = NULL; - const char *type; struct _virStorageBackendQemuImgInfo info = { .format = vol->target.format, + .type = NULL, .path = vol->target.path, .allocation = vol->target.allocation, .encryption = !!vol->target.encryption, @@ -1149,7 +1150,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, if (info.format == VIR_STORAGE_FILE_ISO) info.format = VIR_STORAGE_FILE_RAW; - if (!(type = virStorageFileFormatTypeToString(info.format))) { + if (!(info.type = virStorageFileFormatTypeToString(info.format))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown storage vol type %d"), info.format); @@ -1178,7 +1179,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, return NULL; } if (vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { - type = "luks"; + info.type = "luks"; } else { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Only luks encryption is supported for raw files")); @@ -1195,7 +1196,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, return NULL; if (info.encryption && - storageBackendCreateQemuImgCheckEncryption(info.format, type, vol) < 0) + storageBackendCreateQemuImgCheckEncryption(info.format, info.type, vol) < 0) return NULL; /* Size in KB */ @@ -1209,9 +1210,9 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, if (info.inputPath) virCommandAddArgList(cmd, "convert", "-f", info.inputFormatStr, - "-O", type, NULL); + "-O", info.type, NULL); else - virCommandAddArgList(cmd, "create", "-f", type, NULL); + virCommandAddArgList(cmd, "create", "-f", info.type, NULL); if (info.backingPath) virCommandAddArgList(cmd, "-b", info.backingPath, NULL); -- 2.14.3

Split up virStorageBackendCreateQemuImgCmdFromVol into two parts. It's too long anyway and virStorageBackendCreateQemuImgCmdFromVol should just handle the command line processing. NB: Requires changing info.* into info->* references. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_util.c | 114 +++++++++++++++++++++++++-------------------- 1 file changed, 64 insertions(+), 50 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index c28f427a1a..f7da6743b0 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1116,91 +1116,105 @@ storageBackendResizeQemuImgImageOpts(virCommandPtr cmd, } -/* Create a qemu-img virCommand from the supplied arguments */ -virCommandPtr -virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, - virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol, - unsigned int flags, - const char *create_tool, - const char *secretPath) +static int +virStorageBackendCreateQemuImgSetInfo(virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + struct _virStorageBackendQemuImgInfo *info) { - virCommandPtr cmd = NULL; - struct _virStorageBackendQemuImgInfo info = { - .format = vol->target.format, - .type = NULL, - .path = vol->target.path, - .allocation = vol->target.allocation, - .encryption = !!vol->target.encryption, - .preallocate = !!(flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA), - .compat = vol->target.compat, - .features = vol->target.features, - .nocow = vol->target.nocow, - .secretPath = secretPath, - .secretAlias = NULL, - }; - virStorageEncryptionInfoDefPtr enc = NULL; - - virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); - /* Treat output block devices as 'raw' format */ if (vol->type == VIR_STORAGE_VOL_BLOCK) - info.format = VIR_STORAGE_FILE_RAW; + info->format = VIR_STORAGE_FILE_RAW; - if (info.format == VIR_STORAGE_FILE_ISO) - info.format = VIR_STORAGE_FILE_RAW; + if (info->format == VIR_STORAGE_FILE_ISO) + info->format = VIR_STORAGE_FILE_RAW; - if (!(info.type = virStorageFileFormatTypeToString(info.format))) { + if (!(info->type = virStorageFileFormatTypeToString(info->format))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown storage vol type %d"), - info.format); - return NULL; + info->format); + return -1; } - if (info.preallocate && info.format != VIR_STORAGE_FILE_QCOW2) { + if (info->preallocate && info->format != VIR_STORAGE_FILE_QCOW2) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("metadata preallocation only available with qcow2")); - return NULL; + return -1; } - if (info.compat && info.format != VIR_STORAGE_FILE_QCOW2) { + if (info->compat && info->format != VIR_STORAGE_FILE_QCOW2) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("compatibility option only available with qcow2")); - return NULL; + return -1; } - if (info.features && info.format != VIR_STORAGE_FILE_QCOW2) { + if (info->features && info->format != VIR_STORAGE_FILE_QCOW2) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("format features only available with qcow2")); - return NULL; + return -1; } - if (info.format == VIR_STORAGE_FILE_RAW && vol->target.encryption) { + if (info->format == VIR_STORAGE_FILE_RAW && vol->target.encryption) { if (inputvol) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("cannot use inputvol with encrypted raw volume")); - return NULL; + return -1; } if (vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { - info.type = "luks"; + info->type = "luks"; } else { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Only luks encryption is supported for raw files")); - return NULL; + return -1; } } if (inputvol && - storageBackendCreateQemuImgSetInput(inputvol, &info) < 0) - return NULL; + storageBackendCreateQemuImgSetInput(inputvol, info) < 0) + return -1; if (virStorageSourceHasBacking(&vol->target) && - storageBackendCreateQemuImgSetBacking(pool, vol, inputvol, &info) < 0) - return NULL; + storageBackendCreateQemuImgSetBacking(pool, vol, inputvol, info) < 0) + return -1; - if (info.encryption && - storageBackendCreateQemuImgCheckEncryption(info.format, info.type, vol) < 0) - return NULL; + if (info->encryption && + storageBackendCreateQemuImgCheckEncryption(info->format, info->type, + vol) < 0) + return -1; /* Size in KB */ - info.size_arg = VIR_DIV_UP(vol->target.capacity, 1024); + info->size_arg = VIR_DIV_UP(vol->target.capacity, 1024); + + return 0; +} + + +/* Create a qemu-img virCommand from the supplied arguments */ +virCommandPtr +virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags, + const char *create_tool, + const char *secretPath) +{ + virCommandPtr cmd = NULL; + struct _virStorageBackendQemuImgInfo info = { + .format = vol->target.format, + .type = NULL, + .path = vol->target.path, + .allocation = vol->target.allocation, + .encryption = !!vol->target.encryption, + .preallocate = !!(flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA), + .compat = vol->target.compat, + .features = vol->target.features, + .nocow = vol->target.nocow, + .secretPath = secretPath, + .secretAlias = NULL, + }; + virStorageEncryptionInfoDefPtr enc = NULL; + + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); + + if (virStorageBackendCreateQemuImgSetInfo(pool, vol, inputvol, &info) < 0) + goto error; cmd = virCommandNew(create_tool); -- 2.14.3

Extract out command line setup and run from storageBackendCreateQemuImg as we'll need to run it twice soon. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_util.c | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index f7da6743b0..554fc757ed 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1354,6 +1354,31 @@ storageBackendGenerateSecretData(virStoragePoolObjPtr pool, } +static int +storageBackendDoCreateQemuImg(virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags, + const char *create_tool, + const char *secretPath) +{ + int ret; + virCommandPtr cmd; + + cmd = virStorageBackendCreateQemuImgCmdFromVol(pool, vol, inputvol, + flags, create_tool, + secretPath); + if (!cmd) + return -1; + + ret = virStorageBackendCreateExecCommand(pool, vol, cmd); + + virCommandFree(cmd); + + return ret; +} + + static int storageBackendCreateQemuImg(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, @@ -1362,7 +1387,6 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool, { int ret = -1; char *create_tool; - virCommandPtr cmd; char *secretPath = NULL; virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1); @@ -1378,15 +1402,8 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool, if (storageBackendGenerateSecretData(pool, vol, &secretPath) < 0) goto cleanup; - cmd = virStorageBackendCreateQemuImgCmdFromVol(pool, vol, inputvol, - flags, create_tool, - secretPath); - if (!cmd) - goto cleanup; - - ret = virStorageBackendCreateExecCommand(pool, vol, cmd); - - virCommandFree(cmd); + ret = storageBackendDoCreateQemuImg(pool, vol, inputvol, flags, + create_tool, secretPath); cleanup: if (secretPath) { unlink(secretPath); -- 2.14.3