Re: [PATCH v3 3/5] schema: add TPM emulator <source type='file' path='..'>

On 10/14/24 5:19 AM, Daniel P. Berrangé wrote: > On Fri, Oct 11, 2024 at 10:46:58AM -0400, Stefan Berger wrote: > > > > > > On 10/11/24 10:32 AM, Marc-André Lureau wrote: > > > Hi > > > > > > On Fri, Oct 11, 2024 at 6:17 PM Stefan Berger <stefanb(a)linux.ibm.com&gt; wrote: > > > > > > > > > > > > > > > > On 10/11/24 10:10 AM, Marc-André Lureau wrote: > > > > > Hi > > > > > > > > > > On Fri, Oct 11, 2024 at 5:49 PM Stefan Berger <stefanb(a)linux.ibm.com&gt; wrote: > > > > > > > > > > > > > > > > > > > > > > > > On 10/4/24 9:32 AM, marcandre.lureau(a)redhat.com wrote: > > > > > > > From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; > > > > > > > > > > > > > > Learn to parse a file path for the TPM state. > > > > > > > > > > > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; > > > > > > > --- > > > > > > > docs/formatdomain.rst | 19 ++++++++++++++ > > > > > > > src/conf/domain_conf.c | 28 +++++++++++++++++++++ > > > > > > > src/conf/domain_conf.h | 9 +++++++ > > > > > > > src/conf/schemas/domaincommon.rng | 14 +++++++++++ > > > > > > > tests/qemuxmlconfdata/tpm-emulator-tpm2.xml | 1 + > > > > > > > 5 files changed, 71 insertions(+) > > > > > > > > > > > > > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst > > > > > > > index 4336cff3ac..992bb98730 100644 > > > > > > > --- a/docs/formatdomain.rst > > > > > > > +++ b/docs/formatdomain.rst > > > > > > > @@ -8173,6 +8173,25 @@ Example: usage of the TPM Emulator > > > > > > > The default version used depends on the combination of hypervisor, guest > > > > > > > architecture, TPM model and backend. > > > > > > > > > > > > > > +``source`` > > > > > > > + The ``source`` element specifies the location of the TPM state storage . This > > > > > > > + element only works with the ``emulator`` backend. > > > > > > > + > > > > > > > + If not specified, the storage configuration is left to libvirt discretion. > > > > > > > + > > > > > > > + This element requires that swtpm v0.7 or later is installed. > > > > > > > + > > > > > > > + The following attributes are supported: > > > > > > > + > > > > > > > + ``type`` > > > > > > > + The type of storage. It's possible to provide "file" to utilize a single > > > > > > > + file or block device where the TPM state will be stored. > > > > > > > + > > > > > > > + ``path`` > > > > > > > + The path to the TPM state storage. > > > > > > > > > > > > The file backend of swtpm does not do the locking similar to what the > > > > > > dir backend does because those who added the file backend didn't > > > > > > need/want it. If we now give full control to the path of the TPM state > > > > > > file to the user via the domain XML then whose fault is it if two VMs > > > > > > use the same path to a file backend and stomp on the TPM state file? Is > > > > > > it the fault of the user because of how he defined the path in the XMLs? > > > > > > > > > > Imho, it's desirable to have a similar locking behaviour regardless of > > > > > the backend and prevent users for mistakenly using the same file. > > > > > > > > We will only be able to support the locking with an option on the > > > > command line for swtpm (refelected by a new capability verb) and support > > > > this series here once that has become available with a new version of > > > > swtpm. Otherwise I would avoid giving full control to the path to the > > > > users but let libvirt choose a per-VM unique name for the state file. > > > > > > The use-case is to let the user define a specific block device path. > > > > Why would they store it on a block device rather than a file system? > > If they want to make the storage available to multiple hosts, then using > a block device is simpler, as most filesystems are unsafe to concurrently > expose in multiple hosts. You then need n block devices for n swtpm instances?