Skip to content

Commit d64072c

Browse files
Markus Armbrusterstefanberger
authored andcommitted
Revert "tpm: Clean up error reporting in tpm_init_tpmdev()"
This reverts commit d10e05f. We report some -tpmdev failures, but then continue as if all was fine. Reproducer: $ qemu-system-x86_64 -nodefaults -S -display none -monitor stdio -chardev null,id=tpm0 -tpmdev emulator,id=tpm0,chardev=chrtpm -device tpm-tis,tpmdev=tpm0 qemu-system-x86_64: -tpmdev emulator,id=tpm0,chardev=chrtpm: tpm-emulator: tpm chardev 'chrtpm' not found. qemu-system-x86_64: -tpmdev emulator,id=tpm0,chardev=chrtpm: tpm-emulator: Could not cleanly shutdown the TPM: No such file or directory QEMU 5.0.90 monitor - type 'help' for more information (qemu) qemu-system-x86_64: -device tpm-tis,tpmdev=tpm0: Property 'tpm-tis.tpmdev' can't find value 'tpm0' $ echo $? 1 This is a regression caused by commit d10e05f "tpm: Clean up error reporting in tpm_init_tpmdev()". It's incomplete: be->create(opts) continues to use error_report(), and we don't set an error when it fails. I figure converting the create() methods to Error would make some sense, but I'm not sure it's worth the effort right now. Revert the broken commit instead, and add a comment to tpm_init_tpmdev(). Straightforward conflict in tpm.c resolved. Signed-off-by: Markus Armbruster <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Reviewed-by: Stefan Berger <[email protected]> Signed-off-by: Stefan Berger <[email protected]>
1 parent 7adfbea commit d64072c

File tree

4 files changed

+27
-12
lines changed

4 files changed

+27
-12
lines changed

include/sysemu/tpm.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
#include "qom/object.h"
1717

1818
int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
19-
void tpm_init(void);
19+
int tpm_init(void);
2020
void tpm_cleanup(void);
2121

2222
typedef enum TPMVersion {

softmmu/vl.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4258,7 +4258,9 @@ void qemu_init(int argc, char **argv, char **envp)
42584258
user_creatable_add_opts_foreach,
42594259
object_create_delayed, &error_fatal);
42604260

4261-
tpm_init();
4261+
if (tpm_init() < 0) {
4262+
exit(1);
4263+
}
42624264

42634265
blk_mig_init();
42644266
ram_mig_init();

stubs/tpm.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@
1010
#include "sysemu/tpm.h"
1111
#include "hw/acpi/tpm.h"
1212

13-
void tpm_init(void)
13+
int tpm_init(void)
1414
{
15+
return 0;
1516
}
1617

1718
void tpm_cleanup(void)

tpm.c

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -81,41 +81,49 @@ TPMBackend *qemu_find_tpm_be(const char *id)
8181

8282
static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp)
8383
{
84+
/*
85+
* Use of error_report() in a function with an Error ** parameter
86+
* is suspicious. It is okay here. The parameter only exists to
87+
* make the function usable with qemu_opts_foreach(). It is not
88+
* actually used.
89+
*/
8490
const char *value;
8591
const char *id;
8692
const TPMBackendClass *be;
8793
TPMBackend *drv;
94+
Error *local_err = NULL;
8895
int i;
8996

9097
if (!QLIST_EMPTY(&tpm_backends)) {
91-
error_setg(errp, "Only one TPM is allowed.");
98+
error_report("Only one TPM is allowed.");
9299
return 1;
93100
}
94101

95102
id = qemu_opts_id(opts);
96103
if (id == NULL) {
97-
error_setg(errp, QERR_MISSING_PARAMETER, "id");
104+
error_report(QERR_MISSING_PARAMETER, "id");
98105
return 1;
99106
}
100107

101108
value = qemu_opt_get(opts, "type");
102109
if (!value) {
103-
error_setg(errp, QERR_MISSING_PARAMETER, "type");
110+
error_report(QERR_MISSING_PARAMETER, "type");
104111
tpm_display_backend_drivers();
105112
return 1;
106113
}
107114

108115
i = qapi_enum_parse(&TpmType_lookup, value, -1, NULL);
109116
be = i >= 0 ? tpm_be_find_by_type(i) : NULL;
110117
if (be == NULL) {
111-
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
112-
"a TPM backend type");
118+
error_report(QERR_INVALID_PARAMETER_VALUE,
119+
"type", "a TPM backend type");
113120
tpm_display_backend_drivers();
114121
return 1;
115122
}
116123

117124
/* validate backend specific opts */
118-
if (!qemu_opts_validate(opts, be->opts, errp)) {
125+
if (!qemu_opts_validate(opts, be->opts, &local_err)) {
126+
error_report_err(local_err);
119127
return 1;
120128
}
121129

@@ -148,10 +156,14 @@ void tpm_cleanup(void)
148156
* Initialize the TPM. Process the tpmdev command line options describing the
149157
* TPM backend.
150158
*/
151-
void tpm_init(void)
159+
int tpm_init(void)
152160
{
153-
qemu_opts_foreach(qemu_find_opts("tpmdev"),
154-
tpm_init_tpmdev, NULL, &error_fatal);
161+
if (qemu_opts_foreach(qemu_find_opts("tpmdev"),
162+
tpm_init_tpmdev, NULL, NULL)) {
163+
return -1;
164+
}
165+
166+
return 0;
155167
}
156168

157169
/*

0 commit comments

Comments
 (0)