4:30 a.m.
*** BLURB HERE ***
Michal Prívozník (8):
conf: Report an error when default TPM model is provided
conf: Report error when default TPM version is provided
conf: Drop needless setting of VIR_DOMAIN_TPM_VERSION_DEFAULT
conf: Move _virDomainTPMDef::version into
_virDomainTPMDef::data::emulator
conf: Use virXMLPropEnum more when parsing TPM
qemu_domain: Move TPM post parse code into qemuDomainTPMDefPostParse()
qemu: Move TPMs validation out of PostParse
conf: Don't lose <active_pcr_banks/> when no TPM version is provided
src/conf/domain_conf.c | 81 +++++++++++++-------------------
src/conf/domain_conf.h | 10 ++--
src/conf/domain_validate.c | 28 ++++++++++-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_domain.c | 63 ++++++-------------------
src/qemu/qemu_tpm.c | 10 ++--
src/qemu/qemu_validate.c | 87 ++++++++++++++++++++++++-----------
src/security/virt-aa-helper.c | 2 +-
8 files changed, 147 insertions(+), 136 deletions(-)
--
2.35.1
4:30 a.m.
New subject: [PATCH 1/8] conf: Report an error when default TPM model is provided
When "default" model of a TPM was provided, our parses accepts it
happily even though the value is forbidden by our RNG and not
documented as accepted value. This is because of < 0 vs <= 0
comparison of virDomainTPMModelTypeFromString() retval.
Make the parser error out explicitly in this case. Users can
always chose to not specify the attribute in which case we pick a
sane default (in qemuDomainTPMDefPostParse()).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 2 +-
src/conf/domain_conf.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4c7a5a044c..b7147945da 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10360,7 +10360,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
model = virXMLPropString(node, "model");
if (model != NULL &&
- (def->model = virDomainTPMModelTypeFromString(model)) < 0) {
+ (def->model = virDomainTPMModelTypeFromString(model)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unknown TPM frontend model '%s'"), model);
goto error;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 90de50c12f..5a057c36b8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1400,7 +1400,7 @@ struct _virDomainHubDef {
};
typedef enum {
- VIR_DOMAIN_TPM_MODEL_DEFAULT,
+ VIR_DOMAIN_TPM_MODEL_DEFAULT = 0,
VIR_DOMAIN_TPM_MODEL_TIS,
VIR_DOMAIN_TPM_MODEL_CRB,
VIR_DOMAIN_TPM_MODEL_SPAPR,
--
2.35.1
6:10 a.m.
New subject: [PATCH 1/8] conf: Report an error when default TPM model is provided
On Mon, Jul 18, 2022 at 11:30:43 +0200, Michal Privoznik wrote:
When "default" model of a TPM was provided, our parses
accepts it
happily even though the value is forbidden by our RNG and not
documented as accepted value. This is because of < 0 vs <= 0
comparison of virDomainTPMModelTypeFromString() retval.
Make the parser error out explicitly in this case. Users can
always chose to not specify the attribute in which case we pick a
sane default (in qemuDomainTPMDefPostParse()).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 2 +-
src/conf/domain_conf.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4c7a5a044c..b7147945da 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10360,7 +10360,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
model = virXMLPropString(node, "model");
if (model != NULL &&
- (def->model = virDomainTPMModelTypeFromString(model)) < 0) {
+ (def->model = virDomainTPMModelTypeFromString(model)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unknown TPM frontend model '%s'"), model);
goto error;
'virDomainTPMDefFormat' happily formats 'default' as supported type:
virBufferAsprintf(&attrBuf, " model='%s'",
virDomainTPMModelTypeToString(def->model));
Is there any other code path which would forbid 'default'? If no, then
we might run into a situation where libvirt's parser would reject
parsing a XML formatted by libvirt itself, which is not acceptable.
In such case we'd need to leave the parser as-is and add just validation
where 'defau't will be forbidden, which is acceptable.
8:08 a.m.
New subject: [PATCH 1/8] conf: Report an error when default TPM model is provided
On 8/1/22 13:10, Peter Krempa wrote:
On Mon, Jul 18, 2022 at 11:30:43 +0200, Michal Privoznik wrote:
> When "default" model of a TPM was provided, our parses accepts it
> happily even though the value is forbidden by our RNG and not
> documented as accepted value. This is because of < 0 vs <= 0
> comparison of virDomainTPMModelTypeFromString() retval.
>
> Make the parser error out explicitly in this case. Users can
> always chose to not specify the attribute in which case we pick a
> sane default (in qemuDomainTPMDefPostParse()).
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/conf/domain_conf.c | 2 +-
> src/conf/domain_conf.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 4c7a5a044c..b7147945da 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10360,7 +10360,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
>
> model = virXMLPropString(node, "model");
> if (model != NULL &&
> - (def->model = virDomainTPMModelTypeFromString(model)) < 0) {
> + (def->model = virDomainTPMModelTypeFromString(model)) <= 0) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("Unknown TPM frontend model '%s'"),
model);
> goto error;
'virDomainTPMDefFormat' happily formats 'default' as supported type:
virBufferAsprintf(&attrBuf, " model='%s'",
virDomainTPMModelTypeToString(def->model));
Is there any other code path which would forbid 'default'?
Couple of them, actually. The first one is in
qemuValidateDomainDeviceDefTPM() where
virQEMUCapsFillDomainDeviceTPMCaps(qemuCaps, &tpmCaps); is called
followed by if (!VIR_DOMAIN_CAPS_ENUM_IS_SET(tpmCaps.model, tpm->model))
{}. And the second is qemuDomainTPMDefPostParse() which overwrites the
_DEFAULT to either _TIS or _SPAPR (alright, this is not a check that
forbids 'default' per se).
If no, then
we might run into a situation where libvirt's parser would reject
parsing a XML formatted by libvirt itself, which is not acceptable.
In such case we'd need to leave the parser as-is and add just validation
where 'defau't will be forbidden, which is acceptable.
Michal
9:29 a.m.
New subject: [PATCH 1/8] conf: Report an error when default TPM model is provided
On Mon, Aug 01, 2022 at 15:08:23 +0200, Michal Prívozník wrote:
On 8/1/22 13:10, Peter Krempa wrote:
> On Mon, Jul 18, 2022 at 11:30:43 +0200, Michal Privoznik wrote:
>> When "default" model of a TPM was provided, our parses accepts it
>> happily even though the value is forbidden by our RNG and not
>> documented as accepted value. This is because of < 0 vs <= 0
>> comparison of virDomainTPMModelTypeFromString() retval.
>>
>> Make the parser error out explicitly in this case. Users can
>> always chose to not specify the attribute in which case we pick a
>> sane default (in qemuDomainTPMDefPostParse()).
>>
>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>> ---
>> src/conf/domain_conf.c | 2 +-
>> src/conf/domain_conf.h | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 4c7a5a044c..b7147945da 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -10360,7 +10360,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
>>
>> model = virXMLPropString(node, "model");
>> if (model != NULL &&
>> - (def->model = virDomainTPMModelTypeFromString(model)) < 0) {
>> + (def->model = virDomainTPMModelTypeFromString(model)) <= 0) {
>> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> _("Unknown TPM frontend model '%s'"),
model);
>> goto error;
>
> 'virDomainTPMDefFormat' happily formats 'default' as supported
type:
>
> virBufferAsprintf(&attrBuf, " model='%s'",
> virDomainTPMModelTypeToString(def->model));
>
> Is there any other code path which would forbid 'default'?
Couple of them, actually. The first one is in
qemuValidateDomainDeviceDefTPM() where
virQEMUCapsFillDomainDeviceTPMCaps(qemuCaps, &tpmCaps); is called
followed by if (!VIR_DOMAIN_CAPS_ENUM_IS_SET(tpmCaps.model, tpm->model))
{}. And the second is qemuDomainTPMDefPostParse() which overwrites the
_DEFAULT to either _TIS or _SPAPR (alright, this is not a check that
forbids 'default' per se).
Taken very strictly this would not apply for other drivers though, which
in most cases don't even reject TPM devices, but that would be a very
theoretical excercise.
If you want to go ahead and make the parser more strict, which should be
most likely okay in any reasonable case, you should in such case modify
the formatter to skip the _DEFAULT values so they don't end up in the
XML accidentally and then make it un-parsable.
9:59 a.m.
New subject: [PATCH 1/8] conf: Report an error when default TPM model is provided
On 8/1/22 16:29, Peter Krempa wrote:
On Mon, Aug 01, 2022 at 15:08:23 +0200, Michal Prívozník wrote:
> On 8/1/22 13:10, Peter Krempa wrote:
>> On Mon, Jul 18, 2022 at 11:30:43 +0200, Michal Privoznik wrote:
>>> When "default" model of a TPM was provided, our parses accepts it
>>> happily even though the value is forbidden by our RNG and not
>>> documented as accepted value. This is because of < 0 vs <= 0
>>> comparison of virDomainTPMModelTypeFromString() retval.
>>>
>>> Make the parser error out explicitly in this case. Users can
>>> always chose to not specify the attribute in which case we pick a
>>> sane default (in qemuDomainTPMDefPostParse()).
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>>> ---
>>> src/conf/domain_conf.c | 2 +-
>>> src/conf/domain_conf.h | 2 +-
>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index 4c7a5a044c..b7147945da 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>> @@ -10360,7 +10360,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
>>>
>>> model = virXMLPropString(node, "model");
>>> if (model != NULL &&
>>> - (def->model = virDomainTPMModelTypeFromString(model)) < 0) {
>>> + (def->model = virDomainTPMModelTypeFromString(model)) <= 0) {
>>> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>> _("Unknown TPM frontend model
'%s'"), model);
>>> goto error;
>>
>> 'virDomainTPMDefFormat' happily formats 'default' as supported
type:
>>
>> virBufferAsprintf(&attrBuf, " model='%s'",
>> virDomainTPMModelTypeToString(def->model));
>>
>> Is there any other code path which would forbid 'default'?
>
> Couple of them, actually. The first one is in
> qemuValidateDomainDeviceDefTPM() where
> virQEMUCapsFillDomainDeviceTPMCaps(qemuCaps, &tpmCaps); is called
> followed by if (!VIR_DOMAIN_CAPS_ENUM_IS_SET(tpmCaps.model, tpm->model))
> {}. And the second is qemuDomainTPMDefPostParse() which overwrites the
> _DEFAULT to either _TIS or _SPAPR (alright, this is not a check that
> forbids 'default' per se).
Taken very strictly this would not apply for other drivers though, which
in most cases don't even reject TPM devices, but that would be a very
theoretical excercise.
True, no other driver rejects TPM, but at the same time no other driver
supports it. I believe our policy was to do nothing in such case. We
tell users to try out their XML first to see whether a device is
supported. TPM is not alone in this.
If you want to go ahead and make the parser more strict, which should be
most likely okay in any reasonable case, you should in such case modify
the formatter to skip the _DEFAULT values so they don't end up in the
XML accidentally and then make it un-parsable.
Well, if _DEFAULT model (or _DEFAULT version from our discussion to 2/8)
was provided then already users would have to specifically ignore schema
validation AND our documentation. But yes, it is possible to define the
following XML:
<tpm model='default'>
<backend type='emulator' version='default'/>
</tpm>
for say the test driver, which lacks those default -> sensible value
post parse callbacks. In this case we would format XML and won't be able
to parse it back. Except for QEMU driver.
But okay, consider the following squashed into their respective commits:
diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index 83db800f51..1647692ea2 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -24230,8 +24230,10 @@ virDomainTPMDefFormat(virBuffer *buf,
g_auto(virBuffer) backendAttrBuf = VIR_BUFFER_INITIALIZER;
g_auto(virBuffer) backendChildBuf = VIR_BUFFER_INIT_CHILD(&childBuf);
- virBufferAsprintf(&attrBuf, " model='%s'",
- virDomainTPMModelTypeToString(def->model));
+ if (def->model) {
+ virBufferAsprintf(&attrBuf, " model='%s'",
+ virDomainTPMModelTypeToString(def->model));
+ }
virBufferAsprintf(&backendAttrBuf, " type='%s'",
virDomainTPMBackendTypeToString(def->type));
@@ -24242,8 +24244,10 @@ virDomainTPMDefFormat(virBuffer *buf,
def->data.passthrough.source->data.file.path);
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
- virBufferAsprintf(&backendAttrBuf, " version='%s'",
- virDomainTPMVersionTypeToString(def->version));
+ if (def->version) {
+ virBufferAsprintf(&backendAttrBuf, " version='%s'",
+ virDomainTPMVersionTypeToString(def->version));
+ }
if (def->data.emulator.persistent_state)
virBufferAddLit(&backendAttrBuf, "
persistent_state='yes'");
if (def->data.emulator.hassecretuuid) {
Michal
10:12 a.m.
New subject: [PATCH 1/8] conf: Report an error when default TPM model is provided
On Mon, Aug 01, 2022 at 16:59:26 +0200, Michal Prívozník wrote:
On 8/1/22 16:29, Peter Krempa wrote:
> On Mon, Aug 01, 2022 at 15:08:23 +0200, Michal Prívozník wrote:
>> On 8/1/22 13:10, Peter Krempa wrote:
>>> On Mon, Jul 18, 2022 at 11:30:43 +0200, Michal Privoznik wrote:
>>>> When "default" model of a TPM was provided, our parses accepts
it
>>>> happily even though the value is forbidden by our RNG and not
>>>> documented as accepted value. This is because of < 0 vs <= 0
>>>> comparison of virDomainTPMModelTypeFromString() retval.
>>>>
>>>> Make the parser error out explicitly in this case. Users can
>>>> always chose to not specify the attribute in which case we pick a
>>>> sane default (in qemuDomainTPMDefPostParse()).
>>>>
>>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>>>> ---
>>>> src/conf/domain_conf.c | 2 +-
>>>> src/conf/domain_conf.h | 2 +-
>>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>>> index 4c7a5a044c..b7147945da 100644
>>>> --- a/src/conf/domain_conf.c
>>>> +++ b/src/conf/domain_conf.c
>>>> @@ -10360,7 +10360,7 @@ virDomainTPMDefParseXML(virDomainXMLOption
*xmlopt,
>>>>
>>>> model = virXMLPropString(node, "model");
>>>> if (model != NULL &&
>>>> - (def->model = virDomainTPMModelTypeFromString(model)) <
0) {
>>>> + (def->model = virDomainTPMModelTypeFromString(model)) <=
0) {
>>>> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>>> _("Unknown TPM frontend model
'%s'"), model);
>>>> goto error;
>>>
>>> 'virDomainTPMDefFormat' happily formats 'default' as
supported type:
>>>
>>> virBufferAsprintf(&attrBuf, " model='%s'",
>>> virDomainTPMModelTypeToString(def->model));
>>>
>>> Is there any other code path which would forbid 'default'?
>>
>> Couple of them, actually. The first one is in
>> qemuValidateDomainDeviceDefTPM() where
>> virQEMUCapsFillDomainDeviceTPMCaps(qemuCaps, &tpmCaps); is called
>> followed by if (!VIR_DOMAIN_CAPS_ENUM_IS_SET(tpmCaps.model, tpm->model))
>> {}. And the second is qemuDomainTPMDefPostParse() which overwrites the
>> _DEFAULT to either _TIS or _SPAPR (alright, this is not a check that
>> forbids 'default' per se).
>
> Taken very strictly this would not apply for other drivers though, which
> in most cases don't even reject TPM devices, but that would be a very
> theoretical excercise.
True, no other driver rejects TPM, but at the same time no other driver
supports it. I believe our policy was to do nothing in such case. We
tell users to try out their XML first to see whether a device is
supported. TPM is not alone in this.
Sure, but in many cases where we reject the default at parsing we also
make sure to prevent programming errors from re-parsing the XML as that
can be sub-optimal for the health of a running VM.
In this particular instance you are changing existing code so obviously
it falls under more scrutiny.
>
> If you want to go ahead and make the parser more strict, which should be
> most likely okay in any reasonable case, you should in such case modify
> the formatter to skip the _DEFAULT values so they don't end up in the
> XML accidentally and then make it un-parsable.
>
Well, if _DEFAULT model (or _DEFAULT version from our discussion to 2/8)
was provided then already users would have to specifically ignore schema
validation AND our documentation. But yes, it is possible to define the
following XML:
<tpm model='default'>
<backend type='emulator' version='default'/>
</tpm>
for say the test driver, which lacks those default -> sensible value
post parse callbacks. In this case we would format XML and won't be able
to parse it back. Except for QEMU driver.
Apart from the gross user negligence above, we can have cases where a
device definition is mistakenly allocated and doesn't undergo defaults
filling for some reason, which would create a XML we refuse to parse.
But okay, consider the following squashed into their respective commits:
diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index 83db800f51..1647692ea2 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -24230,8 +24230,10 @@ virDomainTPMDefFormat(virBuffer *buf,
g_auto(virBuffer) backendAttrBuf = VIR_BUFFER_INITIALIZER;
g_auto(virBuffer) backendChildBuf = VIR_BUFFER_INIT_CHILD(&childBuf);
- virBufferAsprintf(&attrBuf, " model='%s'",
- virDomainTPMModelTypeToString(def->model));
+ if (def->model) {
+ virBufferAsprintf(&attrBuf, " model='%s'",
+ virDomainTPMModelTypeToString(def->model));
+ }
virBufferAsprintf(&backendAttrBuf, " type='%s'",
virDomainTPMBackendTypeToString(def->type));
@@ -24242,8 +24244,10 @@ virDomainTPMDefFormat(virBuffer *buf,
def->data.passthrough.source->data.file.path);
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
- virBufferAsprintf(&backendAttrBuf, " version='%s'",
- virDomainTPMVersionTypeToString(def->version));
+ if (def->version) {
+ virBufferAsprintf(&backendAttrBuf, " version='%s'",
+ virDomainTPMVersionTypeToString(def->version));
+ }
if (def->data.emulator.persistent_state)
virBufferAddLit(&backendAttrBuf, "
persistent_state='yes'");
if (def->data.emulator.hassecretuuid) {
Preferrably use an explicit comparison against the appropriate _DEFAULT.
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
4:30 a.m.
New subject: [PATCH 2/8] conf: Report error when default TPM version is provided
When "default" version of TPM was provided, our parses accepts it
happily even though the value is forbidden by our RNG and not
documented as accepted value. This is because of < 0 vs <= 0
comparison of virDomainTPMModelTypeFromString() retval.
Make the parser error out explicitly in this case. Users can
always chose to not specify the attribute in which case we pick a
sane default (in qemuDomainDefTPMsPostParse()).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 2 +-
src/conf/domain_conf.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b7147945da..6c178783af 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10400,7 +10400,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
if (!version) {
def->version = VIR_DOMAIN_TPM_VERSION_DEFAULT;
} else {
- if ((def->version = virDomainTPMVersionTypeFromString(version)) < 0) {
+ if ((def->version = virDomainTPMVersionTypeFromString(version)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unsupported TPM version '%s'"),
version);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5a057c36b8..7139b91aca 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1417,7 +1417,7 @@ typedef enum {
} virDomainTPMBackendType;
typedef enum {
- VIR_DOMAIN_TPM_VERSION_DEFAULT,
+ VIR_DOMAIN_TPM_VERSION_DEFAULT = 0,
VIR_DOMAIN_TPM_VERSION_1_2,
VIR_DOMAIN_TPM_VERSION_2_0,
--
2.35.1
6:11 a.m.
New subject: [PATCH 2/8] conf: Report error when default TPM version is provided
On Mon, Jul 18, 2022 at 11:30:44 +0200, Michal Privoznik wrote:
When "default" version of TPM was provided, our parses
accepts it
happily even though the value is forbidden by our RNG and not
documented as accepted value. This is because of < 0 vs <= 0
comparison of virDomainTPMModelTypeFromString() retval.
Make the parser error out explicitly in this case. Users can
always chose to not specify the attribute in which case we pick a
sane default (in qemuDomainDefTPMsPostParse()).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 2 +-
src/conf/domain_conf.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
This has the same issue as I've reported in previous patch.
8:08 a.m.
New subject: [PATCH 2/8] conf: Report error when default TPM version is provided
On 8/1/22 13:11, Peter Krempa wrote:
On Mon, Jul 18, 2022 at 11:30:44 +0200, Michal Privoznik wrote:
> When "default" version of TPM was provided, our parses accepts it
> happily even though the value is forbidden by our RNG and not
> documented as accepted value. This is because of < 0 vs <= 0
> comparison of virDomainTPMModelTypeFromString() retval.
>
> Make the parser error out explicitly in this case. Users can
> always chose to not specify the attribute in which case we pick a
> sane default (in qemuDomainDefTPMsPostParse()).
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/conf/domain_conf.c | 2 +-
> src/conf/domain_conf.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
This has the same issue as I've reported in previous patch.
The 'default' gets overwritten in qemuDomainDefTPMsPostParse() to
something sensible. So we would never ever output 'default'.
Michal
9:31 a.m.
New subject: [PATCH 2/8] conf: Report error when default TPM version is provided
On Mon, Aug 01, 2022 at 15:08:27 +0200, Michal Prívozník wrote:
On 8/1/22 13:11, Peter Krempa wrote:
> On Mon, Jul 18, 2022 at 11:30:44 +0200, Michal Privoznik wrote:
>> When "default" version of TPM was provided, our parses accepts it
>> happily even though the value is forbidden by our RNG and not
>> documented as accepted value. This is because of < 0 vs <= 0
>> comparison of virDomainTPMModelTypeFromString() retval.
>>
>> Make the parser error out explicitly in this case. Users can
>> always chose to not specify the attribute in which case we pick a
>> sane default (in qemuDomainDefTPMsPostParse()).
>>
>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>> ---
>> src/conf/domain_conf.c | 2 +-
>> src/conf/domain_conf.h | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> This has the same issue as I've reported in previous patch.
>
The 'default' gets overwritten in qemuDomainDefTPMsPostParse() to
something sensible. So we would never ever output 'default'.
Thus this boils down to same condition I gave with previous patch. If
you want to make the parser more strict, make the formatter skip the
value which would be rejected by the parser.
4:30 a.m.
New subject: [PATCH 3/8] conf: Drop needless setting of VIR_DOMAIN_TPM_VERSION_DEFAULT
In previous commit the VIR_DOMAIN_TPM_VERSION_DEFAULT value was
made just an alias to value of 0. And since all newly allocated
memory is zeroed out (due to use of g_new0()), the def->version
inside of virDomainTPMDefParseXML() is also 0 and thus there is
no need to set it explicitly.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c178783af..2d8989e4ff 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10397,15 +10397,12 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
}
version = virXMLPropString(backends[0], "version");
- if (!version) {
- def->version = VIR_DOMAIN_TPM_VERSION_DEFAULT;
- } else {
- if ((def->version = virDomainTPMVersionTypeFromString(version)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unsupported TPM version '%s'"),
- version);
- goto error;
- }
+ if (version &&
+ (def->version = virDomainTPMVersionTypeFromString(version)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported TPM version '%s'"),
+ version);
+ goto error;
}
switch (def->type) {
--
2.35.1
6:13 a.m.
New subject: [PATCH 3/8] conf: Drop needless setting of VIR_DOMAIN_TPM_VERSION_DEFAULT
On Mon, Jul 18, 2022 at 11:30:45 +0200, Michal Privoznik wrote:
In previous commit the VIR_DOMAIN_TPM_VERSION_DEFAULT value was
made just an alias to value of 0.
This is true even now.
And since all newly allocated
memory is zeroed out (due to use of g_new0()), the def->version
inside of virDomainTPMDefParseXML() is also 0 and thus there is
no need to set it explicitly.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c178783af..2d8989e4ff 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10397,15 +10397,12 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
}
version = virXMLPropString(backends[0], "version");
- if (!version) {
- def->version = VIR_DOMAIN_TPM_VERSION_DEFAULT;
- } else {
- if ((def->version = virDomainTPMVersionTypeFromString(version)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unsupported TPM version '%s'"),
- version);
- goto error;
- }
+ if (version &&
+ (def->version = virDomainTPMVersionTypeFromString(version)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported TPM version '%s'"),
+ version);
You can use:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
With appropriate adaptation if previous patch ends up to be modified.
4:30 a.m.
New subject: [PATCH 4/8] conf: Move _virDomainTPMDef::version into _virDomainTPMDef::data::emulator
The _virDomainTPMDef structure has 'version' member, which is a
bit misplaced. It's only emulator type of TPM that can have a
version, even our documentation says so:
``version``
The ``version`` attribute indicates the version of the TPM. This attribute
only works with the ``emulator`` backend. The following versions are
supported:
Therefore, move the member into that part of union that's
covering emulated TPM devices.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 34 +++++++++++-----------
src/conf/domain_conf.h | 2 +-
src/qemu/qemu_domain.c | 7 +++--
src/qemu/qemu_tpm.c | 10 ++++---
src/qemu/qemu_validate.c | 53 ++++++++++++++++++-----------------
src/security/virt-aa-helper.c | 2 +-
6 files changed, 56 insertions(+), 52 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2d8989e4ff..28f0e75e60 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10396,15 +10396,6 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
goto error;
}
- version = virXMLPropString(backends[0], "version");
- if (version &&
- (def->version = virDomainTPMVersionTypeFromString(version)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unsupported TPM version '%s'"),
- version);
- goto error;
- }
-
switch (def->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
if (!(def->data.passthrough.source = virDomainChrSourceDefNew(xmlopt)))
@@ -10416,6 +10407,15 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
def->data.passthrough.source->data.file.path = g_steal_pointer(&path);
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+ version = virXMLPropString(backends[0], "version");
+ if (version &&
+ (def->data.emulator.version = virDomainTPMVersionTypeFromString(version))
<= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported TPM version '%s'"),
+ version);
+ goto error;
+ }
+
if (!(def->data.emulator.source = virDomainChrSourceDefNew(xmlopt)))
goto error;
secretuuid = virXPathString("string(./backend/encryption/@secret)",
ctxt);
@@ -10437,7 +10437,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
goto error;
}
}
- if (def->version == VIR_DOMAIN_TPM_VERSION_2_0) {
+ if (def->data.emulator.version == VIR_DOMAIN_TPM_VERSION_2_0) {
if ((nnodes = virXPathNodeSet("./backend/active_pcr_banks/*", ctxt,
&nodes)) < 0)
break;
for (i = 0; i < nnodes; i++) {
@@ -20658,14 +20658,14 @@ virDomainTPMDefCheckABIStability(virDomainTPMDef *src,
return false;
}
- if (src->version != dst->version) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Target TPM version doesn't match source"));
- return false;
- }
-
switch (src->type) {
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+ if (src->data.emulator.version != dst->data.emulator.version) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Target TPM version doesn't match source"));
+ return false;
+ }
+
if (src->data.emulator.activePcrBanks != dst->data.emulator.activePcrBanks)
{
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Target active PCR banks doesn't match
source"));
@@ -24219,7 +24219,7 @@ virDomainTPMDefFormat(virBuffer *buf,
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
virBufferAsprintf(&backendAttrBuf, " version='%s'",
- virDomainTPMVersionTypeToString(def->version));
+
virDomainTPMVersionTypeToString(def->data.emulator.version));
if (def->data.emulator.persistent_state)
virBufferAddLit(&backendAttrBuf, "
persistent_state='yes'");
if (def->data.emulator.hassecretuuid) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 7139b91aca..3362042db5 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1439,12 +1439,12 @@ struct _virDomainTPMDef {
int type; /* virDomainTPMBackendType */
virDomainDeviceInfo info;
int model; /* virDomainTPMModel */
- int version; /* virDomainTPMVersion */
union {
struct {
virDomainChrSourceDef *source;
} passthrough;
struct {
+ int version; /* virDomainTPMVersion */
virDomainChrSourceDef *source;
char *storagepath;
char *logfile;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 94b2e3118c..0343fd3597 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4594,13 +4594,14 @@ qemuDomainDefTPMsPostParse(virDomainDef *def)
virDomainTPMDef *tpm = def->tpms[i];
/* TPM 1.2 and 2 are not compatible, so we choose a specific version here */
- if (tpm->version == VIR_DOMAIN_TPM_VERSION_DEFAULT) {
+ if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
+ tpm->data.emulator.version == VIR_DOMAIN_TPM_VERSION_DEFAULT) {
if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR ||
tpm->model == VIR_DOMAIN_TPM_MODEL_CRB ||
qemuDomainIsARMVirt(def))
- tpm->version = VIR_DOMAIN_TPM_VERSION_2_0;
+ tpm->data.emulator.version = VIR_DOMAIN_TPM_VERSION_2_0;
else
- tpm->version = VIR_DOMAIN_TPM_VERSION_1_2;
+ tpm->data.emulator.version = VIR_DOMAIN_TPM_VERSION_1_2;
}
if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) {
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 03829775b8..f28dd2e1e9 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -575,7 +575,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
if (created &&
qemuTPMEmulatorRunSetup(tpm->data.emulator.storagepath, vmname, vmuuid,
privileged, swtpm_user, swtpm_group,
- tpm->data.emulator.logfile, tpm->version,
+ tpm->data.emulator.logfile,
+ tpm->data.emulator.version,
secretuuid, incomingMigration) < 0)
goto error;
@@ -583,7 +584,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
qemuTPMEmulatorReconfigure(tpm->data.emulator.storagepath,
swtpm_user, swtpm_group,
tpm->data.emulator.activePcrBanks,
- tpm->data.emulator.logfile, tpm->version,
+ tpm->data.emulator.logfile,
+ tpm->data.emulator.version,
secretuuid) < 0)
goto error;
@@ -611,7 +613,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
virCommandSetUID(cmd, swtpm_user);
virCommandSetGID(cmd, swtpm_group);
- switch (tpm->version) {
+ switch (tpm->data.emulator.version) {
case VIR_DOMAIN_TPM_VERSION_1_2:
break;
case VIR_DOMAIN_TPM_VERSION_2_0:
@@ -684,7 +686,7 @@ qemuTPMEmulatorInitPaths(virDomainTPMDef *tpm,
if (!tpm->data.emulator.storagepath &&
!(tpm->data.emulator.storagepath =
qemuTPMEmulatorStorageBuildPath(swtpmStorageDir, uuidstr,
- tpm->version)))
+ tpm->data.emulator.version)))
return -1;
if (!tpm->data.emulator.logfile) {
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 764d5b029e..ff164118b7 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4760,33 +4760,34 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
{
virDomainCapsDeviceTPM tpmCaps = { 0 };
- switch (tpm->version) {
- case VIR_DOMAIN_TPM_VERSION_1_2:
- /* TPM 1.2 + CRB do not work */
- if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
- tpm->model == VIR_DOMAIN_TPM_MODEL_CRB) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unsupported interface %s for TPM 1.2"),
- virDomainTPMModelTypeToString(tpm->model));
- return -1;
+ if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) {
+ switch (tpm->data.emulator.version) {
+ case VIR_DOMAIN_TPM_VERSION_1_2:
+ /* TPM 1.2 + CRB do not work */
+ if (tpm->model == VIR_DOMAIN_TPM_MODEL_CRB) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported interface %s for TPM 1.2"),
+ virDomainTPMModelTypeToString(tpm->model));
+ return -1;
+ }
+ /* TPM 1.2 + SPAPR do not work with any 'type' (backend) */
+ if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("TPM 1.2 is not supported with the SPAPR device
model"));
+ return -1;
+ }
+ /* TPM 1.2 + ARM does not work */
+ if (qemuDomainIsARMVirt(def)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("TPM 1.2 is not supported on ARM"));
+ return -1;
+ }
+ break;
+ case VIR_DOMAIN_TPM_VERSION_2_0:
+ case VIR_DOMAIN_TPM_VERSION_DEFAULT:
+ case VIR_DOMAIN_TPM_VERSION_LAST:
+ break;
}
- /* TPM 1.2 + SPAPR do not work with any 'type' (backend) */
- if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("TPM 1.2 is not supported with the SPAPR device
model"));
- return -1;
- }
- /* TPM 1.2 + ARM does not work */
- if (qemuDomainIsARMVirt(def)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("TPM 1.2 is not supported on ARM"));
- return -1;
- }
- break;
- case VIR_DOMAIN_TPM_VERSION_2_0:
- case VIR_DOMAIN_TPM_VERSION_DEFAULT:
- case VIR_DOMAIN_TPM_VERSION_LAST:
- break;
}
virQEMUCapsFillDomainDeviceTPMCaps(qemuCaps, &tpmCaps);
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 8629503e11..2d0bc99c73 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1212,7 +1212,7 @@ get_files(vahControl * ctl)
shortName = virDomainDefGetShortName(ctl->def);
- switch (ctl->def->tpms[i]->version) {
+ switch (ctl->def->tpms[i]->data.emulator.version) {
case VIR_DOMAIN_TPM_VERSION_1_2:
tpmpath = "tpm1.2";
break;
--
2.35.1
6:18 a.m.
New subject: [PATCH 4/8] conf: Move _virDomainTPMDef::version into _virDomainTPMDef::data::emulator
On Mon, Jul 18, 2022 at 11:30:46 +0200, Michal Privoznik wrote:
The _virDomainTPMDef structure has 'version' member, which is
a
bit misplaced. It's only emulator type of TPM that can have a
version, even our documentation says so:
``version``
The ``version`` attribute indicates the version of the TPM. This attribute
only works with the ``emulator`` backend. The following versions are
supported:
Therefore, move the member into that part of union that's
covering emulated TPM devices.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 34 +++++++++++-----------
src/conf/domain_conf.h | 2 +-
src/qemu/qemu_domain.c | 7 +++--
src/qemu/qemu_tpm.c | 10 ++++---
src/qemu/qemu_validate.c | 53 ++++++++++++++++++-----------------
src/security/virt-aa-helper.c | 2 +-
6 files changed, 56 insertions(+), 52 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2d8989e4ff..28f0e75e60 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10396,15 +10396,6 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
goto error;
}
- version = virXMLPropString(backends[0], "version");
- if (version &&
- (def->version = virDomainTPMVersionTypeFromString(version)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unsupported TPM version '%s'"),
- version);
- goto error;
- }
-
switch (def->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
if (!(def->data.passthrough.source = virDomainChrSourceDefNew(xmlopt)))
@@ -10416,6 +10407,15 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
def->data.passthrough.source->data.file.path =
g_steal_pointer(&path);
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+ version = virXMLPropString(backends[0], "version");
+ if (version &&
+ (def->data.emulator.version = virDomainTPMVersionTypeFromString(version))
<= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported TPM version '%s'"),
+ version);
+ goto error;
+ }
+
if (!(def->data.emulator.source = virDomainChrSourceDefNew(xmlopt)))
goto error;
secretuuid = virXPathString("string(./backend/encryption/@secret)",
ctxt);
Same conditions for using my R-b as per previous patch apply.
[...]
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 764d5b029e..ff164118b7 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4760,33 +4760,34 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
{
virDomainCapsDeviceTPM tpmCaps = { 0 };
- switch (tpm->version) {
- case VIR_DOMAIN_TPM_VERSION_1_2:
- /* TPM 1.2 + CRB do not work */
- if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
- tpm->model == VIR_DOMAIN_TPM_MODEL_CRB) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unsupported interface %s for TPM 1.2"),
- virDomainTPMModelTypeToString(tpm->model));
- return -1;
+ if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) {
+ switch (tpm->data.emulator.version) {
+ case VIR_DOMAIN_TPM_VERSION_1_2:
+ /* TPM 1.2 + CRB do not work */
+ if (tpm->model == VIR_DOMAIN_TPM_MODEL_CRB) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported interface %s for TPM 1.2"),
+ virDomainTPMModelTypeToString(tpm->model));
I know this is supposed to be a pure code movement but please add quotes
around '%s' in the error message.
+ return -1;
+ }
+ /* TPM 1.2 + SPAPR do not work with any 'type' (backend) */
+ if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("TPM 1.2 is not supported with the SPAPR device
model"));
+ return -1;
Under same conditions as previous patch:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
4:30 a.m.
New subject: [PATCH 5/8] conf: Use virXMLPropEnum more when parsing TPM
When parsing a TPM device plenty of virXMLPropString() +
enum2int() combos are used. These can be replaced with
virXMLPropEnum().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 39 ++++++++++++---------------------------
src/conf/domain_conf.h | 6 +++---
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_domain.c | 2 +-
4 files changed, 17 insertions(+), 32 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 28f0e75e60..6263d90fdb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10347,9 +10347,6 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
int nnodes;
size_t i;
g_autofree char *path = NULL;
- g_autofree char *model = NULL;
- g_autofree char *backend = NULL;
- g_autofree char *version = NULL;
g_autofree char *secretuuid = NULL;
g_autofree char *persistent_state = NULL;
g_autofree xmlNodePtr *backends = NULL;
@@ -10358,13 +10355,11 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
def = g_new0(virDomainTPMDef, 1);
- model = virXMLPropString(node, "model");
- if (model != NULL &&
- (def->model = virDomainTPMModelTypeFromString(model)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unknown TPM frontend model '%s'"), model);
+ if (virXMLPropEnum(node, "model",
+ virDomainTPMModelTypeFromString,
+ VIR_XML_PROP_NONZERO,
+ &def->model) < 0)
goto error;
- }
ctxt->node = node;
@@ -10383,18 +10378,11 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
goto error;
}
- if (!(backend = virXMLPropString(backends[0], "type"))) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing TPM device backend type"));
+ if (virXMLPropEnum(backends[0], "type",
+ virDomainTPMBackendTypeFromString,
+ VIR_XML_PROP_REQUIRED,
+ &def->type) < 0)
goto error;
- }
-
- if ((def->type = virDomainTPMBackendTypeFromString(backend)) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unknown TPM backend type '%s'"),
- backend);
- goto error;
- }
switch (def->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
@@ -10407,14 +10395,11 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
def->data.passthrough.source->data.file.path = g_steal_pointer(&path);
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
- version = virXMLPropString(backends[0], "version");
- if (version &&
- (def->data.emulator.version = virDomainTPMVersionTypeFromString(version))
<= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unsupported TPM version '%s'"),
- version);
+ if (virXMLPropEnum(backends[0], "version",
+ virDomainTPMVersionTypeFromString,
+ VIR_XML_PROP_NONZERO,
+ &def->data.emulator.version) < 0)
goto error;
- }
if (!(def->data.emulator.source = virDomainChrSourceDefNew(xmlopt)))
goto error;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3362042db5..bab667d026 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1436,15 +1436,15 @@ typedef enum {
#define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0"
struct _virDomainTPMDef {
- int type; /* virDomainTPMBackendType */
+ virDomainTPMModel model;
+ virDomainTPMBackendType type;
virDomainDeviceInfo info;
- int model; /* virDomainTPMModel */
union {
struct {
virDomainChrSourceDef *source;
} passthrough;
struct {
- int version; /* virDomainTPMVersion */
+ virDomainTPMVersion version;
virDomainChrSourceDef *source;
char *storagepath;
char *logfile;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 262fffe5fe..93ea8748c7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9840,7 +9840,7 @@ qemuBuildTPMCommandLine(virCommand *cmd,
g_autoptr(qemuFDPass) passtpm = NULL;
g_autoptr(qemuFDPass) passcancel = NULL;
- switch ((virDomainTPMBackendType) tpm->type) {
+ switch (tpm->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: {
VIR_AUTOCLOSE fdtpm = -1;
VIR_AUTOCLOSE fdcancel = -1;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0343fd3597..09fc88e7fa 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11752,7 +11752,7 @@ qemuDomainDeviceBackendChardevForeachOne(virDomainDeviceDef *dev,
return cb(dev, dev->data.rng->source.chardev, opaque);
case VIR_DOMAIN_DEVICE_TPM:
- switch ((virDomainTPMBackendType) dev->data.tpm->type) {
+ switch (dev->data.tpm->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
return cb(dev, dev->data.tpm->data.passthrough.source, opaque);
--
2.35.1
6:20 a.m.
New subject: [PATCH 5/8] conf: Use virXMLPropEnum more when parsing TPM
On Mon, Jul 18, 2022 at 11:30:47 +0200, Michal Privoznik wrote:
When parsing a TPM device plenty of virXMLPropString() +
enum2int() combos are used. These can be replaced with
virXMLPropEnum().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 39 ++++++++++++---------------------------
src/conf/domain_conf.h | 6 +++---
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_domain.c | 2 +-
4 files changed, 17 insertions(+), 32 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 28f0e75e60..6263d90fdb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
[...]
@@ -10358,13 +10355,11 @@ virDomainTPMDefParseXML(virDomainXMLOption
*xmlopt,
def = g_new0(virDomainTPMDef, 1);
- model = virXMLPropString(node, "model");
- if (model != NULL &&
- (def->model = virDomainTPMModelTypeFromString(model)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unknown TPM frontend model '%s'"), model);
+ if (virXMLPropEnum(node, "model",
+ virDomainTPMModelTypeFromString,
+ VIR_XML_PROP_NONZERO,
^^^
+ &def->model) < 0)
goto error;
- }
ctxt->node = node;
[...]
@@ -10407,14 +10395,11 @@ virDomainTPMDefParseXML(virDomainXMLOption
*xmlopt,
def->data.passthrough.source->data.file.path =
g_steal_pointer(&path);
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
- version = virXMLPropString(backends[0], "version");
- if (version &&
- (def->data.emulator.version = virDomainTPMVersionTypeFromString(version))
<= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unsupported TPM version '%s'"),
- version);
+ if (virXMLPropEnum(backends[0], "version",
+ virDomainTPMVersionTypeFromString,
+ VIR_XML_PROP_NONZERO,
^^^
+ &def->data.emulator.version) <
0)
goto error;
- }
if (!(def->data.emulator.source = virDomainChrSourceDefNew(xmlopt)))
goto error;
Lines marked as '^^^' will likely need to be adapted per reasons in
previous replies. Other than that:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
4:30 a.m.
New subject: [PATCH 6/8] qemu_domain: Move TPM post parse code into qemuDomainTPMDefPostParse()
In the qemuDomainDefPostParse() we aim to fill in top level
values, which require overall view of domain, or those parts of
configuration that are not a device in domain XML (e.g. vCPUs).
However, inside of qemuDomainDefTPMsPostParse(), which is called
from aforementioned function, we do two tings:
1) fill in missing info (TPM version), and
2) validate TPM definition.
Now, if 1) is moved into qemuDomainTPMDefPostParse() (the device
post parse callback), then 2) can be moved into validation step.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_domain.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 09fc88e7fa..bcee4d2602 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4593,17 +4593,6 @@ qemuDomainDefTPMsPostParse(virDomainDef *def)
for (i = 0; i < def->ntpms; i++) {
virDomainTPMDef *tpm = def->tpms[i];
- /* TPM 1.2 and 2 are not compatible, so we choose a specific version here */
- if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
- tpm->data.emulator.version == VIR_DOMAIN_TPM_VERSION_DEFAULT) {
- if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR ||
- tpm->model == VIR_DOMAIN_TPM_MODEL_CRB ||
- qemuDomainIsARMVirt(def))
- tpm->data.emulator.version = VIR_DOMAIN_TPM_VERSION_2_0;
- else
- tpm->data.emulator.version = VIR_DOMAIN_TPM_VERSION_1_2;
- }
-
if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) {
if (proxyTPM) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -5806,15 +5795,26 @@ qemuDomainHostdevDefPostParse(virDomainHostdevDef *hostdev,
static int
qemuDomainTPMDefPostParse(virDomainTPMDef *tpm,
- virArch arch)
+ const virDomainDef *def)
{
if (tpm->model == VIR_DOMAIN_TPM_MODEL_DEFAULT) {
- if (ARCH_IS_PPC64(arch))
+ if (ARCH_IS_PPC64(def->os.arch))
tpm->model = VIR_DOMAIN_TPM_MODEL_SPAPR;
else
tpm->model = VIR_DOMAIN_TPM_MODEL_TIS;
}
+ /* TPM 1.2 and 2 are not compatible, so we choose a specific version here */
+ if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
+ tpm->data.emulator.version == VIR_DOMAIN_TPM_VERSION_DEFAULT) {
+ if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR ||
+ tpm->model == VIR_DOMAIN_TPM_MODEL_CRB ||
+ qemuDomainIsARMVirt(def))
+ tpm->data.emulator.version = VIR_DOMAIN_TPM_VERSION_2_0;
+ else
+ tpm->data.emulator.version = VIR_DOMAIN_TPM_VERSION_1_2;
+ }
+
return 0;
}
@@ -5941,7 +5941,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDef *dev,
break;
case VIR_DOMAIN_DEVICE_TPM:
- ret = qemuDomainTPMDefPostParse(dev->data.tpm, def->os.arch);
+ ret = qemuDomainTPMDefPostParse(dev->data.tpm, def);
break;
case VIR_DOMAIN_DEVICE_MEMORY:
--
2.35.1
6:27 a.m.
New subject: [PATCH 6/8] qemu_domain: Move TPM post parse code into qemuDomainTPMDefPostParse()
On Mon, Jul 18, 2022 at 11:30:48 +0200, Michal Privoznik wrote:
In the qemuDomainDefPostParse() we aim to fill in top level
values, which require overall view of domain, or those parts of
configuration that are not a device in domain XML (e.g. vCPUs).
However, inside of qemuDomainDefTPMsPostParse(), which is called
from aforementioned function, we do two tings:
1) fill in missing info (TPM version), and
2) validate TPM definition.
Now, if 1) is moved into qemuDomainTPMDefPostParse() (the device
post parse callback), then 2) can be moved into validation step.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_domain.c | 28 ++++++++++++++--------------
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
4:30 a.m.
New subject: [PATCH 7/8] qemu: Move TPMs validation out of PostParse
After previous cleanup, the qemuDomainDefTPMsPostParse() function
does nothing more than validates TPM devices. Therefore, it
should live in qemu_validate.c instead of qemu_domain.c. Move it
there and rename to reflect the fact that the function is doing
validation instead of PostParsing.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_domain.c | 34 ----------------------------------
src/qemu/qemu_validate.c | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bcee4d2602..e3d1bb548f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4583,37 +4583,6 @@ qemuDomainDefNumaCPUsPostParse(virDomainDef *def,
}
-static int
-qemuDomainDefTPMsPostParse(virDomainDef *def)
-{
- virDomainTPMDef *proxyTPM = NULL;
- virDomainTPMDef *regularTPM = NULL;
- size_t i;
-
- for (i = 0; i < def->ntpms; i++) {
- virDomainTPMDef *tpm = def->tpms[i];
-
- if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) {
- if (proxyTPM) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("only a single TPM Proxy device is
supported"));
- return -1;
- } else {
- proxyTPM = tpm;
- }
- } else if (regularTPM) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("only a single TPM non-proxy device is
supported"));
- return -1;
- } else {
- regularTPM = tpm;
- }
- }
-
- return 0;
-}
-
-
static int
qemuDomainDefPostParseBasic(virDomainDef *def,
void *opaque G_GNUC_UNUSED)
@@ -4709,9 +4678,6 @@ qemuDomainDefPostParse(virDomainDef *def,
if (qemuDomainDefNumaCPUsPostParse(def, qemuCaps) < 0)
return -1;
- if (qemuDomainDefTPMsPostParse(def) < 0)
- return -1;
-
return 0;
}
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index ff164118b7..ce8f92f301 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1113,6 +1113,37 @@ qemuValidateDomainDefPanic(const virDomainDef *def,
}
+static int
+qemuValidateDomainDefTPMs(const virDomainDef *def)
+{
+ const virDomainTPMDef *proxyTPM = NULL;
+ const virDomainTPMDef *regularTPM = NULL;
+ size_t i;
+
+ for (i = 0; i < def->ntpms; i++) {
+ virDomainTPMDef *tpm = def->tpms[i];
+
+ if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) {
+ if (proxyTPM) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("only a single TPM Proxy device is
supported"));
+ return -1;
+ } else {
+ proxyTPM = tpm;
+ }
+ } else if (regularTPM) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("only a single TPM non-proxy device is
supported"));
+ return -1;
+ } else {
+ regularTPM = tpm;
+ }
+ }
+
+ return 0;
+}
+
+
int
qemuValidateLifecycleAction(virDomainLifecycleAction onPoweroff,
virDomainLifecycleAction onReboot,
@@ -1310,6 +1341,9 @@ qemuValidateDomainDef(const virDomainDef *def,
if (qemuValidateDomainDefPanic(def, qemuCaps) < 0)
return -1;
+ if (qemuValidateDomainDefTPMs(def) < 0)
+ return -1;
+
if (def->sec) {
switch ((virDomainLaunchSecurity) def->sec->sectype) {
case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
--
2.35.1
6:28 a.m.
New subject: [PATCH 7/8] qemu: Move TPMs validation out of PostParse
On Mon, Jul 18, 2022 at 11:30:49 +0200, Michal Privoznik wrote:
After previous cleanup, the qemuDomainDefTPMsPostParse() function
does nothing more than validates TPM devices. Therefore, it
should live in qemu_validate.c instead of qemu_domain.c. Move it
there and rename to reflect the fact that the function is doing
validation instead of PostParsing.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_domain.c | 34 ----------------------------------
src/qemu/qemu_validate.c | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bcee4d2602..e3d1bb548f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
[...]
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index ff164118b7..ce8f92f301 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1113,6 +1113,37 @@ qemuValidateDomainDefPanic(const virDomainDef *def,
}
+static int
+qemuValidateDomainDefTPMs(const virDomainDef *def)
+{
+ const virDomainTPMDef *proxyTPM = NULL;
+ const virDomainTPMDef *regularTPM = NULL;
+ size_t i;
Please refactor the function to un-do unnecessarily complex logic:
+
+ for (i = 0; i < def->ntpms; i++) {
+ virDomainTPMDef *tpm = def->tpms[i];
+
+ if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) {
+ if (proxyTPM) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("only a single TPM Proxy device is
supported"));
+ return -1;
+ } else {
+ proxyTPM = tpm;
+ }
This else section is not needed, and proxyTPM can be assigned
unconditionally.
+ } else if (regularTPM) {
Make this into a plain 'else' and move this condition inside ...
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s",
+ _("only a single TPM non-proxy device is
supported"));
+ return -1;
+ } else {
+ regularTPM = tpm;
... and remove this else branch.
+ }
+ }
+
+ return 0;
+}
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
4:30 a.m.
New subject: [PATCH 8/8] conf: Don't lose <active_pcr_banks/> when no TPM version is provided
When no TPM version is provided in the input XML we may default
to version 2.0 (see qemuDomainTPMDefPostParse()). However,
<active_pcr_banks/> are parsed iff a version 2.0 was specified.
This means that this piece of information might be lost.
It's better to parse everything we've been given and then
validate that the configuration is valid.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2084046
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 21 ++++++++++-----------
src/conf/domain_validate.c | 28 +++++++++++++++++++++++++++-
2 files changed, 37 insertions(+), 12 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6263d90fdb..610fa5262b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10422,18 +10422,17 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
goto error;
}
}
- if (def->data.emulator.version == VIR_DOMAIN_TPM_VERSION_2_0) {
- if ((nnodes = virXPathNodeSet("./backend/active_pcr_banks/*", ctxt,
&nodes)) < 0)
- break;
- for (i = 0; i < nnodes; i++) {
- if ((bank = virDomainTPMPcrBankTypeFromString((const char
*)nodes[i]->name)) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unsupported PCR banks '%s'"),
- nodes[i]->name);
- goto error;
- }
- def->data.emulator.activePcrBanks |= (1 << bank);
+
+ if ((nnodes = virXPathNodeSet("./backend/active_pcr_banks/*", ctxt,
&nodes)) < 0)
+ break;
+ for (i = 0; i < nnodes; i++) {
+ if ((bank = virDomainTPMPcrBankTypeFromString((const char
*)nodes[i]->name)) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported PCR banks '%s'"),
+ nodes[i]->name);
+ goto error;
}
+ def->data.emulator.activePcrBanks |= (1 << bank);
}
break;
case VIR_DOMAIN_TPM_TYPE_LAST:
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 814922cd46..8d4a69f127 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -2611,6 +2611,30 @@ virDomainIOMMUDefValidate(const virDomainIOMMUDef *iommu)
return 0;
}
+
+static int
+virDomainTPMDevValidate(const virDomainTPMDef *tpm)
+{
+ switch (tpm->type) {
+ case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+ if (tpm->data.emulator.activePcrBanks &&
+ tpm->data.emulator.version != VIR_DOMAIN_TPM_VERSION_2_0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("<active_pcr_banks/> requires TPM version
'%s'"),
+ virDomainTPMVersionTypeToString(VIR_DOMAIN_TPM_VERSION_2_0));
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+ case VIR_DOMAIN_TPM_TYPE_LAST:
+ break;
+ }
+
+ return 0;
+}
+
+
static int
virDomainDeviceInfoValidate(const virDomainDeviceDef *dev)
{
@@ -2715,12 +2739,14 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
case VIR_DOMAIN_DEVICE_IOMMU:
return virDomainIOMMUDefValidate(dev->data.iommu);
+ case VIR_DOMAIN_DEVICE_TPM:
+ return virDomainTPMDevValidate(dev->data.tpm);
+
case VIR_DOMAIN_DEVICE_LEASE:
case VIR_DOMAIN_DEVICE_WATCHDOG:
case VIR_DOMAIN_DEVICE_HUB:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
- case VIR_DOMAIN_DEVICE_TPM:
case VIR_DOMAIN_DEVICE_PANIC:
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_LAST:
--
2.35.1
6:39 a.m.
New subject: [PATCH 8/8] conf: Don't lose <active_pcr_banks/> when no TPM version is provided
On Mon, Jul 18, 2022 at 11:30:50 +0200, Michal Privoznik wrote:
When no TPM version is provided in the input XML we may default
to version 2.0 (see qemuDomainTPMDefPostParse()). However,
<active_pcr_banks/> are parsed iff a version 2.0 was specified.
This means that this piece of information might be lost.
It's better to parse everything we've been given and then
validate that the configuration is valid.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2084046
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 21 ++++++++++-----------
src/conf/domain_validate.c | 28 +++++++++++++++++++++++++++-
2 files changed, 37 insertions(+), 12 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6263d90fdb..610fa5262b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10422,18 +10422,17 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
goto error;
}
}
- if (def->data.emulator.version == VIR_DOMAIN_TPM_VERSION_2_0) {
- if ((nnodes = virXPathNodeSet("./backend/active_pcr_banks/*",
ctxt, &nodes)) < 0)
- break;
- for (i = 0; i < nnodes; i++) {
- if ((bank = virDomainTPMPcrBankTypeFromString((const char
*)nodes[i]->name)) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unsupported PCR banks '%s'"),
- nodes[i]->name);
- goto error;
- }
- def->data.emulator.activePcrBanks |= (1 << bank);
+
+ if ((nnodes = virXPathNodeSet("./backend/active_pcr_banks/*", ctxt,
&nodes)) < 0)
+ break;
+ for (i = 0; i < nnodes; i++) {
+ if ((bank = virDomainTPMPcrBankTypeFromString((const char
*)nodes[i]->name)) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported PCR banks '%s'"),
+ nodes[i]->name);
+ goto error;
}
+ def->data.emulator.activePcrBanks |= (1 << bank);
Ewww. This is clearly a job for virBitmap. I'll post a patch to refactor
it on top of this patch, so don't worry about it or conflicts.
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
3:35 a.m.
On 7/18/22 11:30, Michal Privoznik wrote:
*** BLURB HERE ***
Michal Prívozník (8):
conf: Report an error when default TPM model is provided
conf: Report error when default TPM version is provided
conf: Drop needless setting of VIR_DOMAIN_TPM_VERSION_DEFAULT
conf: Move _virDomainTPMDef::version into
_virDomainTPMDef::data::emulator
conf: Use virXMLPropEnum more when parsing TPM
qemu_domain: Move TPM post parse code into qemuDomainTPMDefPostParse()
qemu: Move TPMs validation out of PostParse
conf: Don't lose <active_pcr_banks/> when no TPM version is provided
src/conf/domain_conf.c | 81 +++++++++++++-------------------
src/conf/domain_conf.h | 10 ++--
src/conf/domain_validate.c | 28 ++++++++++-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_domain.c | 63 ++++++-------------------
src/qemu/qemu_tpm.c | 10 ++--
src/qemu/qemu_validate.c | 87 ++++++++++++++++++++++++-----------
src/security/virt-aa-helper.c | 2 +-
8 files changed, 147 insertions(+), 136 deletions(-)
Polite ping.
Michal
843
days inactive
857
days old
23 comments
3 participants
participants (3)
-
Michal Privoznik -
Michal Prívozník
-
Peter Krempa