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

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(a)redhat.com&gt; > --- > 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.

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

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

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

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