[libvirt] [PATCH v2 04/15] util: Add 'usage' for encryption

[John Ferlan]

On 06/24/2016 09:25 AM, Peter Krempa wrote:
> On Thu, Jun 23, 2016 at 13:29:00 -0400, John Ferlan wrote:
>> In order to use more common code and set up for a future type, modify the
>> encryption secret to allow the "usage" attribute or the "uuid" attribute
>> to define the secret. The "usage" in the case of a volume secret would be
>> the path to the volume.
>>
>> This code will make use of the virSecretLookup{Parse|Format}Secret common code.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  docs/formatstorageencryption.html.in               | 15 ++++++---
>>  docs/schemas/storagecommon.rng                     | 11 +++++--
>>  src/qemu/qemu_process.c                            | 13 +++-----
>>  src/storage/storage_backend.c                      |  3 +-
>>  src/storage/storage_backend_fs.c                   |  3 +-
>>  src/util/virstorageencryption.c                    | 26 ++++++----------
>>  src/util/virstorageencryption.h                    |  3 +-
>>  .../qemuxml2argv-encrypted-disk-usage.args         | 24 +++++++++++++++
>>  .../qemuxml2argv-encrypted-disk-usage.xml          | 32 +++++++++++++++++++
>>  tests/qemuxml2argvtest.c                           |  1 +
>>  .../qemuxml2xmlout-encrypted-disk-usage.xml        | 36 ++++++++++++++++++++++
>>  tests/qemuxml2xmltest.c                            |  1 +
>>  12 files changed, 132 insertions(+), 36 deletions(-)
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk-usage.args
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk-usage.xml
>>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-encrypted-disk-usage.xml
>>
>> diff --git a/docs/formatstorageencryption.html.in b/docs/formatstorageencryption.html.in
>> index 04c3346..fae86eb 100644
>> --- a/docs/formatstorageencryption.html.in
>> +++ b/docs/formatstorageencryption.html.in
>> @@ -25,10 +25,17 @@
>>      <p>
>>        The <code>encryption</code> tag can currently contain a sequence of
>>        <code>secret</code> tags, each with mandatory attributes <code>type</code>
>> -      and <code>uuid</code>.  The only currently defined value of
>> -      <code>type</code> is <code>passphrase</code>.  <code>uuid</code>
>> -      refers to a secret known to libvirt.  libvirt can use a secret value
>> -      previously set using <code>virSecretSetValue()</code>, or, if supported
>> +      and either <code>uuid</code> or
>> +      <code>usage</code> (<span class="since">since 2.0.0</span>).
>> +      The only currently defined value of
>> +      <code>type</code> is <code>passphrase</code>. The <code>uuid</code>
>> +      refers to a secret known to libvirt by it's "uuid" value (from the
>> +      output of a <code>virsh secret-list</code>.  The <code>usage</code>
> 
> I don't think it's necessary to describe how to use virsh here.
> 

Ok - removed.

>> +      is the path to the volume as it appears in the volume
> 
> This looks wrong. Passprhase type secrets list 'name' or 'id' as usage
> not the path. This contradicts changes in previous patch.
> 

Looks weird, but that's how the matching is done... The contents of the
"usage" field are matched against the secret's <usage>'s subelement field.


>> +      <code>source</code> element. A secret value can be set in libvirt by
>> +      using either <code>virsh secret-set-value</code> or the
> 
> Again. Mentioning the API is good enoguh.
> 
>> +      <a href="html/libvirt-libvirt-secret.html#virSecretSetValue">
>> +      <code>virSecretSetValue</code></a> API. Alternatively, if supported
>>        by the particular volume format and driver, automatically generate a
>>        secret value at the time of volume creation, and store it using the
>>        specified <code>uuid</code>.
> 

Does the following work?

  The <code>encryption</code> tag can currently contain a sequence of
  <code>secret</code> tags, each with mandatory attributes <code>type</code>
  and either <code>uuid</code> or <code>usage</code>
  (<span class="since">since 2.0.0</span>). The only currently defined
  value of <code>type</code> is <code>passphrase</code>. The
  <code>uuid</code> is "uuid" of the <code>secret</code> while
  <code>usage</code> is the value "usage" subelement field.
  A secret value can be set in libvirt by the
  <a href="html/libvirt-libvirt-secret.html#virSecretSetValue">
  <code>virSecretSetValue</code></a> API. Alternatively, if supported


> 
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index 63da600..7d56ec8 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
> 
> [...]
> 
>> @@ -416,14 +416,9 @@ qemuProcessGetVolumeQcowPassphrase(virConnectPtr conn,
>>          goto cleanup;
>>      }
>>  
>> -    secret = conn->secretDriver->secretLookupByUUID(conn,
>> -                                                    enc->secrets[0]->uuid);
>> -    if (secret == NULL)
>> -        goto cleanup;
>> -    data = conn->secretDriver->secretGetValue(secret, &size, 0,
>> -                                              VIR_SECRET_GET_VALUE_INTERNAL_CALL);
>> -    virObjectUnref(secret);
>> -    if (data == NULL)
>> +    if (virSecretGetSecretString(conn, &enc->secrets[0]->seclookupdef,
>> +                                 VIR_SECRET_USAGE_TYPE_VOLUME,
> 
> Wrong type.
> 

Huh?  This is the "old" VOLUME method not the new PASSPHRASE method. So
VOLUME here is correct.

John

>> +                                 &data, &size) < 0)
>>          goto cleanup;
>>  
>>      if (memchr(data, '\0', size) != NULL) {
> 
>