Revert "tpm: Clean up error reporting in tpm_init_tpmdev()"

This reverts commit d10e05f15d.

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 d10e05f15d "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 <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
This commit is contained in:
Markus Armbruster 2020-07-23 13:58:44 +02:00 committed by Stefan Berger
parent 7adfbea8fd
commit d64072c0ac
4 changed files with 27 additions and 12 deletions

View File

@ -16,7 +16,7 @@
#include "qom/object.h"
int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
void tpm_init(void);
int tpm_init(void);
void tpm_cleanup(void);
typedef enum TPMVersion {

View File

@ -4258,7 +4258,9 @@ void qemu_init(int argc, char **argv, char **envp)
user_creatable_add_opts_foreach,
object_create_delayed, &error_fatal);
tpm_init();
if (tpm_init() < 0) {
exit(1);
}
blk_mig_init();
ram_mig_init();

View File

@ -10,8 +10,9 @@
#include "sysemu/tpm.h"
#include "hw/acpi/tpm.h"
void tpm_init(void)
int tpm_init(void)
{
return 0;
}
void tpm_cleanup(void)

30
tpm.c
View File

@ -81,26 +81,33 @@ TPMBackend *qemu_find_tpm_be(const char *id)
static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp)
{
/*
* Use of error_report() in a function with an Error ** parameter
* is suspicious. It is okay here. The parameter only exists to
* make the function usable with qemu_opts_foreach(). It is not
* actually used.
*/
const char *value;
const char *id;
const TPMBackendClass *be;
TPMBackend *drv;
Error *local_err = NULL;
int i;
if (!QLIST_EMPTY(&tpm_backends)) {
error_setg(errp, "Only one TPM is allowed.");
error_report("Only one TPM is allowed.");
return 1;
}
id = qemu_opts_id(opts);
if (id == NULL) {
error_setg(errp, QERR_MISSING_PARAMETER, "id");
error_report(QERR_MISSING_PARAMETER, "id");
return 1;
}
value = qemu_opt_get(opts, "type");
if (!value) {
error_setg(errp, QERR_MISSING_PARAMETER, "type");
error_report(QERR_MISSING_PARAMETER, "type");
tpm_display_backend_drivers();
return 1;
}
@ -108,14 +115,15 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp)
i = qapi_enum_parse(&TpmType_lookup, value, -1, NULL);
be = i >= 0 ? tpm_be_find_by_type(i) : NULL;
if (be == NULL) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
"a TPM backend type");
error_report(QERR_INVALID_PARAMETER_VALUE,
"type", "a TPM backend type");
tpm_display_backend_drivers();
return 1;
}
/* validate backend specific opts */
if (!qemu_opts_validate(opts, be->opts, errp)) {
if (!qemu_opts_validate(opts, be->opts, &local_err)) {
error_report_err(local_err);
return 1;
}
@ -148,10 +156,14 @@ void tpm_cleanup(void)
* Initialize the TPM. Process the tpmdev command line options describing the
* TPM backend.
*/
void tpm_init(void)
int tpm_init(void)
{
qemu_opts_foreach(qemu_find_opts("tpmdev"),
tpm_init_tpmdev, NULL, &error_fatal);
if (qemu_opts_foreach(qemu_find_opts("tpmdev"),
tpm_init_tpmdev, NULL, NULL)) {
return -1;
}
return 0;
}
/*