[libvirt] [PATCH 19/19] qemu: Add luks support for domain disk

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:58 -0400, John Ferlan wrote:
> Generate the luks command line using the AES secret key to encrypt the
> luks secret. A luks secret object will be in addition to a an AES secret.
> 
> Add tests for sample output
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/qemu/qemu_command.c                            | 12 ++++++--
>  src/qemu/qemu_domain.c                             | 19 ++++++++++--
>  .../qemuxml2argv-luks-disk-cipher.args             | 36 ++++++++++++++++++++++
>  .../qemuxml2argvdata/qemuxml2argv-luks-disks.args  | 36 ++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           | 11 ++++++-
>  5 files changed, 109 insertions(+), 5 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args

Note that this won't work with disk hotplug until you add code that will
add the secret objects too.

Also I've noticed that RBD hotplug is now broken too due to the same
reason. The code that selects whether to use plaintext password or pass
it via the secret object does not check whether it's called on the
hotplug path and thus uses the secret object.

The secret object is not added using the monitor though. You'll need to
fix that first and if you opt for the correct approach this will then
work automagically.

Looks okay but I can't ACK this with hotplug not working.

Also please note that on hot-unplug you need to remove the secrets as
you'll possibly be adding a secret with the same name (Since alias will
be reused)

> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 490260f..7062c17 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1103,6 +1103,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
>      int actualType = virStorageSourceGetActualType(disk->src);
>      qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>      qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo;
> +    qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo;
>      bool emitDeviceSyntax = qemuDiskBusNeedsDeviceArg(disk->bus);
>  
>      if (idx < 0) {
> @@ -1237,10 +1238,13 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
>          qemuBufferEscapeComma(&opt, source);
>          virBufferAddLit(&opt, ",");
>  
> -        if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
> +        if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES)
>              virBufferAsprintf(&opt, "password-secret=%s,",
>                                secinfo->s.aes.alias);
> -        }
> +
> +        if (encinfo)
> +            virQEMUBuildLuksOpts(&opt, disk->src->encryption,
> +                                 encinfo->s.aes.alias);

This wrapper is not really useful here. It only adds "key-secret=" all
the other options are necessary only if creating the volume.

>  
>          if (disk->src->format > 0 &&
>              disk->src->type != VIR_STORAGE_TYPE_DIR)
> @@ -1920,6 +1924,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
>          virDomainDiskDefPtr disk = def->disks[i];
>          qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>          qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo;
> +        qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo;
>  
>          /* PowerPC pseries based VMs do not support floppy device */
>          if ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) &&
> @@ -1949,6 +1954,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
>          if (qemuBuildDiskSecinfoCommandLine(cmd, secinfo) < 0)
>              return -1;
>  
> +        if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0)
> +            return -1;
> +
>          virCommandAddArg(cmd, "-drive");
>  
>          optstr = qemuBuildDriveStr(disk,
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index c288fa0..fb3e91f 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c

[...]

> @@ -1026,7 +1028,6 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
>           src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
>  
>          virSecretUsageType secretUsageType;
> -        qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>  
>          if (VIR_ALLOC(secinfo) < 0)
>              return -1;
> @@ -1044,6 +1045,20 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
>          diskPriv->secinfo = secinfo;
>      }
>  
> +    if (conn && !virStorageSourceIsEmpty(src) &&

This part is the same with the block above. It may be worth extracting
it ...

> +        src->encryption && src->format == VIR_STORAGE_FILE_LUKS) {
> +
> +        if (VIR_ALLOC(secinfo) < 0)
> +            return -1;
> +
> +        if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias,
> +                                  VIR_SECRET_USAGE_TYPE_KEY, NULL,
> +                                  &src->encryption->secrets[0]->secdef) < 0)
> +            goto error;
> +
> +        diskPriv->encinfo = secinfo;
> +    }
> +
>      return 0;
>  
>   error: