[PATCH v2 1/3] Revert "qemu: explicit swtpm state locking"
From: Marc-André Lureau <marcandre.lureau@redhat.com> This reverts commit bb5e26749fe5b5856a3541be2cbe147701e6e121. swtpm-setup doesn't have "tpmstate-lock", only swtpm. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_tpm.c | 11 ++--------- src/util/virtpm.c | 1 - src/util/virtpm.h | 1 - tests/testutilsqemu.c | 1 - 4 files changed, 2 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index f5e0184e54..476e3dd224 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -344,23 +344,16 @@ static char * qemuTPMGetSwtpmSetupStateArg(const virDomainTPMSourceType source_type, const char *source_path) { - const char *lock = ",lock"; - - if (!virTPMSwtpmSetupCapsGet(VIR_TPM_SWTPM_SETUP_FEATURE_TPMSTATE_OPT_LOCK)) { - VIR_WARN("This swtpm version doesn't support explicit locking"); - lock = ""; - } - switch (source_type) { case VIR_DOMAIN_TPM_SOURCE_TYPE_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%s", source_path, lock); + return g_strdup_printf("file://%s", source_path); case VIR_DOMAIN_TPM_SOURCE_TYPE_DIR: case VIR_DOMAIN_TPM_SOURCE_TYPE_DEFAULT: case VIR_DOMAIN_TPM_SOURCE_TYPE_LAST: default: - return g_strdup_printf("%s%s", source_path, lock); + return g_strdup_printf("%s", source_path); } } diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 4016ad8fc4..f90839debe 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -53,7 +53,6 @@ VIR_ENUM_IMPL(virTPMSwtpmSetupFeature, "cmdarg-reconfigure-pcr-banks", "tpm-1.2", "tpm-2.0", - "tpmstate-opt-lock", "cmdarg-profile", ); diff --git a/src/util/virtpm.h b/src/util/virtpm.h index 03fb92629a..4119a903e5 100644 --- a/src/util/virtpm.h +++ b/src/util/virtpm.h @@ -45,7 +45,6 @@ typedef enum { VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS, VIR_TPM_SWTPM_SETUP_FEATURE_TPM_1_2, VIR_TPM_SWTPM_SETUP_FEATURE_TPM_2_0, - VIR_TPM_SWTPM_SETUP_FEATURE_TPMSTATE_OPT_LOCK, VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PROFILE, VIR_TPM_SWTPM_SETUP_FEATURE_LAST diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 5caccbc6b4..abc425b9b7 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -71,7 +71,6 @@ virTPMSwtpmSetupCapsGet(virTPMSwtpmSetupFeature cap) case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES: case VIR_TPM_SWTPM_SETUP_FEATURE_TPM12_NOT_NEED_ROOT: case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS: - case VIR_TPM_SWTPM_SETUP_FEATURE_TPMSTATE_OPT_LOCK: case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PROFILE: case VIR_TPM_SWTPM_SETUP_FEATURE_LAST: break; -- 2.47.0
From: Marc-André Lureau <marcandre.lureau@redhat.com> Commit bb5e26749fe5b ("qemu: explicit swtpm state locking") attempted to lock the state, but only for swtpm-setup. The capability "tpmstate-opt-lock" is actually only exposed by swtpm. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_tpm.c | 15 +++++++++++---- src/util/virtpm.c | 1 + src/util/virtpm.h | 1 + 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 476e3dd224..942ee64263 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -606,17 +606,24 @@ static void qemuTPMVirCommandSwtpmAddTPMState(virCommand *cmd, const virDomainTPMEmulatorDef *emulator) { + const char *lock = ",lock"; + + if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_TPMSTATE_OPT_LOCK)) { + VIR_WARN("This swtpm version doesn't support explicit locking"); + lock = ""; + } + virCommandAddArg(cmd, "--tpmstate"); switch (emulator->source_type) { case VIR_DOMAIN_TPM_SOURCE_TYPE_FILE: - virCommandAddArgFormat(cmd, "backend-uri=file://%s", - emulator->source_path); + virCommandAddArgFormat(cmd, "backend-uri=file://%s%s", + emulator->source_path, lock); break; case VIR_DOMAIN_TPM_SOURCE_TYPE_DIR: case VIR_DOMAIN_TPM_SOURCE_TYPE_DEFAULT: case VIR_DOMAIN_TPM_SOURCE_TYPE_LAST: - virCommandAddArgFormat(cmd, "dir=%s,mode=0600", - emulator->source_path); + virCommandAddArgFormat(cmd, "dir=%s,mode=0600%s", + emulator->source_path, lock); break; } } diff --git a/src/util/virtpm.c b/src/util/virtpm.c index f90839debe..cf0f20e009 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -43,6 +43,7 @@ VIR_ENUM_IMPL(virTPMSwtpmFeature, "nvram-backend-dir", "nvram-backend-file", "cmdarg-print-info", + "tpmstate-opt-lock", ); VIR_ENUM_IMPL(virTPMSwtpmSetupFeature, diff --git a/src/util/virtpm.h b/src/util/virtpm.h index 4119a903e5..c741d28465 100644 --- a/src/util/virtpm.h +++ b/src/util/virtpm.h @@ -34,6 +34,7 @@ typedef enum { VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_DIR, VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_FILE, VIR_TPM_SWTPM_FEATURE_CMDARG_PRINT_INFO, + VIR_TPM_SWTPM_FEATURE_TPMSTATE_OPT_LOCK, VIR_TPM_SWTPM_FEATURE_LAST } virTPMSwtpmFeature; -- 2.47.0
From: Marc-André Lureau <marcandre.lureau@redhat.com> When the vTPM source path is specified, such as: <source type=".." path="/my/tpm"/> Do not delete the parent directory, but only the given file/dir. Fixes: commit f1304cc566 ("qemu_tpm: handle file/block storage source") Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_tpm.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 942ee64263..3e97518c06 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -214,9 +214,31 @@ qemuTPMEmulatorCreateStorage(virDomainTPMDef *tpm, static void qemuTPMEmulatorDeleteStorage(virDomainTPMDef *tpm) { - g_autofree char *path = g_path_get_dirname(tpm->data.emulator.source_path); + const char *source_path = tpm->data.emulator.source_path; + + switch (tpm->data.emulator.source_type) { + case VIR_DOMAIN_TPM_SOURCE_TYPE_FILE: { + if (unlink(source_path) && errno != ENOENT) + virReportSystemError(errno, + _("Cannot delete file '%1$s'"), + source_path); + break; + } + + case VIR_DOMAIN_TPM_SOURCE_TYPE_DIR: { + ignore_value(virFileDeleteTree(source_path)); + break; + } - ignore_value(virFileDeleteTree(path)); + case VIR_DOMAIN_TPM_SOURCE_TYPE_DEFAULT: + case VIR_DOMAIN_TPM_SOURCE_TYPE_LAST: + default: { + g_autofree char *vm_uuid_dir = g_path_get_dirname(source_path); + + ignore_value(virFileDeleteTree(vm_uuid_dir)); + } + + } } -- 2.47.0
On 12/11/24 5:37 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
When the vTPM source path is specified, such as: <source type=".." path="/my/tpm"/>
Do not delete the parent directory, but only the given file/dir.
Fixes: commit f1304cc566 ("qemu_tpm: handle file/block storage source")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested this and directory for default backend and also file for new file backend are being deleted now. Tested-by: Stefan Berger <stefanb@linux.ibm.com>
--- src/qemu/qemu_tpm.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 942ee64263..3e97518c06 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -214,9 +214,31 @@ qemuTPMEmulatorCreateStorage(virDomainTPMDef *tpm, static void qemuTPMEmulatorDeleteStorage(virDomainTPMDef *tpm) { - g_autofree char *path = g_path_get_dirname(tpm->data.emulator.source_path); + const char *source_path = tpm->data.emulator.source_path; + + switch (tpm->data.emulator.source_type) { + case VIR_DOMAIN_TPM_SOURCE_TYPE_FILE: { + if (unlink(source_path) && errno != ENOENT) + virReportSystemError(errno, + _("Cannot delete file '%1$s'"), + source_path); + break; + } + + case VIR_DOMAIN_TPM_SOURCE_TYPE_DIR: { + ignore_value(virFileDeleteTree(source_path)); + break; + }
- ignore_value(virFileDeleteTree(path)); + case VIR_DOMAIN_TPM_SOURCE_TYPE_DEFAULT: + case VIR_DOMAIN_TPM_SOURCE_TYPE_LAST: + default: { + g_autofree char *vm_uuid_dir = g_path_get_dirname(source_path); + + ignore_value(virFileDeleteTree(vm_uuid_dir)); + } + + } }
On 12/11/24 5:37 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
This reverts commit bb5e26749fe5b5856a3541be2cbe147701e6e121.
swtpm-setup doesn't have "tpmstate-lock", only swtpm.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
--- src/qemu/qemu_tpm.c | 11 ++--------- src/util/virtpm.c | 1 - src/util/virtpm.h | 1 - tests/testutilsqemu.c | 1 - 4 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index f5e0184e54..476e3dd224 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -344,23 +344,16 @@ static char * qemuTPMGetSwtpmSetupStateArg(const virDomainTPMSourceType source_type, const char *source_path) { - const char *lock = ",lock"; - - if (!virTPMSwtpmSetupCapsGet(VIR_TPM_SWTPM_SETUP_FEATURE_TPMSTATE_OPT_LOCK)) { - VIR_WARN("This swtpm version doesn't support explicit locking"); - lock = ""; - } - switch (source_type) { case VIR_DOMAIN_TPM_SOURCE_TYPE_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%s", source_path, lock); + return g_strdup_printf("file://%s", source_path); case VIR_DOMAIN_TPM_SOURCE_TYPE_DIR: case VIR_DOMAIN_TPM_SOURCE_TYPE_DEFAULT: case VIR_DOMAIN_TPM_SOURCE_TYPE_LAST: default: - return g_strdup_printf("%s%s", source_path, lock); + return g_strdup_printf("%s", source_path); } }
diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 4016ad8fc4..f90839debe 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -53,7 +53,6 @@ VIR_ENUM_IMPL(virTPMSwtpmSetupFeature, "cmdarg-reconfigure-pcr-banks", "tpm-1.2", "tpm-2.0", - "tpmstate-opt-lock", "cmdarg-profile", );
diff --git a/src/util/virtpm.h b/src/util/virtpm.h index 03fb92629a..4119a903e5 100644 --- a/src/util/virtpm.h +++ b/src/util/virtpm.h @@ -45,7 +45,6 @@ typedef enum { VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS, VIR_TPM_SWTPM_SETUP_FEATURE_TPM_1_2, VIR_TPM_SWTPM_SETUP_FEATURE_TPM_2_0, - VIR_TPM_SWTPM_SETUP_FEATURE_TPMSTATE_OPT_LOCK, VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PROFILE,
VIR_TPM_SWTPM_SETUP_FEATURE_LAST diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 5caccbc6b4..abc425b9b7 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -71,7 +71,6 @@ virTPMSwtpmSetupCapsGet(virTPMSwtpmSetupFeature cap) case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES: case VIR_TPM_SWTPM_SETUP_FEATURE_TPM12_NOT_NEED_ROOT: case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS: - case VIR_TPM_SWTPM_SETUP_FEATURE_TPMSTATE_OPT_LOCK: case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PROFILE: case VIR_TPM_SWTPM_SETUP_FEATURE_LAST: break;
Hi On Wed, Dec 11, 2024 at 5:30 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
On 12/11/24 5:37 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
This reverts commit bb5e26749fe5b5856a3541be2cbe147701e6e121.
swtpm-setup doesn't have "tpmstate-lock", only swtpm.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Can someone queue the series? thanks
--- src/qemu/qemu_tpm.c | 11 ++--------- src/util/virtpm.c | 1 - src/util/virtpm.h | 1 - tests/testutilsqemu.c | 1 - 4 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index f5e0184e54..476e3dd224 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -344,23 +344,16 @@ static char * qemuTPMGetSwtpmSetupStateArg(const virDomainTPMSourceType source_type, const char *source_path) { - const char *lock = ",lock"; - - if (!virTPMSwtpmSetupCapsGet(VIR_TPM_SWTPM_SETUP_FEATURE_TPMSTATE_OPT_LOCK)) { - VIR_WARN("This swtpm version doesn't support explicit locking"); - lock = ""; - } - switch (source_type) { case VIR_DOMAIN_TPM_SOURCE_TYPE_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%s", source_path, lock); + return g_strdup_printf("file://%s", source_path); case VIR_DOMAIN_TPM_SOURCE_TYPE_DIR: case VIR_DOMAIN_TPM_SOURCE_TYPE_DEFAULT: case VIR_DOMAIN_TPM_SOURCE_TYPE_LAST: default: - return g_strdup_printf("%s%s", source_path, lock); + return g_strdup_printf("%s", source_path); } }
diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 4016ad8fc4..f90839debe 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -53,7 +53,6 @@ VIR_ENUM_IMPL(virTPMSwtpmSetupFeature, "cmdarg-reconfigure-pcr-banks", "tpm-1.2", "tpm-2.0", - "tpmstate-opt-lock", "cmdarg-profile", );
diff --git a/src/util/virtpm.h b/src/util/virtpm.h index 03fb92629a..4119a903e5 100644 --- a/src/util/virtpm.h +++ b/src/util/virtpm.h @@ -45,7 +45,6 @@ typedef enum { VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS, VIR_TPM_SWTPM_SETUP_FEATURE_TPM_1_2, VIR_TPM_SWTPM_SETUP_FEATURE_TPM_2_0, - VIR_TPM_SWTPM_SETUP_FEATURE_TPMSTATE_OPT_LOCK, VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PROFILE,
VIR_TPM_SWTPM_SETUP_FEATURE_LAST diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 5caccbc6b4..abc425b9b7 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -71,7 +71,6 @@ virTPMSwtpmSetupCapsGet(virTPMSwtpmSetupFeature cap) case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES: case VIR_TPM_SWTPM_SETUP_FEATURE_TPM12_NOT_NEED_ROOT: case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS: - case VIR_TPM_SWTPM_SETUP_FEATURE_TPMSTATE_OPT_LOCK: case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PROFILE: case VIR_TPM_SWTPM_SETUP_FEATURE_LAST: break;
-- Marc-André Lureau
On 12/30/24 12:56, Marc-André Lureau wrote:
Hi
On Wed, Dec 11, 2024 at 5:30 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
On 12/11/24 5:37 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
This reverts commit bb5e26749fe5b5856a3541be2cbe147701e6e121.
swtpm-setup doesn't have "tpmstate-lock", only swtpm.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Can someone queue the series? thanks
Oh yes, sorry. Series: Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and merged. Michal
participants (4)
-
Marc-André Lureau -
marcandre.lureau＠redhat.com -
Michal Prívozník -
Stefan Berger