[libvirt] [PATCH 0/3] Clean up some virstorageencryption code
While working through adding luks support for libvirt, I have a few patches that aren't really germane to adding support and rather than drop a large patch series when I'm done - I figured I'd post a few adjustments. In the long run the encryption code probably doesn't work, but I'm trying to move it to the side at least. John Ferlan (3): util: Clean up code formatting in virstorageencryption storage: Split out setting default secret for encryption storage: Split out a helper for encryption checks src/storage/storage_backend.c | 80 ++++++++++++++++++++++++++-------------- src/storage/storage_backend_fs.c | 79 ++++++++++++++++++++++++--------------- src/util/virstorageencryption.c | 58 ++++++++++++++--------------- 3 files changed, 128 insertions(+), 89 deletions(-) -- 2.5.5
Bring style more in line with more recent code. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/virstorageencryption.c | 58 +++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c index ec4a8cb..8105158 100644 --- a/src/util/virstorageencryption.c +++ b/src/util/virstorageencryption.c @@ -112,8 +112,7 @@ virStorageEncryptionSecretParse(xmlXPathContextPtr ctxt, { xmlNodePtr old_node; virStorageEncryptionSecretPtr ret; - char *type_str; - int type; + char *type_str = NULL; char *uuidstr = NULL; if (VIR_ALLOC(ret) < 0) @@ -122,25 +121,21 @@ virStorageEncryptionSecretParse(xmlXPathContextPtr ctxt, old_node = ctxt->node; ctxt->node = node; - type_str = virXPathString("string(./@type)", ctxt); - if (type_str == NULL) { + if (!(type_str = virXPathString("string(./@type)", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("unknown volume encryption secret type")); goto cleanup; } - type = virStorageEncryptionSecretTypeFromString(type_str); - if (type < 0) { + + if ((ret->type = virStorageEncryptionSecretTypeFromString(type_str)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown volume encryption secret type %s"), type_str); - VIR_FREE(type_str); goto cleanup; } VIR_FREE(type_str); - ret->type = type; - uuidstr = virXPathString("string(./@uuid)", ctxt); - if (uuidstr) { + if ((uuidstr = virXPathString("string(./@uuid)", ctxt))) { if (virUUIDParse(uuidstr, ret->uuid) < 0) { virReportError(VIR_ERR_XML_ERROR, _("malformed volume encryption uuid '%s'"), @@ -157,6 +152,7 @@ virStorageEncryptionSecretParse(xmlXPathContextPtr ctxt, return ret; cleanup: + VIR_FREE(type_str); virStorageEncryptionSecretFree(ret); VIR_FREE(uuidstr); ctxt->node = old_node; @@ -168,46 +164,48 @@ virStorageEncryptionParseXML(xmlXPathContextPtr ctxt) { xmlNodePtr *nodes = NULL; virStorageEncryptionPtr ret; - char *format_str; - int format, n; + char *format_str = NULL; + int n; size_t i; if (VIR_ALLOC(ret) < 0) return NULL; - format_str = virXPathString("string(./@format)", ctxt); - if (format_str == NULL) { + if (!(format_str = virXPathString("string(./@format)", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("unknown volume encryption format")); goto cleanup; } - format = virStorageEncryptionFormatTypeFromString(format_str); - if (format < 0) { + + if ((ret->format = + virStorageEncryptionFormatTypeFromString(format_str)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown volume encryption format type %s"), format_str); - VIR_FREE(format_str); goto cleanup; } VIR_FREE(format_str); - ret->format = format; - n = virXPathNodeSet("./secret", ctxt, &nodes); - if (n < 0) + if ((n = virXPathNodeSet("./secret", ctxt, &nodes)) < 0) goto cleanup; - if (n != 0 && VIR_ALLOC_N(ret->secrets, n) < 0) - goto cleanup; - ret->nsecrets = n; - for (i = 0; i < n; i++) { - ret->secrets[i] = virStorageEncryptionSecretParse(ctxt, nodes[i]); - if (ret->secrets[i] == NULL) + + if (n > 0) { + if (VIR_ALLOC_N(ret->secrets, n) < 0) goto cleanup; + ret->nsecrets = n; + + for (i = 0; i < n; i++) { + if (!(ret->secrets[i] = + virStorageEncryptionSecretParse(ctxt, nodes[i]))) + goto cleanup; + } + VIR_FREE(nodes); } - VIR_FREE(nodes); return ret; cleanup: + VIR_FREE(format_str); VIR_FREE(nodes); virStorageEncryptionFree(ret); return NULL; @@ -248,8 +246,7 @@ virStorageEncryptionSecretFormat(virBufferPtr buf, const char *type; char uuidstr[VIR_UUID_STRING_BUFLEN]; - type = virStorageEncryptionSecretTypeToString(secret->type); - if (!type) { + if (!(type = virStorageEncryptionSecretTypeToString(secret->type))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unexpected volume encryption secret type")); return -1; @@ -268,8 +265,7 @@ virStorageEncryptionFormat(virBufferPtr buf, const char *format; size_t i; - format = virStorageEncryptionFormatTypeToString(enc->format); - if (!format) { + if (!(format = virStorageEncryptionFormatTypeToString(enc->format))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unexpected encryption format")); return -1; -- 2.5.5
Split the qcow setting of encryption secrets into a helper Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_fs.c | 79 +++++++++++++++++++++++++--------------- 1 file changed, 49 insertions(+), 30 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 45474cb..a11df36 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1280,6 +1280,51 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, } +/* virStorageBackendFileSystemLoadDefaultSecrets: + * @conn: Connection pointer to fetch secret + * @vol: volume being refreshed + * + * If the volume had a QCOW secret generated, we need to regenerate the + * secret + * + * Returns 0 if no secret or secret setup was successful, + * -1 on failures w/ error message set + */ +static int +virStorageBackendFileSystemLoadDefaultSecrets(virConnectPtr conn, + virStorageVolDefPtr vol) +{ + virSecretPtr sec; + virStorageEncryptionSecretPtr encsec = NULL; + + /* Only necessary for qcow format */ + if (!vol->target.encryption || + vol->target.encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW || + vol->target.encryption->nsecrets != 0) + return 0; + + if (!(sec = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_VOLUME, + vol->target.path))) + return 0; + + if (VIR_ALLOC_N(vol->target.encryption->secrets, 1) < 0 || + VIR_ALLOC(encsec) < 0) { + VIR_FREE(vol->target.encryption->secrets); + virObjectUnref(sec); + return -1; + } + + vol->target.encryption->nsecrets = 1; + vol->target.encryption->secrets[0] = encsec; + + encsec->type = VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE; + virSecretGetUUID(sec, encsec->uuid); + virObjectUnref(sec); + + return 0; +} + + /** * Update info about a volume's capacity/allocation */ @@ -1291,39 +1336,13 @@ virStorageBackendFileSystemVolRefresh(virConnectPtr conn, int ret; /* Refresh allocation / capacity / permissions info in case its changed */ - ret = virStorageBackendUpdateVolInfo(vol, false, - VIR_STORAGE_VOL_FS_OPEN_FLAGS, 0); - if (ret < 0) + if ((ret = virStorageBackendUpdateVolInfo(vol, false, + VIR_STORAGE_VOL_FS_OPEN_FLAGS, + 0)) < 0) return ret; /* Load any secrets if possible */ - if (vol->target.encryption && - vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_QCOW && - vol->target.encryption->nsecrets == 0) { - virSecretPtr sec; - virStorageEncryptionSecretPtr encsec = NULL; - - sec = virSecretLookupByUsage(conn, - VIR_SECRET_USAGE_TYPE_VOLUME, - vol->target.path); - if (sec) { - if (VIR_ALLOC_N(vol->target.encryption->secrets, 1) < 0 || - VIR_ALLOC(encsec) < 0) { - VIR_FREE(vol->target.encryption->secrets); - virObjectUnref(sec); - return -1; - } - - vol->target.encryption->nsecrets = 1; - vol->target.encryption->secrets[0] = encsec; - - encsec->type = VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE; - virSecretGetUUID(sec, encsec->uuid); - virObjectUnref(sec); - } - } - - return 0; + return virStorageBackendFileSystemLoadDefaultSecrets(conn, vol); } static int -- 2.5.5
Split out a helper from virStorageBackendCreateQemuImgCmdFromVol to check the encryption - soon a new encryption sheriff will be patroling and that'll mean all sorts of new checks. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend.c | 80 ++++++++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 28 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 3a23cd7..ef74b50 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1010,6 +1010,54 @@ virStorageBackendCreateQemuImgOpts(char **opts, return -1; } + +/* virStorageBackendCreateQemuImgCheckEncryption: + * @format: format of file found + * @conn: pointer to connection + * @vol: pointer to volume def + * + * xxx + * + * Returns 0 on success, -1 on failure w/ error set + */ +static int +virStorageBackendCreateQemuImgCheckEncryption(int format, + const char *type, + virConnectPtr conn, + virStorageVolDefPtr vol) +{ + virStorageEncryptionPtr enc; + + if (format == VIR_STORAGE_FILE_QCOW || format == VIR_STORAGE_FILE_QCOW2) { + enc = vol->target.encryption; + if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW && + enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported volume encryption format %d"), + vol->target.encryption->format); + return -1; + } + if (enc->nsecrets > 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("too many secrets for qcow encryption")); + return -1; + } + if (enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT || + enc->nsecrets == 0) { + if (virStorageGenerateQcowEncryption(conn, vol) < 0) + return -1; + } + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("qcow volume encryption unsupported with " + "volume format %s"), type); + return -1; + } + + return 0; +} + + /* Create a qemu-img virCommand from the supplied binary path, * volume definitions and imgformat */ @@ -1133,35 +1181,11 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, } } - if (info.encryption) { - virStorageEncryptionPtr enc; + if (info.encryption && + virStorageBackendCreateQemuImgCheckEncryption(info.format, type, + conn, vol) < 0) + return NULL; - if (info.format != VIR_STORAGE_FILE_QCOW && - info.format != VIR_STORAGE_FILE_QCOW2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("qcow volume encryption unsupported with " - "volume format %s"), type); - return NULL; - } - enc = vol->target.encryption; - if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW && - enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported volume encryption format %d"), - vol->target.encryption->format); - return NULL; - } - if (enc->nsecrets > 1) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("too many secrets for qcow encryption")); - return NULL; - } - if (enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT || - enc->nsecrets == 0) { - if (virStorageGenerateQcowEncryption(conn, vol) < 0) - return NULL; - } - } /* Size in KB */ info.size_arg = VIR_DIV_UP(vol->target.capacity, 1024); -- 2.5.5
On 05/26/2016 05:52 PM, John Ferlan wrote:
While working through adding luks support for libvirt, I have a few patches that aren't really germane to adding support and rather than drop a large patch series when I'm done - I figured I'd post a few adjustments.
In the long run the encryption code probably doesn't work, but I'm trying to move it to the side at least.
John Ferlan (3): util: Clean up code formatting in virstorageencryption storage: Split out setting default secret for encryption storage: Split out a helper for encryption checks
src/storage/storage_backend.c | 80 ++++++++++++++++++++++++++-------------- src/storage/storage_backend_fs.c | 79 ++++++++++++++++++++++++--------------- src/util/virstorageencryption.c | 58 ++++++++++++++--------------- 3 files changed, 128 insertions(+), 89 deletions(-)
Going to post an update soon in the form of a new series... Going to add http://www.redhat.com/archives/libvir-list/2016-May/msg02147.html to the end of this plus add 3 more qemu-img patches. So, drop this series... John
participants (1)
-
John Ferlan