Revisited this one on review of v2, replying here for context.
Luiz Capitulino <lcapitul...@redhat.com> writes:
> On Thu, 02 Aug 2012 13:35:54 +0200
> Markus Armbruster <arm...@redhat.com> wrote:
>
>> Luiz Capitulino <lcapitul...@redhat.com> writes:
>>
>> > Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
>> > ---
>> > block.c | 1 +
>> > qapi-schema.json | 7 +++++--
>> > 2 files changed, 6 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/block.c b/block.c
>> > index b38940b..9c113b8 100644
>> > --- a/block.c
>> > +++ b/block.c
>> > @@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp)
>> > info->value->inserted->ro = bs->read_only;
>> > info->value->inserted->drv = g_strdup(bs->drv->format_name);
>> > info->value->inserted->encrypted = bs->encrypted;
>> > + info->value->inserted->valid_encryption_key = bs->valid_key;
>> > if (bs->backing_file[0]) {
>> > info->value->inserted->has_backing_file = true;
>> > info->value->inserted->backing_file =
>> > g_strdup(bs->backing_file);
>> > diff --git a/qapi-schema.json b/qapi-schema.json
>> > index bc55ed2..1b2d7f5 100644
>> > --- a/qapi-schema.json
>> > +++ b/qapi-schema.json
>> > @@ -400,6 +400,8 @@
>> > #
>> > # @encrypted: true if the backing device is encrypted
>> > #
>> > +# @valid_encryption_key: true if a valid encryption key has been set
>> > +#
>> > # @bps: total throughput limit in bytes per second is specified
>> > #
>> > # @bps_rd: read throughput limit in bytes per second is specified
>> > @@ -419,8 +421,9 @@
>> > { 'type': 'BlockDeviceInfo',
>> > 'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
>> > '*backing_file': 'str', 'encrypted': 'bool',
>> > - 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
>> > - 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
>> > + 'valid_encryption_key': 'bool', 'bps': 'int',
>> > + 'bps_rd': 'int', 'bps_wr': 'int', 'iops': 'int',
>> > + 'iops_rd': 'int', 'iops_wr': 'int'} }
>> >
>> > ##
>> > # @BlockDeviceIoStatus:
>>
>> BlockDeviceInfo is API, isn't it?
>
> Yes.
>
>> Note that bs->valid_key currently implies bs->encrypted. bs->valid_key
>> && !bs->encrypted is impossible. Should we make valid_encryption_key
>> only available when encrypted?
>
> I don't think so. It's a bool, so it's ok for it to be false when
> encrypted is false.
What bothers me is encrypted=false, valid_encryption_key=true.
>> valid_encryption_key is a bit long for my taste. Yours may be
>> different.
>
> We should choose more descriptive and self-documenting names for the
> protocol. Besides, I can't think of anything shorter that won't get
> cryptic.
>
> Suggestions are always welcome though :)
valid_encryption_key sounds like the value is the valid key.
got_crypt_key? Also avoids "valid". Good, because current encrypted
formats don't actually validate the key; they happily accept any key.
GIGO. In theory, you can trash a disk that way. In practice, we can
hope the guest will refuse to touch the disk, because it can't recognize
partition table / filesystems.
