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

[Eric Blake]

On 12/05/2014 06:27 AM, Max Reitz wrote:
>> 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).
> 
> I'd rather put all of the encryption stuff back until we have figured
> out how to fix it (that is, if we want to fix it at all)...

Yeah, I totally understand (no point adding encryption unless we add it
right).  Which is why it doesn't hold up this series.  But I _do_ want
to make sure we're thinking through the design, and not painting
ourselves into a corner; so at this point, I guess I'm just looking for
confirmation on my thought experiment that we are extensible enough that
we can support encryption at the QMP level down the road by adding
optional parameters, regardless of how we map that at the HMP level.


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

signature.asc
Description: OpenPGP digital signature