On 2014-12-05 at 14:24, Eric Blake wrote:
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 <mre...@redhat.com>
---
  hmp.c | 27 +++++++++++++++------------
  1 file changed, 15 insertions(+), 12 deletions(-)
Reviewed-by: Eric Blake <ebl...@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).

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)...

Max

Reply via email to