Re: [Qemu-devel] [PATCH RFC 2/2] block: Drop code supporting encryption outside qemu-img

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3273 bytes --]

On 03/10/2015 11:26 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block.c                   | 30 --------------------
>  blockdev.c                | 43 +---------------------------
>  hmp-commands.hx           | 14 ---------
>  hmp.c                     | 41 ---------------------------
>  hmp.h                     |  1 -
>  hw/usb/dev-storage.c      | 26 -----------------
>  include/monitor/monitor.h |  7 -----
>  monitor.c                 | 72 -----------------------------------------------
>  qapi-schema.json          | 13 ++-------
>  qapi/block-core.json      | 42 ++-------------------------
>  qapi/common.json          |  5 +---
>  qmp-commands.hx           | 26 -----------------
>  qmp.c                     |  8 ------
>  13 files changed, 6 insertions(+), 322 deletions(-)
> 

> +++ b/qapi/block-core.json

>  ##
> -# @block_passwd:
> -#
> -# This command sets the password of a block device that has not been open
> -# with a password and requires one.
> -#
> -# The two cases where this can happen are a block device is created through
> -# QEMU's initial command line or a block device is changed through the legacy
> -# @change interface.
> -#
> -# In the event that the block device is created through the initial command
> -# line, the VM will start in the stopped state regardless of whether '-S' is
> -# used.  The intention is for a management tool to query the block devices to
> -# determine which ones are encrypted, set the passwords with this command, and
> -# then start the guest with the @cont command.
> -#
> -# Either @device or @node-name must be set but not both.
> -#
> -# @device: #optional the name of the block backend device to set the password on
> -#
> -# @node-name: #optional graph node name to set the password on (Since 2.0)
> -#
> -# @password: the password to use for the device
> -#
> -# Returns: nothing on success
> -#          If @device is not a valid block device, DeviceNotFound
> -#          If @device is not encrypted, DeviceNotEncrypted
> -#
> -# Notes:  Not all block formats support encryption and some that do are not
> -#         able to validate that a password is correct.  Disk corruption may
> -#         occur if an invalid password is specified.
> -#
> -# Since: 0.14.0
> -##
> -{ 'command': 'block_passwd', 'data': {'*device': 'str',
> -                                      '*node-name': 'str', 'password': 'str'} }

Good - removing this command means 'query-commands' will have an easy
probe for whether qemu is in the window of time where old broken
encryption could even be attempted, or when a newer (hopefully!) qemu
can support sane LUKS encryption, so that libvirt can issue sane errors
to the user telling them that their qemu cannot support encryption.

I agree with the decision of removing the existing crufty interface so
that any future additions can add in a working design from the get-go,
rather than trying to retrofit fixes for all of the confusing aspects
that you pointed out.  As such, I could live with:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]