2:05 a.m.
From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
Hi,
When swtpm capabilities reports "nvram-backend-dir", it can accepts a single
file or block device where TPM state will be stored.
--tpmstate must be backend-uri=file://.
v2:
- add <source dir='..'/> support as well (Daniel)
Related: https://issues.redhat.com/browse/CNV-35250
Marc-André Lureau (4):
util: check swtpm nvram-backend-dir capability
schema: add TPM emulator <source file='..'>
schema: add TPM emulator <source dir='..'>
qemu_tpm: handle file/block storage source
docs/formatdomain.rst | 18 +++++
src/conf/domain_conf.c | 28 +++++++
src/conf/domain_conf.h | 7 ++
src/conf/schemas/domaincommon.rng | 20 +++++
src/qemu/qemu_tpm.c | 76 +++++++++++++++----
src/util/virtpm.c | 1 +
src/util/virtpm.h | 1 +
.../qemuxmlconfdata/tpm-emulator-tpm2-enc.xml | 1 +
tests/qemuxmlconfdata/tpm-emulator-tpm2.xml | 1 +
9 files changed, 140 insertions(+), 13 deletions(-)
--
2.45.2.827.g557ae147e6
2:05 a.m.
New subject: [PATCH v2 1/4] util: check swtpm nvram-backend-dir capability
From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com>
---
src/util/virtpm.c | 1 +
src/util/virtpm.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index 81fd6166cf..84ed2f0edd 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -40,6 +40,7 @@ VIR_ENUM_IMPL(virTPMSwtpmFeature,
VIR_TPM_SWTPM_FEATURE_LAST,
"cmdarg-pwd-fd",
"cmdarg-migration",
+ "nvram-backend-dir",
);
VIR_ENUM_IMPL(virTPMSwtpmSetupFeature,
diff --git a/src/util/virtpm.h b/src/util/virtpm.h
index fb330effa8..088bb2f667 100644
--- a/src/util/virtpm.h
+++ b/src/util/virtpm.h
@@ -31,6 +31,7 @@ bool virTPMHasSwtpm(void);
typedef enum {
VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD,
VIR_TPM_SWTPM_FEATURE_CMDARG_MIGRATION,
+ VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_DIR,
VIR_TPM_SWTPM_FEATURE_LAST
} virTPMSwtpmFeature;
--
2.45.2.827.g557ae147e6
7:56 a.m.
New subject: [PATCH v2 1/4] util: check swtpm nvram-backend-dir capability
On 9/10/24 3:05 AM, marcandre.lureau(a)redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com>
Reviewed-by: Stefan Berger <stefanb(a)linux.ibm.com>
> ---
> src/util/virtpm.c | 1 +
> src/util/virtpm.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/src/util/virtpm.c b/src/util/virtpm.c
> index 81fd6166cf..84ed2f0edd 100644
> --- a/src/util/virtpm.c
> +++ b/src/util/virtpm.c
> @@ -40,6 +40,7 @@ VIR_ENUM_IMPL(virTPMSwtpmFeature,
> VIR_TPM_SWTPM_FEATURE_LAST,
> "cmdarg-pwd-fd",
> "cmdarg-migration",
> + "nvram-backend-dir",
> );
>
> VIR_ENUM_IMPL(virTPMSwtpmSetupFeature,
> diff --git a/src/util/virtpm.h b/src/util/virtpm.h
> index fb330effa8..088bb2f667 100644
> --- a/src/util/virtpm.h
> +++ b/src/util/virtpm.h
> @@ -31,6 +31,7 @@ bool virTPMHasSwtpm(void);
> typedef enum {
> VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD,
> VIR_TPM_SWTPM_FEATURE_CMDARG_MIGRATION,
> + VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_DIR,
>
> VIR_TPM_SWTPM_FEATURE_LAST
> } virTPMSwtpmFeature;
2:01 a.m.
New subject: [PATCH v2 1/4] util: check swtpm nvram-backend-dir capability
On Tue, Sep 10, 2024 at 11:05:57AM +0400, marcandre.lureau(a)redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com>
---
src/util/virtpm.c | 1 +
src/util/virtpm.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index 81fd6166cf..84ed2f0edd 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -40,6 +40,7 @@ VIR_ENUM_IMPL(virTPMSwtpmFeature,
VIR_TPM_SWTPM_FEATURE_LAST,
"cmdarg-pwd-fd",
"cmdarg-migration",
+ "nvram-backend-dir",
We should be using both nvram-backend-dir and nvram-backend-file. Even
if they were added in the same release and it does not look like it
makes sense it is there for a reason and it won't hurt, it will be more
future-proof.
);
VIR_ENUM_IMPL(virTPMSwtpmSetupFeature,
diff --git a/src/util/virtpm.h b/src/util/virtpm.h
index fb330effa8..088bb2f667 100644
--- a/src/util/virtpm.h
+++ b/src/util/virtpm.h
@@ -31,6 +31,7 @@ bool virTPMHasSwtpm(void);
typedef enum {
VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD,
VIR_TPM_SWTPM_FEATURE_CMDARG_MIGRATION,
+ VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_DIR,
VIR_TPM_SWTPM_FEATURE_LAST
} virTPMSwtpmFeature;
--
2.45.2.827.g557ae147e6
2:15 a.m.
New subject: [PATCH v2 1/4] util: check swtpm nvram-backend-dir capability
Hi
On Wed, Oct 2, 2024 at 11:01 AM Martin Kletzander <mkletzan(a)redhat.com> wrote:
On Tue, Sep 10, 2024 at 11:05:57AM +0400, marcandre.lureau(a)redhat.com wrote:
>From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
>
>Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com>
>---
> src/util/virtpm.c | 1 +
> src/util/virtpm.h | 1 +
> 2 files changed, 2 insertions(+)
>
>diff --git a/src/util/virtpm.c b/src/util/virtpm.c
>index 81fd6166cf..84ed2f0edd 100644
>--- a/src/util/virtpm.c
>+++ b/src/util/virtpm.c
>@@ -40,6 +40,7 @@ VIR_ENUM_IMPL(virTPMSwtpmFeature,
> VIR_TPM_SWTPM_FEATURE_LAST,
> "cmdarg-pwd-fd",
> "cmdarg-migration",
>+ "nvram-backend-dir",
We should be using both nvram-backend-dir and nvram-backend-file. Even
if they were added in the same release and it does not look like it
makes sense it is there for a reason and it won't hurt, it will be more
future-proof.
ok, done
> );
>
> VIR_ENUM_IMPL(virTPMSwtpmSetupFeature,
>diff --git a/src/util/virtpm.h b/src/util/virtpm.h
>index fb330effa8..088bb2f667 100644
>--- a/src/util/virtpm.h
>+++ b/src/util/virtpm.h
>@@ -31,6 +31,7 @@ bool virTPMHasSwtpm(void);
> typedef enum {
> VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD,
> VIR_TPM_SWTPM_FEATURE_CMDARG_MIGRATION,
>+ VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_DIR,
>
> VIR_TPM_SWTPM_FEATURE_LAST
> } virTPMSwtpmFeature;
>--
>2.45.2.827.g557ae147e6
>
2:05 a.m.
New subject: [PATCH v2 2/4] schema: add TPM emulator <source file='..'>
From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
Learn to parse a file path for the TPM state.
Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com>
---
docs/formatdomain.rst | 15 +++++++++++++++
src/conf/domain_conf.c | 21 +++++++++++++++++++++
src/conf/domain_conf.h | 6 ++++++
src/conf/schemas/domaincommon.rng | 11 +++++++++++
tests/qemuxmlconfdata/tpm-emulator-tpm2.xml | 1 +
5 files changed, 54 insertions(+)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 47d3e2125e..4818113bc2 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -8170,6 +8170,21 @@ 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.
+
+ The following attributes are supported:
+
+ ``path``
+ The path to the TPM state storage file.
+
+ This attribute requires that swtpm v0.7 or later is installed.
+
+ :since:`Since v10.8.0`
+
``persistent_state``
The ``persistent_state`` attribute indicates whether 'swtpm' TPM state is
kept or not when a transient domain is powered off or undefined. This
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a263612ef7..18c58d16dc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10789,6 +10789,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
int nbackends;
int nnodes;
size_t i;
+ xmlNodePtr source_node = NULL;
g_autofree char *path = NULL;
g_autofree char *secretuuid = NULL;
g_autofree char *persistent_state = NULL;
@@ -10862,6 +10863,18 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
def->data.emulator.hassecretuuid = true;
}
+ source_node = virXPathNode("./backend/source", ctxt);
+ if (source_node) {
+ path = virXMLPropString(source_node, "file");
+ if (!path) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing TPM file source"));
+ goto error;
+ }
+ def->data.emulator.storage_type = VIR_DOMAIN_TPM_STORAGE_FILE;
+ def->data.emulator.storagepath = g_steal_pointer(&path);
+ }
+
persistent_state = virXMLPropString(backends[0], "persistent_state");
if (persistent_state) {
if (virStringParseYesNo(persistent_state,
@@ -25066,6 +25079,14 @@ virDomainTPMDefFormat(virBuffer *buf,
virXMLFormatElement(&backendChildBuf, "active_pcr_banks", NULL,
&activePcrBanksBuf);
}
+ switch (def->data.emulator.storage_type) {
+ case VIR_DOMAIN_TPM_STORAGE_FILE:
+ virBufferAsprintf(&backendChildBuf, "<source
file='%s'/>\n",
+ def->data.emulator.storagepath);
+ break;
+ case VIR_DOMAIN_TPM_STORAGE_DEFAULT:
+ break;
+ }
break;
case VIR_DOMAIN_TPM_TYPE_EXTERNAL:
if (def->data.external.source->type == VIR_DOMAIN_CHR_TYPE_UNIX) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 659299bdd1..371e6ecf6c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1463,6 +1463,11 @@ typedef enum {
VIR_DOMAIN_TPM_PCR_BANK_LAST
} virDomainPcrBank;
+typedef enum {
+ VIR_DOMAIN_TPM_STORAGE_DEFAULT,
+ VIR_DOMAIN_TPM_STORAGE_FILE,
+} virDomainTPMStorage;
+
#define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0"
struct _virDomainTPMDef {
@@ -1478,6 +1483,7 @@ struct _virDomainTPMDef {
struct {
virDomainTPMVersion version;
virDomainChrSourceDef *source;
+ virDomainTPMStorage storage_type;
char *storagepath;
char *logfile;
unsigned int debug;
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index efb5f00d77..62d3f0e6fe 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -5923,6 +5923,7 @@
<interleave>
<ref name="tpm-backend-emulator-encryption"/>
<ref name="tpm-backend-emulator-active-pcr-banks"/>
+ <ref name="tpm-backend-emulator-source"/>
</interleave>
<optional>
<attribute name="persistent_state">
@@ -5981,6 +5982,16 @@
</optional>
</define>
+ <define name="tpm-backend-emulator-source">
+ <optional>
+ <element name="source">
+ <attribute name="file">
+ <ref name="filePath"/>
+ </attribute>
+ </element>
+ </optional>
+ </define>
+
<define name="tpm-backend-emulator-encryption">
<optional>
<element name="encryption">
diff --git a/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml
b/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml
index 8a613db456..4c61e2645b 100644
--- a/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml
+++ b/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml
@@ -34,6 +34,7 @@
<sha256/>
<sha512/>
</active_pcr_banks>
+ <source file='/path/to/state'/>
</backend>
</tpm>
<audio id='1' type='none'/>
--
2.45.2.827.g557ae147e6
7:58 a.m.
New subject: [PATCH v2 2/4] schema: add TPM emulator <source file='..'>
On 9/10/24 3:05 AM, marcandre.lureau(a)redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
Learn to parse a file path for the TPM state.
Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com>
Reviewed-by: Stefan Berger <stefanb(a)linux.ibm.com>
> ---
> docs/formatdomain.rst | 15 +++++++++++++++
> src/conf/domain_conf.c | 21 +++++++++++++++++++++
> src/conf/domain_conf.h | 6 ++++++
> src/conf/schemas/domaincommon.rng | 11 +++++++++++
> tests/qemuxmlconfdata/tpm-emulator-tpm2.xml | 1 +
> 5 files changed, 54 insertions(+)
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 47d3e2125e..4818113bc2 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -8170,6 +8170,21 @@ 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.
> +
> + The following attributes are supported:
> +
> + ``path``
> + The path to the TPM state storage file.
> +
> + This attribute requires that swtpm v0.7 or later is installed.
> +
> + :since:`Since v10.8.0`
> +
> ``persistent_state``
> The ``persistent_state`` attribute indicates whether 'swtpm' TPM state
is
> kept or not when a transient domain is powered off or undefined. This
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a263612ef7..18c58d16dc 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10789,6 +10789,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
> int nbackends;
> int nnodes;
> size_t i;
> + xmlNodePtr source_node = NULL;
> g_autofree char *path = NULL;
> g_autofree char *secretuuid = NULL;
> g_autofree char *persistent_state = NULL;
> @@ -10862,6 +10863,18 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
> def->data.emulator.hassecretuuid = true;
> }
>
> + source_node = virXPathNode("./backend/source", ctxt);
> + if (source_node) {
> + path = virXMLPropString(source_node, "file");
> + if (!path) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("missing TPM file source"));
> + goto error;
> + }
> + def->data.emulator.storage_type = VIR_DOMAIN_TPM_STORAGE_FILE;
> + def->data.emulator.storagepath = g_steal_pointer(&path);
> + }
> +
> persistent_state = virXMLPropString(backends[0],
"persistent_state");
> if (persistent_state) {
> if (virStringParseYesNo(persistent_state,
> @@ -25066,6 +25079,14 @@ virDomainTPMDefFormat(virBuffer *buf,
>
> virXMLFormatElement(&backendChildBuf, "active_pcr_banks",
NULL, &activePcrBanksBuf);
> }
> + switch (def->data.emulator.storage_type) {
> + case VIR_DOMAIN_TPM_STORAGE_FILE:
> + virBufferAsprintf(&backendChildBuf, "<source
file='%s'/>\n",
> + def->data.emulator.storagepath);
> + break;
> + case VIR_DOMAIN_TPM_STORAGE_DEFAULT:
> + break;
> + }
> break;
> case VIR_DOMAIN_TPM_TYPE_EXTERNAL:
> if (def->data.external.source->type == VIR_DOMAIN_CHR_TYPE_UNIX) {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 659299bdd1..371e6ecf6c 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1463,6 +1463,11 @@ typedef enum {
> VIR_DOMAIN_TPM_PCR_BANK_LAST
> } virDomainPcrBank;
>
> +typedef enum {
> + VIR_DOMAIN_TPM_STORAGE_DEFAULT,
> + VIR_DOMAIN_TPM_STORAGE_FILE,
> +} virDomainTPMStorage;
> +
> #define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0"
>
> struct _virDomainTPMDef {
> @@ -1478,6 +1483,7 @@ struct _virDomainTPMDef {
> struct {
> virDomainTPMVersion version;
> virDomainChrSourceDef *source;
> + virDomainTPMStorage storage_type;
> char *storagepath;
> char *logfile;
> unsigned int debug;
> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
> index efb5f00d77..62d3f0e6fe 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -5923,6 +5923,7 @@
> <interleave>
> <ref name="tpm-backend-emulator-encryption"/>
> <ref name="tpm-backend-emulator-active-pcr-banks"/>
> + <ref name="tpm-backend-emulator-source"/>
> </interleave>
> <optional>
> <attribute name="persistent_state">
> @@ -5981,6 +5982,16 @@
> </optional>
> </define>
>
> + <define name="tpm-backend-emulator-source">
> + <optional>
> + <element name="source">
> + <attribute name="file">
> + <ref name="filePath"/>
> + </attribute>
> + </element>
> + </optional>
> + </define>
> +
> <define name="tpm-backend-emulator-encryption">
> <optional>
> <element name="encryption">
> diff --git a/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml
b/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml
> index 8a613db456..4c61e2645b 100644
> --- a/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml
> +++ b/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml
> @@ -34,6 +34,7 @@
> <sha256/>
> <sha512/>
> </active_pcr_banks>
> + <source file='/path/to/state'/>
> </backend>
> </tpm>
> <audio id='1' type='none'/>
2:18 a.m.
New subject: [PATCH v2 2/4] schema: add TPM emulator <source file='..'>
On Tue, Sep 10, 2024 at 11:05:58AM +0400, marcandre.lureau(a)redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
Learn to parse a file path for the TPM state.
Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com>
---
docs/formatdomain.rst | 15 +++++++++++++++
src/conf/domain_conf.c | 21 +++++++++++++++++++++
src/conf/domain_conf.h | 6 ++++++
src/conf/schemas/domaincommon.rng | 11 +++++++++++
tests/qemuxmlconfdata/tpm-emulator-tpm2.xml | 1 +
5 files changed, 54 insertions(+)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 47d3e2125e..4818113bc2 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -8170,6 +8170,21 @@ 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.
+
+ The following attributes are supported:
+
+ ``path``
You name it "path" here, but everywhere below it is a "file".
2:24 a.m.
New subject: [PATCH v2 2/4] schema: add TPM emulator <source file='..'>
On Wed, Oct 2, 2024 at 11:18 AM Martin Kletzander <mkletzan(a)redhat.com> wrote:
On Tue, Sep 10, 2024 at 11:05:58AM +0400, marcandre.lureau(a)redhat.com wrote:
>From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
>
>Learn to parse a file path for the TPM state.
>
>Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com>
>---
> docs/formatdomain.rst | 15 +++++++++++++++
> src/conf/domain_conf.c | 21 +++++++++++++++++++++
> src/conf/domain_conf.h | 6 ++++++
> src/conf/schemas/domaincommon.rng | 11 +++++++++++
> tests/qemuxmlconfdata/tpm-emulator-tpm2.xml | 1 +
> 5 files changed, 54 insertions(+)
>
>diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
>index 47d3e2125e..4818113bc2 100644
>--- a/docs/formatdomain.rst
>+++ b/docs/formatdomain.rst
>@@ -8170,6 +8170,21 @@ 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.
>+
>+ The following attributes are supported:
>+
>+ ``path``
You name it "path" here, but everywhere below it is a "file".
good catch, fixed
2:05 a.m.
New subject: [PATCH v2 3/4] schema: add TPM emulator <source dir='..'>
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;
+ }
if (!path) {
virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing TPM file source"));
+ _("missing TPM file or directory source"));
goto error;
}
- def->data.emulator.storage_type = VIR_DOMAIN_TPM_STORAGE_FILE;
def->data.emulator.storagepath = g_steal_pointer(&path);
}
@@ -25084,6 +25087,10 @@ virDomainTPMDefFormat(virBuffer *buf,
virBufferAsprintf(&backendChildBuf, "<source
file='%s'/>\n",
def->data.emulator.storagepath);
break;
+ case VIR_DOMAIN_TPM_STORAGE_DIR:
+ virBufferAsprintf(&backendChildBuf, "<source
dir='%s'/>\n",
+ def->data.emulator.storagepath);
+ break;
case VIR_DOMAIN_TPM_STORAGE_DEFAULT:
break;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 371e6ecf6c..4e4ae2e048 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1465,6 +1465,7 @@ typedef enum {
typedef enum {
VIR_DOMAIN_TPM_STORAGE_DEFAULT,
+ VIR_DOMAIN_TPM_STORAGE_DIR,
VIR_DOMAIN_TPM_STORAGE_FILE,
} virDomainTPMStorage;
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index 62d3f0e6fe..f6b47ae97e 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -5985,9 +5985,18 @@
<define name="tpm-backend-emulator-source">
<optional>
<element name="source">
- <attribute name="file">
- <ref name="filePath"/>
- </attribute>
+ <choice>
+ <group>
+ <attribute name="dir">
+ <ref name="absDirPath"/>
+ </attribute>
+ </group>
+ <group>
+ <attribute name="file">
+ <ref name="filePath"/>
+ </attribute>
+ </group>
+ </choice>
</element>
</optional>
</define>
diff --git a/tests/qemuxmlconfdata/tpm-emulator-tpm2-enc.xml
b/tests/qemuxmlconfdata/tpm-emulator-tpm2-enc.xml
index 9c2279b28b..016c68296c 100644
--- a/tests/qemuxmlconfdata/tpm-emulator-tpm2-enc.xml
+++ b/tests/qemuxmlconfdata/tpm-emulator-tpm2-enc.xml
@@ -30,6 +30,7 @@
<tpm model='tpm-tis'>
<backend type='emulator' version='2.0'>
<encryption secret='32ee7e76-2178-47a1-ab7b-269e6e348015'/>
+ <source dir='/some/dir'/>
</backend>
</tpm>
<audio id='1' type='none'/>
--
2.45.2.827.g557ae147e6
8 a.m.
New subject: [PATCH v2 3/4] schema: add TPM emulator <source dir='..'>
On 9/10/24 3:05 AM, 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>
Reviewed-by: Stefan Berger <stefanb(a)linux.ibm.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;
> + }
> if (!path) {
> virReportError(VIR_ERR_XML_ERROR, "%s",
> - _("missing TPM file source"));
> + _("missing TPM file or directory
source"));
> goto error;
> }
> - def->data.emulator.storage_type = VIR_DOMAIN_TPM_STORAGE_FILE;
> def->data.emulator.storagepath = g_steal_pointer(&path);
> }
>
> @@ -25084,6 +25087,10 @@ virDomainTPMDefFormat(virBuffer *buf,
> virBufferAsprintf(&backendChildBuf, "<source
file='%s'/>\n",
> def->data.emulator.storagepath);
> break;
> + case VIR_DOMAIN_TPM_STORAGE_DIR:
> + virBufferAsprintf(&backendChildBuf, "<source
dir='%s'/>\n",
> + def->data.emulator.storagepath);
> + break;
> case VIR_DOMAIN_TPM_STORAGE_DEFAULT:
> break;
> }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 371e6ecf6c..4e4ae2e048 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1465,6 +1465,7 @@ typedef enum {
>
> typedef enum {
> VIR_DOMAIN_TPM_STORAGE_DEFAULT,
> + VIR_DOMAIN_TPM_STORAGE_DIR,
> VIR_DOMAIN_TPM_STORAGE_FILE,
> } virDomainTPMStorage;
>
> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
> index 62d3f0e6fe..f6b47ae97e 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -5985,9 +5985,18 @@
> <define name="tpm-backend-emulator-source">
> <optional>
> <element name="source">
> - <attribute name="file">
> - <ref name="filePath"/>
> - </attribute>
> + <choice>
> + <group>
> + <attribute name="dir">
> + <ref name="absDirPath"/>
> + </attribute>
> + </group>
> + <group>
> + <attribute name="file">
> + <ref name="filePath"/>
> + </attribute>
> + </group>
> + </choice>
> </element>
> </optional>
> </define>
> diff --git a/tests/qemuxmlconfdata/tpm-emulator-tpm2-enc.xml
b/tests/qemuxmlconfdata/tpm-emulator-tpm2-enc.xml
> index 9c2279b28b..016c68296c 100644
> --- a/tests/qemuxmlconfdata/tpm-emulator-tpm2-enc.xml
> +++ b/tests/qemuxmlconfdata/tpm-emulator-tpm2-enc.xml
> @@ -30,6 +30,7 @@
> <tpm model='tpm-tis'>
> <backend type='emulator' version='2.0'>
> <encryption secret='32ee7e76-2178-47a1-ab7b-269e6e348015'/>
> + <source dir='/some/dir'/>
> </backend>
> </tpm>
> <audio id='1' type='none'/>
2:19 a.m.
New subject: [PATCH v2 3/4] schema: add TPM emulator <source dir='..'>
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).
2:30 a.m.
New subject: [PATCH v2 3/4] schema: add TPM emulator <source dir='..'>
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.
2:34 a.m.
New subject: [PATCH v2 3/4] schema: add TPM emulator <source dir='..'>
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"/>
6:43 a.m.
New subject: [PATCH v2 3/4] schema: add TPM emulator <source dir='..'>
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.
9:01 a.m.
New subject: [PATCH v2 3/4] schema: add TPM emulator <source dir='..'>
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(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='...'>,
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/B...
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.
6:34 a.m.
New subject: [PATCH v2 3/4] schema: add TPM emulator <source dir='..'>
Hi Martin, Daniel
On Wed, Oct 2, 2024 at 6:01 PM Martin Kletzander <mkletzan(a)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(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='...'>,
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/B...
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 :)
3:37 a.m.
New subject: [PATCH v2 3/4] schema: add TPM emulator <source dir='..'>
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.
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 :|
2:06 a.m.
New subject: [PATCH v2 4/4] qemu_tpm: handle file/block storage source
From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
When swtpm reports "nvram-backend-dir", it can accepts a single file or
block device where TPM state will be stored. --tpmstate must be
backend-uri=file://<path>.
Teach the storage to use custom directory or file source location.
Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com>
---
src/qemu/qemu_tpm.c | 76 +++++++++++++++++++++++++++++++++++++--------
1 file changed, 63 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 2f17918cbb..faf07556d2 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -340,9 +340,28 @@ qemuTPMVirCommandAddEncryption(virCommand *cmd,
}
+static char *
+qemuTPMGetSwtpmSetupStateArg(const virDomainTPMStorage storage_type,
+ const char *storagepath)
+{
+ switch (storage_type) {
+ case VIR_DOMAIN_TPM_STORAGE_FILE:
+ /* the file:// prefix is supported since swtpm_setup 0.7.0 */
+ /* assume the capability check for swtpm is redundant. */
+ return g_strdup_printf("file://%s", storagepath);
+ case VIR_DOMAIN_TPM_STORAGE_DIR:
+ case VIR_DOMAIN_TPM_STORAGE_DEFAULT:
+ return g_strdup_printf("%s", storagepath);
+ }
+
+ return NULL;
+}
+
+
/*
* qemuTPMEmulatorRunSetup
*
+ * @storage_type: type of storage
* @storagepath: path to the directory for TPM state
* @vmname: the name of the VM
* @vmuuid: the UUID of the VM
@@ -360,7 +379,8 @@ qemuTPMVirCommandAddEncryption(virCommand *cmd,
* certificates for it.
*/
static int
-qemuTPMEmulatorRunSetup(const char *storagepath,
+qemuTPMEmulatorRunSetup(const virDomainTPMStorage storage_type,
+ const char *storagepath,
const char *vmname,
const unsigned char *vmuuid,
bool privileged,
@@ -376,6 +396,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
char uuid[VIR_UUID_STRING_BUFLEN];
g_autofree char *vmid = NULL;
g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
+ g_autofree char *tpm_state = qemuTPMGetSwtpmSetupStateArg(storage_type,
storagepath);
if (!swtpm_setup)
return -1;
@@ -413,7 +434,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
if (!incomingMigration) {
virCommandAddArgList(cmd,
- "--tpm-state", storagepath,
+ "--tpm-state", tpm_state,
"--vmid", vmid,
"--logfile", logfile,
"--createek",
@@ -424,7 +445,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
NULL);
} else {
virCommandAddArgList(cmd,
- "--tpm-state", storagepath,
+ "--tpm-state", tpm_state,
"--logfile", logfile,
"--overwrite",
NULL);
@@ -465,6 +486,7 @@ qemuTPMPcrBankBitmapToStr(virBitmap *activePcrBanks)
* qemuTPMEmulatorReconfigure
*
*
+ * @storage_type: type of storage
* @storagepath: path to the directory for TPM state
* @swtpm_user: The userid to switch to when setting up the TPM;
* typically this should be the uid of 'tss' or 'root'
@@ -478,7 +500,8 @@ qemuTPMPcrBankBitmapToStr(virBitmap *activePcrBanks)
* Reconfigure the active PCR banks of a TPM 2.
*/
static int
-qemuTPMEmulatorReconfigure(const char *storagepath,
+qemuTPMEmulatorReconfigure(const virDomainTPMStorage storage_type,
+ const char *storagepath,
uid_t swtpm_user,
gid_t swtpm_group,
virBitmap *activePcrBanks,
@@ -490,6 +513,7 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
int exitstatus;
g_autofree char *activePcrBanksStr = NULL;
g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
+ g_autofree char *tpm_state = qemuTPMGetSwtpmSetupStateArg(storage_type,
storagepath);
if (!swtpm_setup)
return -1;
@@ -510,7 +534,7 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
return -1;
virCommandAddArgList(cmd,
- "--tpm-state", storagepath,
+ "--tpm-state", tpm_state,
"--logfile", logfile,
"--pcr-banks", activePcrBanksStr,
"--reconfigure",
@@ -555,6 +579,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
{
g_autoptr(virCommand) cmd = NULL;
bool created = false;
+ bool run_setup = false;
g_autofree char *swtpm = virTPMGetSwtpm();
int pwdfile_fd = -1;
int migpwdfile_fd = -1;
@@ -565,6 +590,18 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
if (!swtpm)
return NULL;
+ if (tpm->data.emulator.storage_type == VIR_DOMAIN_TPM_STORAGE_FILE) {
+ if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_DIR)) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ _("%1$s does not support file storage"),
+ swtpm);
+ goto error;
+ }
+ create_storage = false;
+ /* setup is run with --not-overwrite */
+ run_setup = true;
+ }
+
/* Do not create storage and run swtpm_setup on incoming migration over
* shared storage
*/
@@ -572,15 +609,18 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
if (incomingMigration && on_shared_storage)
create_storage = false;
- if (create_storage &&
- qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group) < 0)
- return NULL;
+ if (create_storage) {
+ if (qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group) <
0)
+ return NULL;
+ run_setup = created;
+ }
if (tpm->data.emulator.hassecretuuid)
secretuuid = tpm->data.emulator.secretuuid;
- if (created &&
- qemuTPMEmulatorRunSetup(tpm->data.emulator.storagepath, vmname, vmuuid,
+ if (run_setup &&
+ qemuTPMEmulatorRunSetup(tpm->data.emulator.storage_type,
+ tpm->data.emulator.storagepath, vmname, vmuuid,
privileged, swtpm_user, swtpm_group,
tpm->data.emulator.logfile,
tpm->data.emulator.version,
@@ -588,7 +628,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
goto error;
if (!incomingMigration &&
- qemuTPMEmulatorReconfigure(tpm->data.emulator.storagepath,
+ qemuTPMEmulatorReconfigure(tpm->data.emulator.storage_type,
+ tpm->data.emulator.storagepath,
swtpm_user, swtpm_group,
tpm->data.emulator.activePcrBanks,
tpm->data.emulator.logfile,
@@ -607,8 +648,17 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
tpm->data.emulator.source->data.nix.path);
virCommandAddArg(cmd, "--tpmstate");
- virCommandAddArgFormat(cmd, "dir=%s,mode=0600",
- tpm->data.emulator.storagepath);
+ switch (tpm->data.emulator.storage_type) {
+ case VIR_DOMAIN_TPM_STORAGE_FILE:
+ virCommandAddArgFormat(cmd, "backend-uri=file://%s",
+ tpm->data.emulator.storagepath);
+ break;
+ case VIR_DOMAIN_TPM_STORAGE_DIR:
+ case VIR_DOMAIN_TPM_STORAGE_DEFAULT:
+ virCommandAddArgFormat(cmd, "dir=%s,mode=0600",
+ tpm->data.emulator.storagepath);
+ break;
+ }
virCommandAddArg(cmd, "--log");
if (tpm->data.emulator.debug != 0)
--
2.45.2.827.g557ae147e6
8:14 a.m.
New subject: [PATCH v2 4/4] qemu_tpm: handle file/block storage source
On 9/10/24 3:06 AM, marcandre.lureau(a)redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
When swtpm reports "nvram-backend-dir", it can accepts a single file or
s/accepts/accept
block device where TPM state will be stored. --tpmstate must be
backend-uri=file://<path>.
Teach the storage to use custom directory or file source location.
Is it a concern that the file backend does not lock access to the TPM
state file like the dir backend does? The dir backend would prevent
concurrent usage of the state by two different instances of swtpm even
now that the user gets control over the file paths but the file backend
will not prevent it ...
Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com>
Reviewed-by: Stefan Berger <stefanb(a)linux.ibm.com>
> ---
> src/qemu/qemu_tpm.c | 76 +++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 63 insertions(+), 13 deletions(-)
>
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 2f17918cbb..faf07556d2 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -340,9 +340,28 @@ qemuTPMVirCommandAddEncryption(virCommand *cmd,
> }
>
>
> +static char *
> +qemuTPMGetSwtpmSetupStateArg(const virDomainTPMStorage storage_type,
> + const char *storagepath)
> +{
> + switch (storage_type) {
> + case VIR_DOMAIN_TPM_STORAGE_FILE:
> + /* the file:// prefix is supported since swtpm_setup 0.7.0 */
> + /* assume the capability check for swtpm is redundant. */
> + return g_strdup_printf("file://%s", storagepath);
> + case VIR_DOMAIN_TPM_STORAGE_DIR:
> + case VIR_DOMAIN_TPM_STORAGE_DEFAULT:
> + return g_strdup_printf("%s", storagepath);
> + }
> +
> + return NULL;
> +}
> +
> +
> /*
> * qemuTPMEmulatorRunSetup
> *
> + * @storage_type: type of storage
> * @storagepath: path to the directory for TPM state
> * @vmname: the name of the VM
> * @vmuuid: the UUID of the VM
> @@ -360,7 +379,8 @@ qemuTPMVirCommandAddEncryption(virCommand *cmd,
> * certificates for it.
> */
> static int
> -qemuTPMEmulatorRunSetup(const char *storagepath,
> +qemuTPMEmulatorRunSetup(const virDomainTPMStorage storage_type,
> + const char *storagepath,
> const char *vmname,
> const unsigned char *vmuuid,
> bool privileged,
> @@ -376,6 +396,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
> char uuid[VIR_UUID_STRING_BUFLEN];
> g_autofree char *vmid = NULL;
> g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
> + g_autofree char *tpm_state = qemuTPMGetSwtpmSetupStateArg(storage_type,
storagepath);
>
> if (!swtpm_setup)
> return -1;
> @@ -413,7 +434,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>
> if (!incomingMigration) {
> virCommandAddArgList(cmd,
> - "--tpm-state", storagepath,
> + "--tpm-state", tpm_state,
> "--vmid", vmid,
> "--logfile", logfile,
> "--createek",
> @@ -424,7 +445,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
> NULL);
> } else {
> virCommandAddArgList(cmd,
> - "--tpm-state", storagepath,
> + "--tpm-state", tpm_state,
> "--logfile", logfile,
> "--overwrite",
> NULL);
> @@ -465,6 +486,7 @@ qemuTPMPcrBankBitmapToStr(virBitmap *activePcrBanks)
> * qemuTPMEmulatorReconfigure
> *
> *
> + * @storage_type: type of storage
> * @storagepath: path to the directory for TPM state
> * @swtpm_user: The userid to switch to when setting up the TPM;
> * typically this should be the uid of 'tss' or
'root'
> @@ -478,7 +500,8 @@ qemuTPMPcrBankBitmapToStr(virBitmap *activePcrBanks)
> * Reconfigure the active PCR banks of a TPM 2.
> */
> static int
> -qemuTPMEmulatorReconfigure(const char *storagepath,
> +qemuTPMEmulatorReconfigure(const virDomainTPMStorage storage_type,
> + const char *storagepath,
> uid_t swtpm_user,
> gid_t swtpm_group,
> virBitmap *activePcrBanks,
> @@ -490,6 +513,7 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
> int exitstatus;
> g_autofree char *activePcrBanksStr = NULL;
> g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
> + g_autofree char *tpm_state = qemuTPMGetSwtpmSetupStateArg(storage_type,
storagepath);
>
> if (!swtpm_setup)
> return -1;
> @@ -510,7 +534,7 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
> return -1;
>
> virCommandAddArgList(cmd,
> - "--tpm-state", storagepath,
> + "--tpm-state", tpm_state,
> "--logfile", logfile,
> "--pcr-banks", activePcrBanksStr,
> "--reconfigure",
> @@ -555,6 +579,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
> {
> g_autoptr(virCommand) cmd = NULL;
> bool created = false;
> + bool run_setup = false;
> g_autofree char *swtpm = virTPMGetSwtpm();
> int pwdfile_fd = -1;
> int migpwdfile_fd = -1;
> @@ -565,6 +590,18 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
> if (!swtpm)
> return NULL;
>
> + if (tpm->data.emulator.storage_type == VIR_DOMAIN_TPM_STORAGE_FILE) {
> + if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_DIR)) {
> + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> + _("%1$s does not support file storage"),
> + swtpm);
> + goto error;
> + }
> + create_storage = false;
> + /* setup is run with --not-overwrite */
> + run_setup = true;
> + }
> +
> /* Do not create storage and run swtpm_setup on incoming migration over
> * shared storage
> */
> @@ -572,15 +609,18 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
> if (incomingMigration && on_shared_storage)
> create_storage = false;
>
> - if (create_storage &&
> - qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group)
< 0)
> - return NULL;
> + if (create_storage) {
> + if (qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group)
< 0)
> + return NULL;
> + run_setup = created;
> + }
>
> if (tpm->data.emulator.hassecretuuid)
> secretuuid = tpm->data.emulator.secretuuid;
>
> - if (created &&
> - qemuTPMEmulatorRunSetup(tpm->data.emulator.storagepath, vmname, vmuuid,
> + if (run_setup &&
> + qemuTPMEmulatorRunSetup(tpm->data.emulator.storage_type,
> + tpm->data.emulator.storagepath, vmname, vmuuid,
> privileged, swtpm_user, swtpm_group,
> tpm->data.emulator.logfile,
> tpm->data.emulator.version,
> @@ -588,7 +628,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
> goto error;
>
> if (!incomingMigration &&
> - qemuTPMEmulatorReconfigure(tpm->data.emulator.storagepath,
> + qemuTPMEmulatorReconfigure(tpm->data.emulator.storage_type,
> + tpm->data.emulator.storagepath,
> swtpm_user, swtpm_group,
> tpm->data.emulator.activePcrBanks,
> tpm->data.emulator.logfile,
> @@ -607,8 +648,17 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
> tpm->data.emulator.source->data.nix.path);
>
> virCommandAddArg(cmd, "--tpmstate");
> - virCommandAddArgFormat(cmd, "dir=%s,mode=0600",
> - tpm->data.emulator.storagepath);
> + switch (tpm->data.emulator.storage_type) {
> + case VIR_DOMAIN_TPM_STORAGE_FILE:
> + virCommandAddArgFormat(cmd, "backend-uri=file://%s",
> + tpm->data.emulator.storagepath);
> + break;
> + case VIR_DOMAIN_TPM_STORAGE_DIR:
> + case VIR_DOMAIN_TPM_STORAGE_DEFAULT:
> + virCommandAddArgFormat(cmd, "dir=%s,mode=0600",
> + tpm->data.emulator.storagepath);
> + break;
> + }
>
> virCommandAddArg(cmd, "--log");
> if (tpm->data.emulator.debug != 0)
1:37 a.m.
New subject: [PATCH v2 4/4] qemu_tpm: handle file/block storage source
Hi
On Fri, Sep 13, 2024 at 5:14 PM Stefan Berger <stefanb(a)linux.ibm.com> wrote:
On 9/10/24 3:06 AM, marcandre.lureau(a)redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
>
> When swtpm reports "nvram-backend-dir", it can accepts a single file or
s/accepts/accept
> block device where TPM state will be stored. --tpmstate must be
> backend-uri=file://<path>.
>
> Teach the storage to use custom directory or file source location.
Is it a concern that the file backend does not lock access to the TPM
state file like the dir backend does? The dir backend would prevent
concurrent usage of the state by two different instances of swtpm even
now that the user gets control over the file paths but the file backend
will not prevent it ...
Why not use a lock on the file too?
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com>
Reviewed-by: Stefan Berger <stefanb(a)linux.ibm.com>
thanks
> ---
> src/qemu/qemu_tpm.c | 76 +++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 63 insertions(+), 13 deletions(-)
>
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 2f17918cbb..faf07556d2 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -340,9 +340,28 @@ qemuTPMVirCommandAddEncryption(virCommand *cmd,
> }
>
>
> +static char *
> +qemuTPMGetSwtpmSetupStateArg(const virDomainTPMStorage storage_type,
> + const char *storagepath)
> +{
> + switch (storage_type) {
> + case VIR_DOMAIN_TPM_STORAGE_FILE:
> + /* the file:// prefix is supported since swtpm_setup 0.7.0 */
> + /* assume the capability check for swtpm is redundant. */
> + return g_strdup_printf("file://%s", storagepath);
> + case VIR_DOMAIN_TPM_STORAGE_DIR:
> + case VIR_DOMAIN_TPM_STORAGE_DEFAULT:
> + return g_strdup_printf("%s", storagepath);
> + }
> +
> + return NULL;
> +}
> +
> +
> /*
> * qemuTPMEmulatorRunSetup
> *
> + * @storage_type: type of storage
> * @storagepath: path to the directory for TPM state
> * @vmname: the name of the VM
> * @vmuuid: the UUID of the VM
> @@ -360,7 +379,8 @@ qemuTPMVirCommandAddEncryption(virCommand *cmd,
> * certificates for it.
> */
> static int
> -qemuTPMEmulatorRunSetup(const char *storagepath,
> +qemuTPMEmulatorRunSetup(const virDomainTPMStorage storage_type,
> + const char *storagepath,
> const char *vmname,
> const unsigned char *vmuuid,
> bool privileged,
> @@ -376,6 +396,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
> char uuid[VIR_UUID_STRING_BUFLEN];
> g_autofree char *vmid = NULL;
> g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
> + g_autofree char *tpm_state = qemuTPMGetSwtpmSetupStateArg(storage_type,
storagepath);
>
> if (!swtpm_setup)
> return -1;
> @@ -413,7 +434,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>
> if (!incomingMigration) {
> virCommandAddArgList(cmd,
> - "--tpm-state", storagepath,
> + "--tpm-state", tpm_state,
> "--vmid", vmid,
> "--logfile", logfile,
> "--createek",
> @@ -424,7 +445,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
> NULL);
> } else {
> virCommandAddArgList(cmd,
> - "--tpm-state", storagepath,
> + "--tpm-state", tpm_state,
> "--logfile", logfile,
> "--overwrite",
> NULL);
> @@ -465,6 +486,7 @@ qemuTPMPcrBankBitmapToStr(virBitmap *activePcrBanks)
> * qemuTPMEmulatorReconfigure
> *
> *
> + * @storage_type: type of storage
> * @storagepath: path to the directory for TPM state
> * @swtpm_user: The userid to switch to when setting up the TPM;
> * typically this should be the uid of 'tss' or
'root'
> @@ -478,7 +500,8 @@ qemuTPMPcrBankBitmapToStr(virBitmap *activePcrBanks)
> * Reconfigure the active PCR banks of a TPM 2.
> */
> static int
> -qemuTPMEmulatorReconfigure(const char *storagepath,
> +qemuTPMEmulatorReconfigure(const virDomainTPMStorage storage_type,
> + const char *storagepath,
> uid_t swtpm_user,
> gid_t swtpm_group,
> virBitmap *activePcrBanks,
> @@ -490,6 +513,7 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
> int exitstatus;
> g_autofree char *activePcrBanksStr = NULL;
> g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
> + g_autofree char *tpm_state = qemuTPMGetSwtpmSetupStateArg(storage_type,
storagepath);
>
> if (!swtpm_setup)
> return -1;
> @@ -510,7 +534,7 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
> return -1;
>
> virCommandAddArgList(cmd,
> - "--tpm-state", storagepath,
> + "--tpm-state", tpm_state,
> "--logfile", logfile,
> "--pcr-banks", activePcrBanksStr,
> "--reconfigure",
> @@ -555,6 +579,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
> {
> g_autoptr(virCommand) cmd = NULL;
> bool created = false;
> + bool run_setup = false;
> g_autofree char *swtpm = virTPMGetSwtpm();
> int pwdfile_fd = -1;
> int migpwdfile_fd = -1;
> @@ -565,6 +590,18 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
> if (!swtpm)
> return NULL;
>
> + if (tpm->data.emulator.storage_type == VIR_DOMAIN_TPM_STORAGE_FILE) {
> + if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_DIR)) {
> + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> + _("%1$s does not support file storage"),
> + swtpm);
> + goto error;
> + }
> + create_storage = false;
> + /* setup is run with --not-overwrite */
> + run_setup = true;
> + }
> +
> /* Do not create storage and run swtpm_setup on incoming migration over
> * shared storage
> */
> @@ -572,15 +609,18 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
> if (incomingMigration && on_shared_storage)
> create_storage = false;
>
> - if (create_storage &&
> - qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group)
< 0)
> - return NULL;
> + if (create_storage) {
> + if (qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user,
swtpm_group) < 0)
> + return NULL;
> + run_setup = created;
> + }
>
> if (tpm->data.emulator.hassecretuuid)
> secretuuid = tpm->data.emulator.secretuuid;
>
> - if (created &&
> - qemuTPMEmulatorRunSetup(tpm->data.emulator.storagepath, vmname, vmuuid,
> + if (run_setup &&
> + qemuTPMEmulatorRunSetup(tpm->data.emulator.storage_type,
> + tpm->data.emulator.storagepath, vmname, vmuuid,
> privileged, swtpm_user, swtpm_group,
> tpm->data.emulator.logfile,
> tpm->data.emulator.version,
> @@ -588,7 +628,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
> goto error;
>
> if (!incomingMigration &&
> - qemuTPMEmulatorReconfigure(tpm->data.emulator.storagepath,
> + qemuTPMEmulatorReconfigure(tpm->data.emulator.storage_type,
> + tpm->data.emulator.storagepath,
> swtpm_user, swtpm_group,
> tpm->data.emulator.activePcrBanks,
> tpm->data.emulator.logfile,
> @@ -607,8 +648,17 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
> tpm->data.emulator.source->data.nix.path);
>
> virCommandAddArg(cmd, "--tpmstate");
> - virCommandAddArgFormat(cmd, "dir=%s,mode=0600",
> - tpm->data.emulator.storagepath);
> + switch (tpm->data.emulator.storage_type) {
> + case VIR_DOMAIN_TPM_STORAGE_FILE:
> + virCommandAddArgFormat(cmd, "backend-uri=file://%s",
> + tpm->data.emulator.storagepath);
> + break;
> + case VIR_DOMAIN_TPM_STORAGE_DIR:
> + case VIR_DOMAIN_TPM_STORAGE_DEFAULT:
> + virCommandAddArgFormat(cmd, "dir=%s,mode=0600",
> + tpm->data.emulator.storagepath);
> + break;
> + }
>
> virCommandAddArg(cmd, "--log");
> if (tpm->data.emulator.debug != 0)
8:10 a.m.
New subject: [PATCH v2 4/4] qemu_tpm: handle file/block storage source
On 9/23/24 2:37 AM, Marc-André Lureau wrote:
Hi
On Fri, Sep 13, 2024 at 5:14 PM Stefan Berger <stefanb(a)linux.ibm.com> wrote:
>
>
>
> On 9/10/24 3:06 AM, marcandre.lureau(a)redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
>>
>> When swtpm reports "nvram-backend-dir", it can accepts a single file
or
>
> s/accepts/accept
>
>> block device where TPM state will be stored. --tpmstate must be
>> backend-uri=file://<path>.
>>
>> Teach the storage to use custom directory or file source location.
>
> Is it a concern that the file backend does not lock access to the TPM
> state file like the dir backend does? The dir backend would prevent
> concurrent usage of the state by two different instances of swtpm even
> now that the user gets control over the file paths but the file backend
> will not prevent it ...
>
Why not use a lock on the file too?
Those guys who contributed the file backend didn't seem to need it. If
the file paths per VM are separated you don't really need. Otherwise it
may again make the coordination during migration over shared storage
more complicated.
>
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com>
>>
>> Reviewed-by: Stefan Berger <stefanb(a)linux.ibm.com>
>
> thanks
>
>>
>>> ---
>>> src/qemu/qemu_tpm.c | 76 +++++++++++++++++++++++++++++++++++++--------
>>> 1 file changed, 63 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
>>> index 2f17918cbb..faf07556d2 100644
>>> --- a/src/qemu/qemu_tpm.c
>>> +++ b/src/qemu/qemu_tpm.c
>>> @@ -340,9 +340,28 @@ qemuTPMVirCommandAddEncryption(virCommand *cmd,
>>> }
>>>
>>>
>>> +static char *
>>> +qemuTPMGetSwtpmSetupStateArg(const virDomainTPMStorage storage_type,
>>> + const char *storagepath)
>>> +{
>>> + switch (storage_type) {
>>> + case VIR_DOMAIN_TPM_STORAGE_FILE:
>>> + /* the file:// prefix is supported since swtpm_setup 0.7.0 */
>>> + /* assume the capability check for swtpm is redundant. */
>>> + return g_strdup_printf("file://%s", storagepath);
>>> + case VIR_DOMAIN_TPM_STORAGE_DIR:
>>> + case VIR_DOMAIN_TPM_STORAGE_DEFAULT:
>>> + return g_strdup_printf("%s", storagepath);
>>> + }
>>> +
>>> + return NULL;
>>> +}
>>> +
>>> +
>>> /*
>>> * qemuTPMEmulatorRunSetup
>>> *
>>> + * @storage_type: type of storage
>>> * @storagepath: path to the directory for TPM state
>>> * @vmname: the name of the VM
>>> * @vmuuid: the UUID of the VM
>>> @@ -360,7 +379,8 @@ qemuTPMVirCommandAddEncryption(virCommand *cmd,
>>> * certificates for it.
>>> */
>>> static int
>>> -qemuTPMEmulatorRunSetup(const char *storagepath,
>>> +qemuTPMEmulatorRunSetup(const virDomainTPMStorage storage_type,
>>> + const char *storagepath,
>>> const char *vmname,
>>> const unsigned char *vmuuid,
>>> bool privileged,
>>> @@ -376,6 +396,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>>> char uuid[VIR_UUID_STRING_BUFLEN];
>>> g_autofree char *vmid = NULL;
>>> g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
>>> + g_autofree char *tpm_state = qemuTPMGetSwtpmSetupStateArg(storage_type,
storagepath);
>>>
>>> if (!swtpm_setup)
>>> return -1;
>>> @@ -413,7 +434,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>>>
>>> if (!incomingMigration) {
>>> virCommandAddArgList(cmd,
>>> - "--tpm-state", storagepath,
>>> + "--tpm-state", tpm_state,
>>> "--vmid", vmid,
>>> "--logfile", logfile,
>>> "--createek",
>>> @@ -424,7 +445,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>>> NULL);
>>> } else {
>>> virCommandAddArgList(cmd,
>>> - "--tpm-state", storagepath,
>>> + "--tpm-state", tpm_state,
>>> "--logfile", logfile,
>>> "--overwrite",
>>> NULL);
>>> @@ -465,6 +486,7 @@ qemuTPMPcrBankBitmapToStr(virBitmap *activePcrBanks)
>>> * qemuTPMEmulatorReconfigure
>>> *
>>> *
>>> + * @storage_type: type of storage
>>> * @storagepath: path to the directory for TPM state
>>> * @swtpm_user: The userid to switch to when setting up the TPM;
>>> * typically this should be the uid of 'tss' or
'root'
>>> @@ -478,7 +500,8 @@ qemuTPMPcrBankBitmapToStr(virBitmap *activePcrBanks)
>>> * Reconfigure the active PCR banks of a TPM 2.
>>> */
>>> static int
>>> -qemuTPMEmulatorReconfigure(const char *storagepath,
>>> +qemuTPMEmulatorReconfigure(const virDomainTPMStorage storage_type,
>>> + const char *storagepath,
>>> uid_t swtpm_user,
>>> gid_t swtpm_group,
>>> virBitmap *activePcrBanks,
>>> @@ -490,6 +513,7 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
>>> int exitstatus;
>>> g_autofree char *activePcrBanksStr = NULL;
>>> g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
>>> + g_autofree char *tpm_state = qemuTPMGetSwtpmSetupStateArg(storage_type,
storagepath);
>>>
>>> if (!swtpm_setup)
>>> return -1;
>>> @@ -510,7 +534,7 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
>>> return -1;
>>>
>>> virCommandAddArgList(cmd,
>>> - "--tpm-state", storagepath,
>>> + "--tpm-state", tpm_state,
>>> "--logfile", logfile,
>>> "--pcr-banks", activePcrBanksStr,
>>> "--reconfigure",
>>> @@ -555,6 +579,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
>>> {
>>> g_autoptr(virCommand) cmd = NULL;
>>> bool created = false;
>>> + bool run_setup = false;
>>> g_autofree char *swtpm = virTPMGetSwtpm();
>>> int pwdfile_fd = -1;
>>> int migpwdfile_fd = -1;
>>> @@ -565,6 +590,18 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
>>> if (!swtpm)
>>> return NULL;
>>>
>>> + if (tpm->data.emulator.storage_type == VIR_DOMAIN_TPM_STORAGE_FILE)
{
>>> + if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_DIR)) {
>>> + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
>>> + _("%1$s does not support file
storage"),
>>> + swtpm);
>>> + goto error;
>>> + }
>>> + create_storage = false;
>>> + /* setup is run with --not-overwrite */
>>> + run_setup = true;
>>> + }
>>> +
>>> /* Do not create storage and run swtpm_setup on incoming migration
over
>>> * shared storage
>>> */
>>> @@ -572,15 +609,18 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
>>> if (incomingMigration && on_shared_storage)
>>> create_storage = false;
>>>
>>> - if (create_storage &&
>>> - qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user,
swtpm_group) < 0)
>>> - return NULL;
>>> + if (create_storage) {
>>> + if (qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user,
swtpm_group) < 0)
>>> + return NULL;
>>> + run_setup = created;
>>> + }
>>>
>>> if (tpm->data.emulator.hassecretuuid)
>>> secretuuid = tpm->data.emulator.secretuuid;
>>>
>>> - if (created &&
>>> - qemuTPMEmulatorRunSetup(tpm->data.emulator.storagepath, vmname,
vmuuid,
>>> + if (run_setup &&
>>> + qemuTPMEmulatorRunSetup(tpm->data.emulator.storage_type,
>>> + tpm->data.emulator.storagepath, vmname,
vmuuid,
>>> privileged, swtpm_user, swtpm_group,
>>> tpm->data.emulator.logfile,
>>> tpm->data.emulator.version,
>>> @@ -588,7 +628,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
>>> goto error;
>>>
>>> if (!incomingMigration &&
>>> - qemuTPMEmulatorReconfigure(tpm->data.emulator.storagepath,
>>> + qemuTPMEmulatorReconfigure(tpm->data.emulator.storage_type,
>>> + tpm->data.emulator.storagepath,
>>> swtpm_user, swtpm_group,
>>> tpm->data.emulator.activePcrBanks,
>>> tpm->data.emulator.logfile,
>>> @@ -607,8 +648,17 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
>>>
tpm->data.emulator.source->data.nix.path);
>>>
>>> virCommandAddArg(cmd, "--tpmstate");
>>> - virCommandAddArgFormat(cmd, "dir=%s,mode=0600",
>>> - tpm->data.emulator.storagepath);
>>> + switch (tpm->data.emulator.storage_type) {
>>> + case VIR_DOMAIN_TPM_STORAGE_FILE:
>>> + virCommandAddArgFormat(cmd, "backend-uri=file://%s",
>>> + tpm->data.emulator.storagepath);
>>> + break;
>>> + case VIR_DOMAIN_TPM_STORAGE_DIR:
>>> + case VIR_DOMAIN_TPM_STORAGE_DEFAULT:
>>> + virCommandAddArgFormat(cmd, "dir=%s,mode=0600",
>>> + tpm->data.emulator.storagepath);
>>> + break;
>>> + }
>>>
>>> virCommandAddArg(cmd, "--log");
>>> if (tpm->data.emulator.debug != 0)
>>
>
2:26 a.m.
New subject: [PATCH v2 4/4] qemu_tpm: handle file/block storage source
On Tue, Sep 10, 2024 at 11:06:00AM +0400, marcandre.lureau(a)redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
When swtpm reports "nvram-backend-dir", it can accepts a single file or
block device where TPM state will be stored. --tpmstate must be
backend-uri=file://<path>.
Teach the storage to use custom directory or file source location.
Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com>
---
src/qemu/qemu_tpm.c | 76 +++++++++++++++++++++++++++++++++++++--------
1 file changed, 63 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 2f17918cbb..faf07556d2 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -340,9 +340,28 @@ qemuTPMVirCommandAddEncryption(virCommand *cmd,
}
+static char *
+qemuTPMGetSwtpmSetupStateArg(const virDomainTPMStorage storage_type,
+ const char *storagepath)
+{
+ switch (storage_type) {
+ case VIR_DOMAIN_TPM_STORAGE_FILE:
+ /* the file:// prefix is supported since swtpm_setup 0.7.0 */
+ /* assume the capability check for swtpm is redundant. */
+ return g_strdup_printf("file://%s", storagepath);
+ case VIR_DOMAIN_TPM_STORAGE_DIR:
+ case VIR_DOMAIN_TPM_STORAGE_DEFAULT:
+ return g_strdup_printf("%s", storagepath);
+ }
+
+ return NULL;
+}
+
+
/*
* qemuTPMEmulatorRunSetup
*
+ * @storage_type: type of storage
* @storagepath: path to the directory for TPM state
* @vmname: the name of the VM
* @vmuuid: the UUID of the VM
@@ -360,7 +379,8 @@ qemuTPMVirCommandAddEncryption(virCommand *cmd,
* certificates for it.
*/
static int
-qemuTPMEmulatorRunSetup(const char *storagepath,
+qemuTPMEmulatorRunSetup(const virDomainTPMStorage storage_type,
+ const char *storagepath,
const char *vmname,
const unsigned char *vmuuid,
bool privileged,
@@ -376,6 +396,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
char uuid[VIR_UUID_STRING_BUFLEN];
g_autofree char *vmid = NULL;
g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
+ g_autofree char *tpm_state = qemuTPMGetSwtpmSetupStateArg(storage_type,
storagepath);
if (!swtpm_setup)
return -1;
@@ -413,7 +434,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
if (!incomingMigration) {
virCommandAddArgList(cmd,
- "--tpm-state", storagepath,
+ "--tpm-state", tpm_state,
"--vmid", vmid,
"--logfile", logfile,
"--createek",
@@ -424,7 +445,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
NULL);
} else {
virCommandAddArgList(cmd,
- "--tpm-state", storagepath,
+ "--tpm-state", tpm_state,
"--logfile", logfile,
"--overwrite",
NULL);
@@ -465,6 +486,7 @@ qemuTPMPcrBankBitmapToStr(virBitmap *activePcrBanks)
* qemuTPMEmulatorReconfigure
*
*
+ * @storage_type: type of storage
* @storagepath: path to the directory for TPM state
* @swtpm_user: The userid to switch to when setting up the TPM;
* typically this should be the uid of 'tss' or 'root'
@@ -478,7 +500,8 @@ qemuTPMPcrBankBitmapToStr(virBitmap *activePcrBanks)
* Reconfigure the active PCR banks of a TPM 2.
*/
static int
-qemuTPMEmulatorReconfigure(const char *storagepath,
+qemuTPMEmulatorReconfigure(const virDomainTPMStorage storage_type,
+ const char *storagepath,
uid_t swtpm_user,
gid_t swtpm_group,
virBitmap *activePcrBanks,
@@ -490,6 +513,7 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
int exitstatus;
g_autofree char *activePcrBanksStr = NULL;
g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
+ g_autofree char *tpm_state = qemuTPMGetSwtpmSetupStateArg(storage_type,
storagepath);
if (!swtpm_setup)
return -1;
@@ -510,7 +534,7 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
return -1;
virCommandAddArgList(cmd,
- "--tpm-state", storagepath,
+ "--tpm-state", tpm_state,
"--logfile", logfile,
"--pcr-banks", activePcrBanksStr,
"--reconfigure",
@@ -555,6 +579,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
{
g_autoptr(virCommand) cmd = NULL;
bool created = false;
+ bool run_setup = false;
g_autofree char *swtpm = virTPMGetSwtpm();
int pwdfile_fd = -1;
int migpwdfile_fd = -1;
@@ -565,6 +590,18 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
if (!swtpm)
return NULL;
+ if (tpm->data.emulator.storage_type == VIR_DOMAIN_TPM_STORAGE_FILE) {
+ if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_DIR)) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ _("%1$s does not support file storage"),
+ swtpm);
+ goto error;
+ }
+ create_storage = false;
+ /* setup is run with --not-overwrite */
+ run_setup = true;
+ }
+
/* Do not create storage and run swtpm_setup on incoming migration over
* shared storage
*/
@@ -572,15 +609,18 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
if (incomingMigration && on_shared_storage)
create_storage = false;
- if (create_storage &&
- qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group) < 0)
- return NULL;
+ if (create_storage) {
+ if (qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group) <
0)
+ return NULL;
+ run_setup = created;
+ }
if (tpm->data.emulator.hassecretuuid)
secretuuid = tpm->data.emulator.secretuuid;
- if (created &&
- qemuTPMEmulatorRunSetup(tpm->data.emulator.storagepath, vmname, vmuuid,
+ if (run_setup &&
+ qemuTPMEmulatorRunSetup(tpm->data.emulator.storage_type,
+ tpm->data.emulator.storagepath, vmname, vmuuid,
privileged, swtpm_user, swtpm_group,
tpm->data.emulator.logfile,
tpm->data.emulator.version,
Should we also check that if that file is a block device that it is "big
enough"? Even though I'm not sure if we know what "big enough" means.
If the size depends on the guest, then of course there is no point in
doing that, I'm just wondering if we could avoid some errors by
reporting an issue earlier.
@@ -588,7 +628,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
goto error;
if (!incomingMigration &&
- qemuTPMEmulatorReconfigure(tpm->data.emulator.storagepath,
+ qemuTPMEmulatorReconfigure(tpm->data.emulator.storage_type,
+ tpm->data.emulator.storagepath,
swtpm_user, swtpm_group,
tpm->data.emulator.activePcrBanks,
tpm->data.emulator.logfile,
@@ -607,8 +648,17 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
tpm->data.emulator.source->data.nix.path);
virCommandAddArg(cmd, "--tpmstate");
- virCommandAddArgFormat(cmd, "dir=%s,mode=0600",
- tpm->data.emulator.storagepath);
+ switch (tpm->data.emulator.storage_type) {
+ case VIR_DOMAIN_TPM_STORAGE_FILE:
+ virCommandAddArgFormat(cmd, "backend-uri=file://%s",
+ tpm->data.emulator.storagepath);
+ break;
+ case VIR_DOMAIN_TPM_STORAGE_DIR:
+ case VIR_DOMAIN_TPM_STORAGE_DEFAULT:
This suggests we should rather default to "dir" and not have the third
case (actually named "default") here.
+ virCommandAddArgFormat(cmd, "dir=%s,mode=0600",
+ tpm->data.emulator.storagepath);
+ break;
+ }
virCommandAddArg(cmd, "--log");
if (tpm->data.emulator.debug != 0)
--
2.45.2.827.g557ae147e6
48
days inactive
72
days old
22 comments
5 participants
participants (5)
-
Daniel P. Berrangé -
Marc-André Lureau -
marcandre.lureau＠redhat.com
-
Martin Kletzander -
Stefan Berger