Hi
On Wed, Oct 2, 2024 at 11:34 AM Martin Kletzander <mkletzan(a)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(a)redhat.com> wrote:
>>
>> On Tue, Sep 10, 2024 at 11:05:59AM +0400, marcandre.lureau(a)redhat.com wrote:
>> >From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
>> >
>> >Learn to parse a directory for the TPM state.
>> >
>> >Signed-off-by: Marc-André Lureau <marcandre.lureau(a)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='...'>, but someone should decide and move on.