On Tue, Jul 21, 2009 at 01:12:00PM +0200, Miloslav Trma?? wrote:
> Supports only virStorageVolCreateXML, not virStorageVolCreateXMLFrom.
>
> Curiously, qemu-img does not need the passphrase for anything to create
> an encrypted volume. This implementation is sufficient for the qcow2
> format, and for other formats when all encryption parameters are
> pre-specified.
>
> The qcow2 passphrase is stored in libvirt only during the volume
> creation operation, and automatically deleted afterwards. Other users
> might be able to query the volume properties while the volume is being
> created, but virStorageVolGetXMLDesc() won't reveal the passphrase to
> them.
>
> An earlier patch description has mentioned the possibility of a libvirt
> user specifying only the encryption format (or not even the format),
> letting libvirt to choose the parameters and return them. This would
> require libvirt interface extensions, and it is not supported by this
> patch.
> ---
> src/storage_backend.c | 41 +++++++++++++++++++++++++++++++++++++++--
> src/storage_backend_disk.c | 7 +++++++
> src/storage_backend_fs.c | 7 +++++++
> src/storage_backend_logical.c | 7 +++++++
> src/storage_driver.c | 4 ++++
> 5 files changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/src/storage_backend.c b/src/storage_backend.c
> index 1664804..42c59ad 100644
> --- a/src/storage_backend.c
> +++ b/src/storage_backend.c
> @@ -246,6 +246,13 @@ virStorageBackendCreateRaw(virConnectPtr conn,
> unsigned long long remain;
> char *buf = NULL;
>
> + if (vol->encryption != NULL) {
> + virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
> + "%s", _("storage pool does not support encrypted "
> + "volumes"));
> + return -1;
> + }
> +
> if ((fd = open(vol->target.path, O_RDWR | O_CREAT | O_EXCL,
> vol->target.perms.mode)) < 0) {
> virReportSystemError(conn, errno,
> @@ -346,15 +353,17 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
> NULL;
>
> const char **imgargv;
> + /* The extra NULL field is for indicating encryption (-e). */
> const char *imgargvnormal[] = {
> NULL, "create",
> "-f", type,
> vol->target.path,
> size,
> NULL,
> + NULL
> };
> /* Extra NULL fields are for including "backingType" when using
> - * kvm-img. It's -F backingType
> + * kvm-img (-F backingType), and for indicating encryption (-e).
> */
> const char *imgargvbacking[] = {
> NULL, "create",
> @@ -364,6 +373,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
> size,
> NULL,
> NULL,
> + NULL,
> NULL
> };
> const char *convargv[] = {
> @@ -417,6 +427,22 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
> }
> }
>
> + if (vol->encryption != NULL) {
> + if (vol->encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW) {
> + virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
> + _("unsupported volume encryption format %d"),
> + vol->encryption->format);
> + return -1;
> + }
> + if (vol->target.format != VIR_STORAGE_VOL_FILE_QCOW &&
> + vol->target.format != VIR_STORAGE_VOL_FILE_QCOW2) {
> + virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
> + _("qcow volume encryption unsupported with "
> + "volume format %s"), type);
> + return -1;
> + }
> + }
> +
> if ((create_tool = virFindFileInPath("kvm-img")) != NULL)
> use_kvmimg = 1;
> else if ((create_tool = virFindFileInPath("qemu-img")) != NULL)
> @@ -437,11 +463,16 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
> imgargvbacking[7] = backingType;
> imgargvbacking[8] = vol->target.path;
> imgargvbacking[9] = size;
> - }
> + if (vol->encryption != NULL)
> + imgargvbacking[10] = "-e";
> + } else if (vol->encryption != NULL)
> + imgargvbacking[8] = "-e";
> imgargv = imgargvbacking;
> } else {
> imgargvnormal[0] = create_tool;
> imgargv = imgargvnormal;
> + if (vol->encryption != NULL)
> + imgargv[6] = "-e";
> }
>
>
> @@ -490,6 +521,12 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn,
> "qcow-create"));
> return -1;
> }
> + if (vol->encryption != NULL) {
> + virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
> + "%s", _("encrypted volumes not supported with "
> + "qcow-create"));
> + return -1;
> + }
>
> /* Size in MB - yes different units to qemu-img :-( */
> snprintf(size, sizeof(size), "%llu", vol->capacity/1024/1024);
> diff --git a/src/storage_backend_disk.c b/src/storage_backend_disk.c
> index ae2acae..8f4f0ed 100644
> --- a/src/storage_backend_disk.c
> +++ b/src/storage_backend_disk.c
> @@ -557,6 +557,13 @@ virStorageBackendDiskCreateVol(virConnectPtr conn,
> NULL
> };
>
> + if (vol->encryption != NULL) {
> + virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
> + "%s", _("storage pool does not support encrypted "
> + "volumes"));
> + return -1;
> + }
> +
> if (virStorageBackendDiskPartFormat(conn, pool, vol, partFormat) != 0) {
> return -1;
> }
> diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
> index 0219eb6..b43daef 100644
> --- a/src/storage_backend_fs.c
> +++ b/src/storage_backend_fs.c
> @@ -1070,6 +1070,13 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn,
> int tool_type;
>
> if (inputvol) {
> + if (vol->encryption != NULL) {
> + virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
> + "%s", _("storage pool does not support "
> + "building encrypted volumes from "
> + "other volumes"));
> + return -1;
> + }
> create_func = virStorageBackendGetBuildVolFromFunction(conn, vol,
> inputvol);
> if (!create_func)
> diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c
> index 6c123ae..822d321 100644
> --- a/src/storage_backend_logical.c
> +++ b/src/storage_backend_logical.c
> @@ -581,6 +581,13 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
> };
> const char **cmdargv = cmdargvnew;
>
> + if (vol->encryption != NULL) {
> + virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
> + "%s", _("storage pool does not support encrypted "
> + "volumes"));
> + return -1;
> + }
> +
> if (vol->backingStore.path) {
> cmdargv = cmdargvsnap;
> }
> diff --git a/src/storage_driver.c b/src/storage_driver.c
> index e9ecb20..98db9a0 100644
> --- a/src/storage_driver.c
> +++ b/src/storage_driver.c
> @@ -1305,6 +1305,8 @@ storageVolumeCreateXML(virStoragePoolPtr obj,
> cleanup:
> if (volobj)
> virUnrefStorageVol(volobj);
> + if (voldef)
> + virStorageEncryptionDropSecrets(voldef->encryption);
> virStorageVolDefFree(voldef);
> if (pool)
> virStoragePoolObjUnlock(pool);
> @@ -1466,6 +1468,8 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj,
> cleanup:
> if (volobj)
> virUnrefStorageVol(volobj);
> + if (newvol)
> + virStorageEncryptionDropSecrets(newvol->encryption);
> virStorageVolDefFree(newvol);
> if (pool)
> virStoragePoolObjUnlock(pool);
As per other mails, I don't think its neccessary to drop the secrets
from volumes here. At least not until we introduce keystores as an
explicit public API
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list