qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Max Reitz <mreitz@redhat.com>, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Luiz Capitulino <lcapitulino@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 2/4] hmp: Use blockdev-change-medium for change command
Date: Fri, 05 Dec 2014 06:45:58 -0700	[thread overview]
Message-ID: <5481B716.1000803@redhat.com> (raw)
In-Reply-To: <5481B2DC.40402@redhat.com>

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

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


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

  reply	other threads:[~2014-12-05 13:46 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-05 10:08 [Qemu-devel] [PATCH v2 0/4] blockdev: Add blockdev-change-medium with read-only option Max Reitz
2014-12-05 10:08 ` [Qemu-devel] [PATCH v2 1/4] qmp: Introduce blockdev-change-medium Max Reitz
2014-12-05 13:10   ` Eric Blake
2014-12-05 13:18     ` Max Reitz
2014-12-05 13:25       ` Eric Blake
2014-12-05 10:08 ` [Qemu-devel] [PATCH v2 2/4] hmp: Use blockdev-change-medium for change command Max Reitz
2014-12-05 13:24   ` Eric Blake
2014-12-05 13:27     ` Max Reitz
2014-12-05 13:45       ` Eric Blake [this message]
2014-12-05 10:08 ` [Qemu-devel] [PATCH v2 3/4] blockdev: Add read-only option to blockdev-change-medium Max Reitz
2014-12-05 13:28   ` Eric Blake
2014-12-05 10:08 ` [Qemu-devel] [PATCH v2 4/4] hmp: Add read-only option to change command Max Reitz
2014-12-05 13:49   ` Eric Blake
2014-12-05 13:14 ` [Qemu-devel] [PATCH v2 0/4] blockdev: Add blockdev-change-medium with read-only option Eric Blake
2014-12-05 13:25   ` Max Reitz
2014-12-05 13:31 ` Markus Armbruster
2014-12-05 13:47   ` Max Reitz
2014-12-05 14:07     ` Eric Blake
2014-12-05 14:21       ` Max Reitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5481B716.1000803@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).