Hi Martin, Daniel On Wed, Oct 2, 2024 at 6:01 PM Martin Kletzander <mkletzan@redhat.com> wrote:
On Wed, Oct 02, 2024 at 03:43:22PM +0400, Marc-André Lureau wrote:
Hi
On Wed, Oct 2, 2024 at 11:34 AM Martin Kletzander <mkletzan@redhat.com> wrote:
On Wed, Oct 02, 2024 at 11:30:39AM +0400, Marc-André Lureau wrote:
Hi
On Wed, Oct 2, 2024 at 11:19 AM Martin Kletzander <mkletzan@redhat.com> wrote:
On Tue, Sep 10, 2024 at 11:05:59AM +0400, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Learn to parse a directory for the TPM state.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- docs/formatdomain.rst | 3 +++ src/conf/domain_conf.c | 13 ++++++++++--- src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 15 ++++++++++++--- tests/qemuxmlconfdata/tpm-emulator-tpm2-enc.xml | 1 + 5 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 4818113bc2..24dcc6daaa 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -8183,6 +8183,9 @@ Example: usage of the TPM Emulator
This attribute requires that swtpm v0.7 or later is installed.
+ ``dir`` + The path to the TPM state storage directory. + :since:`Since v10.8.0`
``persistent_state`` diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 18c58d16dc..d1e9e4a50c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10865,13 +10865,16 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
source_node = virXPathNode("./backend/source", ctxt); if (source_node) { - path = virXMLPropString(source_node, "file"); + if ((path = virXMLPropString(source_node, "file"))) { + def->data.emulator.storage_type = VIR_DOMAIN_TPM_STORAGE_FILE; + } else if ((path = virXMLPropString(source_node, "dir"))) { + def->data.emulator.storage_type = VIR_DOMAIN_TPM_STORAGE_DIR; + }
It would be nicer to figure out which one to use based on a value of an attribute rather than what attribute we stumble upon first. Daniel mentioned <backend type="..."> but that is already used for the type of the backend, however we could have <source type="(dir|file)"> and maybe leave the path in as is in the docs (instead of having different attribute names).
Not sure I follow. Doesnt the XML schema prevent having several <source>, or "file" and "dir" attributes (or "type" for what you suggest).
So you would rather have:
<source type="file">'/path/to/state'</source>
rather than:
<source file='/path/to/state'/>
libvirt domain for <disk> for example uses the second form. I find it more idiomatic now.
I meant <source type="(dir|file)" path="/path/to/state"/>
I don't want to bikeshed that. I prefer consistency with other <source file='...'>,
Compared to disks we have:
<source file= <source dir= <source dev= <source protocol= ... and more, based on the "type" of the device (backend).
but someone should decide and move on.
I think Daniel also suggested something similar in his review of v1:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/BVZG...
with the only difference that the <backend> already has a type attribute so it does not work precisely, but the basis of the point is similar I think.
I still don't understand what you suggest. diff --git a/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml b/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml index 4c61e2645b..6ff8404d84 100644 --- a/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml +++ b/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml @@ -28,13 +28,13 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <tpm model='tpm-tis'> - <backend type='emulator' version='2.0' debug='3'> + <backend type='emulator' version='2.0' debug='3' storage='path' > <encryption secret='b4a117f1-8af2-44a4-91b8-7f0d2d4d68a3'/> <active_pcr_banks> <sha256/> <sha512/> </active_pcr_banks> - <source file='/path/to/state'/> + <source>/path/to/state'<source/> </backend> </tpm> <audio id='1' type='none'/> It feels wrong, please enlighten me :)