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@xxxxxxxxxx> wrote:
> >
> > On Tue, Sep 10, 2024 at 11:05:59AM +0400, marcandre.lureau@xxxxxxxxxx wrote:
> > >From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> > >
> > >Learn to parse a directory for the TPM state.
> > >
> > >Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> > >---
> > > 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.
That isn't a valid comparison, because there is a "type" attribute on
the parent <disk> element that dictates what schema branch is followed
for <source>.
The TPM *must* have an equivalent attribute somewhere that dictates
the schema branch for <source>.
This means either we put "type=file|dir" on the <source>, or we invent
a new "source=file|dir" on the parent <tpm> element. I tend to favour
the former, since "type" is the common attribute name we use for
schema branch discriminators.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
