1:32 p.m.
This fixes tpm-tis usage for armv7l and riscv arches, and then
switches qemu tpm validation to use domcaps as the source of truth
Cole Robinson (4):
qemu: validate: Drop tpm-tis arch validation
qemu: command: Use correct tpm device for all non-x86
tests: mock swtpm initialization for all qemu tests
qemu: validate: use domcaps for tpm validation
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_validate.c | 77 +++++++++-------------------------------
tests/domaincapstest.c | 7 ----
tests/testutilsqemu.c | 8 +++++
4 files changed, 26 insertions(+), 68 deletions(-)
--
2.36.1
1:32 p.m.
New subject: [PATCH 1/4] qemu: validate: Drop tpm-tis arch validation
Checking against qemu capabilities should be enough here
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/321
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu/qemu_validate.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index f8aa83c1cb..db47fcaa9c 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4798,12 +4798,6 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
switch (tpm->model) {
case VIR_DOMAIN_TPM_MODEL_TIS:
- if (!ARCH_IS_X86(def->os.arch) && (def->os.arch !=
VIR_ARCH_AARCH64)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("TPM model '%s' is only available for x86 and
aarch64 guests"),
- virDomainTPMModelTypeToString(tpm->model));
- return -1;
- }
flag = QEMU_CAPS_DEVICE_TPM_TIS;
break;
case VIR_DOMAIN_TPM_MODEL_CRB:
--
2.36.1
1:37 p.m.
New subject: [PATCH 1/4] qemu: validate: Drop tpm-tis arch validation
On 6/18/22 2:32 PM, Cole Robinson wrote:
Checking against qemu capabilities should be enough here
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/321
Should be https://gitlab.com/libvirt/libvirt/-/issues/329
I've fixed locally
1:32 p.m.
New subject: [PATCH 2/4] qemu: command: Use correct tpm device for all non-x86
The qemu `tpm-tis` device is an ISA device, so only really applicable
to x86 archs. For all non-x86 archs we should use `tpm-tis-device`
This fixes tpm-tis usage on armv7l and riscv
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu/qemu_command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 57334ab246..b307d3139c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9731,7 +9731,7 @@ qemuBuildTPMDevCmd(virCommand *cmd,
const char *model = virDomainTPMModelTypeToString(tpm->model);
g_autofree char *tpmdev = g_strdup_printf("tpm-%s", tpm->info.alias);
- if (tpm->model == VIR_DOMAIN_TPM_MODEL_TIS && def->os.arch ==
VIR_ARCH_AARCH64)
+ if (tpm->model == VIR_DOMAIN_TPM_MODEL_TIS &&
!ARCH_IS_X86(def->os.arch))
model = "tpm-tis-device";
if (virJSONValueObjectAdd(&props,
--
2.36.1
1:32 p.m.
New subject: [PATCH 3/4] tests: mock swtpm initialization for all qemu tests
Don't restrict this to domcaps testing only, we will soon
need it for qemu command line validation
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/domaincapstest.c | 7 -------
tests/testutilsqemu.c | 8 ++++++++
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index da5c629fd4..3b8216a8f6 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -22,7 +22,6 @@
#include "domain_capabilities.h"
#include "virfilewrapper.h"
#include "configmake.h"
-#include "virtpm.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -129,12 +128,6 @@ fillQemuCaps(virDomainCaps *domCaps,
}
-/* Enough to tell capabilities code that swtpm is usable */
-bool virTPMHasSwtpm(void)
-{
- return true;
-}
-
#endif /* WITH_QEMU */
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index d26caba5fb..6dabbaf36a 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -11,6 +11,7 @@
# include "qemu/qemu_capspriv.h"
# include "virstring.h"
# include "virfilecache.h"
+# include "virtpm.h"
# include <sys/types.h>
# include <fcntl.h>
@@ -138,6 +139,13 @@ virFindFileInPath(const char *file)
}
+/* Enough to tell capabilities code that swtpm is usable */
+bool virTPMHasSwtpm(void)
+{
+ return true;
+}
+
+
virCapsHostNUMA *
virCapabilitiesHostNUMANewHost(void)
{
--
2.36.1
1:32 p.m.
New subject: [PATCH 4/4] qemu: validate: use domcaps for tpm validation
Replace tpm->type and tpm->model qemuCaps validation with the
similar logic in domcaps.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu/qemu_validate.c | 71 ++++++++++------------------------------
1 file changed, 17 insertions(+), 54 deletions(-)
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index db47fcaa9c..39210ba65b 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4750,7 +4750,7 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
const virDomainDef *def,
virQEMUCaps *qemuCaps)
{
- virQEMUCapsFlags flag;
+ virDomainCapsDeviceTPM tpmCaps = { 0 };
switch (tpm->version) {
case VIR_DOMAIN_TPM_VERSION_1_2:
@@ -4781,57 +4781,28 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
break;
}
- switch (tpm->type) {
- case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_PASSTHROUGH))
- goto no_support;
- break;
-
- case VIR_DOMAIN_TPM_TYPE_EMULATOR:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_EMULATOR))
- goto no_support;
+ virQEMUCapsFillDomainDeviceTPMCaps(qemuCaps, &tpmCaps);
- break;
- case VIR_DOMAIN_TPM_TYPE_LAST:
- break;
+ if (!VIR_DOMAIN_CAPS_ENUM_IS_SET(tpmCaps.backendModel, tpm->type)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("The QEMU executable %s does not support TPM "
+ "backend type %s"),
+ def->emulator,
+ virDomainTPMBackendTypeToString(tpm->type));
+ return -1;
}
- switch (tpm->model) {
- case VIR_DOMAIN_TPM_MODEL_TIS:
- flag = QEMU_CAPS_DEVICE_TPM_TIS;
- break;
- case VIR_DOMAIN_TPM_MODEL_CRB:
- flag = QEMU_CAPS_DEVICE_TPM_CRB;
- break;
- case VIR_DOMAIN_TPM_MODEL_SPAPR:
- flag = QEMU_CAPS_DEVICE_TPM_SPAPR;
- break;
- case VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY:
- if (!ARCH_IS_PPC64(def->os.arch)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("TPM Proxy model %s is only available for "
- "PPC64 guests"),
- virDomainTPMModelTypeToString(tpm->model));
- return -1;
- }
-
- /* TPM Proxy devices have 'passthrough' backend */
- if (tpm->type != VIR_DOMAIN_TPM_TYPE_PASSTHROUGH) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("TPM Proxy model %s requires "
- "'Passthrough' backend"),
- virDomainTPMModelTypeToString(tpm->model));
- }
-
- flag = QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY;
- break;
- case VIR_DOMAIN_TPM_MODEL_LAST:
- default:
- virReportEnumRangeError(virDomainTPMModel, tpm->model);
+ if (ARCH_IS_PPC64(def->os.arch) &&
+ tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY &&
+ tpm->type != VIR_DOMAIN_TPM_TYPE_PASSTHROUGH) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("TPM Proxy model %s requires "
+ "'Passthrough' backend"),
+ virDomainTPMModelTypeToString(tpm->model));
return -1;
}
- if (!virQEMUCapsGet(qemuCaps, flag)) {
+ if (!VIR_DOMAIN_CAPS_ENUM_IS_SET(tpmCaps.model, tpm->model)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("The QEMU executable %s does not support TPM "
"model %s"),
@@ -4841,14 +4812,6 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
}
return 0;
-
- no_support:
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("The QEMU executable %s does not support TPM "
- "backend type %s"),
- def->emulator,
- virDomainTPMBackendTypeToString(tpm->type));
- return -1;
}
--
2.36.1
3:11 a.m.
New subject: [PATCH 4/4] qemu: validate: use domcaps for tpm validation
On 6/18/22 20:32, Cole Robinson wrote:
Replace tpm->type and tpm->model qemuCaps validation with the
similar logic in domcaps.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu/qemu_validate.c | 71 ++++++++++------------------------------
1 file changed, 17 insertions(+), 54 deletions(-)
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index db47fcaa9c..39210ba65b 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4750,7 +4750,7 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
const virDomainDef *def,
virQEMUCaps *qemuCaps)
{
- virQEMUCapsFlags flag;
+ virDomainCapsDeviceTPM tpmCaps = { 0 };
switch (tpm->version) {
case VIR_DOMAIN_TPM_VERSION_1_2:
@@ -4781,57 +4781,28 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
break;
}
- switch (tpm->type) {
- case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_PASSTHROUGH))
- goto no_support;
- break;
-
- case VIR_DOMAIN_TPM_TYPE_EMULATOR:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_EMULATOR))
- goto no_support;
+ virQEMUCapsFillDomainDeviceTPMCaps(qemuCaps, &tpmCaps);
- break;
- case VIR_DOMAIN_TPM_TYPE_LAST:
- break;
+ if (!VIR_DOMAIN_CAPS_ENUM_IS_SET(tpmCaps.backendModel, tpm->type)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("The QEMU executable %s does not support TPM "
+ "backend type %s"),
+ def->emulator,
+ virDomainTPMBackendTypeToString(tpm->type));
Whoa, very nice idea! And looking around the file I can see it used
already. How could this slipped by me? I mean, the more I think about it
the more possibilities for code deduplication I see. And on the flip
side - we would be motivated to keep domcaps on the bleeding edge.
Michal
7:18 a.m.
New subject: [PATCH 4/4] qemu: validate: use domcaps for tpm validation
On 6/21/22 4:11 AM, Michal Prívozník wrote:
On 6/18/22 20:32, Cole Robinson wrote:
> Replace tpm->type and tpm->model qemuCaps validation with the
> similar logic in domcaps.
>
> Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
> ---
> src/qemu/qemu_validate.c | 71 ++++++++++------------------------------
> 1 file changed, 17 insertions(+), 54 deletions(-)
>
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index db47fcaa9c..39210ba65b 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -4750,7 +4750,7 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
> const virDomainDef *def,
> virQEMUCaps *qemuCaps)
> {
> - virQEMUCapsFlags flag;
> + virDomainCapsDeviceTPM tpmCaps = { 0 };
>
> switch (tpm->version) {
> case VIR_DOMAIN_TPM_VERSION_1_2:
> @@ -4781,57 +4781,28 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
> break;
> }
>
> - switch (tpm->type) {
> - case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_PASSTHROUGH))
> - goto no_support;
> - break;
> -
> - case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_EMULATOR))
> - goto no_support;
> + virQEMUCapsFillDomainDeviceTPMCaps(qemuCaps, &tpmCaps);
>
> - break;
> - case VIR_DOMAIN_TPM_TYPE_LAST:
> - break;
> + if (!VIR_DOMAIN_CAPS_ENUM_IS_SET(tpmCaps.backendModel, tpm->type)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("The QEMU executable %s does not support TPM "
> + "backend type %s"),
> + def->emulator,
> + virDomainTPMBackendTypeToString(tpm->type));
Whoa, very nice idea! And looking around the file I can see it used
already. How could this slipped by me? I mean, the more I think about it
the more possibilities for code deduplication I see. And on the flip
side - we would be motivated to keep domcaps on the bleeding edge.
Heh, it's your code :)
https://listman.redhat.com/archives/libvir-list/2020-November/211844.html
But yes I agree. domcaps can be a detriment to apps if it's not
reliable, and duplicating the qemuCaps checking is always going to lead
to issues like this. It would be nice if we could normalize adding
domcaps coverage for basic qemuCaps validation cases like this.
Thanks,
Cole
3:11 a.m.
On 6/18/22 20:32, Cole Robinson wrote:
This fixes tpm-tis usage for armv7l and riscv arches, and then
switches qemu tpm validation to use domcaps as the source of truth
Cole Robinson (4):
qemu: validate: Drop tpm-tis arch validation
qemu: command: Use correct tpm device for all non-x86
tests: mock swtpm initialization for all qemu tests
qemu: validate: use domcaps for tpm validation
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_validate.c | 77 +++++++++-------------------------------
tests/domaincapstest.c | 7 ----
tests/testutilsqemu.c | 8 +++++
4 files changed, 26 insertions(+), 68 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
933
days inactive
936
days old
8 comments
2 participants
participants (2)
-
Cole Robinson -
Michal Prívozník