Re: [Qemu-devel] [PATCH v2 2/4] hmp: Use blockdev-change-medium for change command

[Eric Blake <eblake@redhat.com>]

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

On 12/05/2014 03:08 AM, Max Reitz wrote:
> Use separate code paths for the two overloaded functions of the 'change'
> HMP command, and invoke the 'blockdev-change-medium' QMP command if used
> on a block device (by calling qmp_blockdev_change_medium()).
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  hmp.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)

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

> +    } else {
> +        qmp_blockdev_change_medium(device, target, !!arg, arg, &err);
> +        if (err &&
> +            error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
> +            error_free(err);
> +            monitor_read_block_device_key(mon, device, NULL, NULL);
>              return;

Hmm. Not a problem with this patch (which is just reorganizing control
flow), but a possible design issue.  The old 'change' QMP command cannot
provide the encryption code, hence it cannot succeed when changing to an
encrypted media.  But now that we are adding a new QMP command, we could
possibly rectify that matter.  However, the way I'm envisioning that is
that blockdev-change-medium gains an optional parameter for encryption
key.  Then the HMP command gains a new optional parameter for specifying
the key, and the logic flow would look a bit more like:

qmp_blockdev_change_medium(device, target, !!arg, arg,
                           !!key, key, &err);
if (err && error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED &&
    !key) {
    error_free(err);
    key = prompt_for_key_from_monitor;
    qmp_blockdev_change_medium(device, target, !!arg, arg,
                               true, key, &err);
}

which would mean that a QMP command can now supply the key in one
command, rather than HMP being the only way to supply a password because
of how it does a two-step process (that is,
monitor_read_block_device_key isn't really accessible from QMP).

-- 
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: 539 bytes --]