* [PATCH] tpm_emulator: Report an error if chardev is missing
@ 2020-07-24 13:30 Stefan Berger
2020-07-24 13:58 ` Marc-André Lureau
2020-07-24 14:00 ` Markus Armbruster
0 siblings, 2 replies; 4+ messages in thread
From: Stefan Berger @ 2020-07-24 13:30 UTC (permalink / raw)
To: qemu-devel, philmd; +Cc: marcandre.lureau, Stefan Berger, armbru, Stefan Berger
This patch fixes the odd error reporting when trying to send a file
descriptor to the TPM emulator if one has not passed a valid chardev.
$ x86_64-softmmu/qemu-system-x86_64 -tpmdev emulator,id=tpm0
qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: Failed to send CMD_SET_DATAFD: Success
qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: Could not cleanly shutdown the TPM: Success
This is the new error report:
$ x86_64-softmmu/qemu-system-x86_64 -tpmdev emulator,id=tpm0
qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: missing chardev
This change does not hide the display of supported TPM types if a non-existent type is passed:
$ x86_64-softmmu/qemu-system-x86_64 -tpmdev nonexistent,id=tpm0
qemu-system-x86_64: -tpmdev nonexistent,id=tpm0: Parameter 'type' expects a TPM backend type
Supported TPM types (choose only one):
passthrough Passthrough TPM backend driver
emulator TPM emulator backend driver
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
backends/tpm/tpm_emulator.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index 9605339f93..55cbc9c60e 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -568,6 +568,9 @@ static int tpm_emulator_handle_device_opts(TPMEmulator *tpm_emu, QemuOpts *opts)
}
tpm_emu->options->chardev = g_strdup(value);
+ } else {
+ error_report("tpm-emulator: missing chardev");
+ goto err;
}
if (tpm_emulator_prepare_data_fd(tpm_emu) < 0) {
@@ -925,6 +928,11 @@ static void tpm_emulator_shutdown(TPMEmulator *tpm_emu)
{
ptm_res res;
+ if (!tpm_emu->options->chardev) {
+ /* was never properly initialized */
+ return;
+ }
+
if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SHUTDOWN, &res, 0, sizeof(res)) < 0) {
error_report("tpm-emulator: Could not cleanly shutdown the TPM: %s",
strerror(errno));
--
2.24.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] tpm_emulator: Report an error if chardev is missing
2020-07-24 13:30 [PATCH] tpm_emulator: Report an error if chardev is missing Stefan Berger
@ 2020-07-24 13:58 ` Marc-André Lureau
2020-07-24 14:00 ` Markus Armbruster
1 sibling, 0 replies; 4+ messages in thread
From: Marc-André Lureau @ 2020-07-24 13:58 UTC (permalink / raw)
To: Stefan Berger
Cc: Markus Armbruster, Philippe Mathieu-Daudé, QEMU, Stefan Berger
[-- Attachment #1: Type: text/plain, Size: 2364 bytes --]
On Fri, Jul 24, 2020 at 5:30 PM Stefan Berger <stefanb@linux.vnet.ibm.com>
wrote:
> This patch fixes the odd error reporting when trying to send a file
> descriptor to the TPM emulator if one has not passed a valid chardev.
>
> $ x86_64-softmmu/qemu-system-x86_64 -tpmdev emulator,id=tpm0
> qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: Failed to send
> CMD_SET_DATAFD: Success
> qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: Could not
> cleanly shutdown the TPM: Success
>
> This is the new error report:
>
> $ x86_64-softmmu/qemu-system-x86_64 -tpmdev emulator,id=tpm0
> qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: missing chardev
>
> This change does not hide the display of supported TPM types if a
> non-existent type is passed:
>
> $ x86_64-softmmu/qemu-system-x86_64 -tpmdev nonexistent,id=tpm0
> qemu-system-x86_64: -tpmdev nonexistent,id=tpm0: Parameter 'type' expects
> a TPM backend type
> Supported TPM types (choose only one):
> passthrough Passthrough TPM backend driver
> emulator TPM emulator backend driver
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
> backends/tpm/tpm_emulator.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index 9605339f93..55cbc9c60e 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -568,6 +568,9 @@ static int tpm_emulator_handle_device_opts(TPMEmulator
> *tpm_emu, QemuOpts *opts)
> }
>
> tpm_emu->options->chardev = g_strdup(value);
> + } else {
> + error_report("tpm-emulator: missing chardev");
> + goto err;
> }
>
> if (tpm_emulator_prepare_data_fd(tpm_emu) < 0) {
> @@ -925,6 +928,11 @@ static void tpm_emulator_shutdown(TPMEmulator
> *tpm_emu)
> {
> ptm_res res;
>
> + if (!tpm_emu->options->chardev) {
> + /* was never properly initialized */
> + return;
> + }
> +
> if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SHUTDOWN, &res, 0, sizeof(res))
> < 0) {
> error_report("tpm-emulator: Could not cleanly shutdown the TPM:
> %s",
> strerror(errno));
> --
> 2.24.1
>
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 3246 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tpm_emulator: Report an error if chardev is missing
2020-07-24 13:30 [PATCH] tpm_emulator: Report an error if chardev is missing Stefan Berger
2020-07-24 13:58 ` Marc-André Lureau
@ 2020-07-24 14:00 ` Markus Armbruster
2020-07-24 14:11 ` Stefan Berger
1 sibling, 1 reply; 4+ messages in thread
From: Markus Armbruster @ 2020-07-24 14:00 UTC (permalink / raw)
To: Stefan Berger; +Cc: marcandre.lureau, philmd, qemu-devel, Stefan Berger
Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
> This patch fixes the odd error reporting when trying to send a file
> descriptor to the TPM emulator if one has not passed a valid chardev.
>
> $ x86_64-softmmu/qemu-system-x86_64 -tpmdev emulator,id=tpm0
> qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: Failed to send CMD_SET_DATAFD: Success
> qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: Could not cleanly shutdown the TPM: Success
>
> This is the new error report:
>
> $ x86_64-softmmu/qemu-system-x86_64 -tpmdev emulator,id=tpm0
> qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: missing chardev
~~~~~~~~~~~~~~
The "tpm-emulator: " part looks superfluous.
Hmm, many error messages in this file have it. Feel free to keep it
now for consistency, and clean it up later.
We usually report missing option parameters like "parameter 'chardev' is
missing". Please consider sticking to that.
>
> This change does not hide the display of supported TPM types if a non-existent type is passed:
>
> $ x86_64-softmmu/qemu-system-x86_64 -tpmdev nonexistent,id=tpm0
> qemu-system-x86_64: -tpmdev nonexistent,id=tpm0: Parameter 'type' expects a TPM backend type
> Supported TPM types (choose only one):
> passthrough Passthrough TPM backend driver
> emulator TPM emulator backend driver
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
> backends/tpm/tpm_emulator.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index 9605339f93..55cbc9c60e 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -568,6 +568,9 @@ static int tpm_emulator_handle_device_opts(TPMEmulator *tpm_emu, QemuOpts *opts)
> }
>
> tpm_emu->options->chardev = g_strdup(value);
> + } else {
> + error_report("tpm-emulator: missing chardev");
> + goto err;
> }
>
> if (tpm_emulator_prepare_data_fd(tpm_emu) < 0) {
This is minimally invasive.
I'd prefer
Error *err = NULL;
const char *value;
Chardev *dev;
value = qemu_opt_get(opts, "chardev");
if (!value) {
error_report("tpm-emulator: missing chardev");
goto err;
}
dev = qemu_chr_find(value);
if (!dev) {
error_report("tpm-emulator: tpm chardev '%s' not found.", value);
goto err;
}
Your choice.
> @@ -925,6 +928,11 @@ static void tpm_emulator_shutdown(TPMEmulator *tpm_emu)
> {
> ptm_res res;
>
> + if (!tpm_emu->options->chardev) {
> + /* was never properly initialized */
> + return;
> + }
> +
Is this still reachable? If yes, how?
If not, drop the hunk and have my
Reviewed-by: Markus Armbruster <armbru@redhat.com>
> if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SHUTDOWN, &res, 0, sizeof(res)) < 0) {
> error_report("tpm-emulator: Could not cleanly shutdown the TPM: %s",
> strerror(errno));
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tpm_emulator: Report an error if chardev is missing
2020-07-24 14:00 ` Markus Armbruster
@ 2020-07-24 14:11 ` Stefan Berger
0 siblings, 0 replies; 4+ messages in thread
From: Stefan Berger @ 2020-07-24 14:11 UTC (permalink / raw)
To: Markus Armbruster, Stefan Berger; +Cc: marcandre.lureau, philmd, qemu-devel
On 7/24/20 10:00 AM, Markus Armbruster wrote:
> Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
>
>> This patch fixes the odd error reporting when trying to send a file
>> descriptor to the TPM emulator if one has not passed a valid chardev.
>>
>> $ x86_64-softmmu/qemu-system-x86_64 -tpmdev emulator,id=tpm0
>> qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: Failed to send CMD_SET_DATAFD: Success
>> qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: Could not cleanly shutdown the TPM: Success
>>
>> This is the new error report:
>>
>> $ x86_64-softmmu/qemu-system-x86_64 -tpmdev emulator,id=tpm0
>> qemu-system-x86_64: -tpmdev emulator,id=tpm0: tpm-emulator: missing chardev
> ~~~~~~~~~~~~~~
>
> The "tpm-emulator: " part looks superfluous.
>
> Hmm, many error messages in this file have it. Feel free to keep it
> now for consistency, and clean it up later.
>
> We usually report missing option parameters like "parameter 'chardev' is
> missing". Please consider sticking to that.
Fixed.
>
>> This change does not hide the display of supported TPM types if a non-existent type is passed:
>>
>> $ x86_64-softmmu/qemu-system-x86_64 -tpmdev nonexistent,id=tpm0
>> qemu-system-x86_64: -tpmdev nonexistent,id=tpm0: Parameter 'type' expects a TPM backend type
>> Supported TPM types (choose only one):
>> passthrough Passthrough TPM backend driver
>> emulator TPM emulator backend driver
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> ---
>> backends/tpm/tpm_emulator.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
>> index 9605339f93..55cbc9c60e 100644
>> --- a/backends/tpm/tpm_emulator.c
>> +++ b/backends/tpm/tpm_emulator.c
>> @@ -568,6 +568,9 @@ static int tpm_emulator_handle_device_opts(TPMEmulator *tpm_emu, QemuOpts *opts)
>> }
>>
>> tpm_emu->options->chardev = g_strdup(value);
>> + } else {
>> + error_report("tpm-emulator: missing chardev");
>> + goto err;
>> }
>>
>> if (tpm_emulator_prepare_data_fd(tpm_emu) < 0) {
> This is minimally invasive.
>
> I'd prefer
>
> Error *err = NULL;
> const char *value;
> Chardev *dev;
>
> value = qemu_opt_get(opts, "chardev");
> if (!value) {
> error_report("tpm-emulator: missing chardev");
> goto err;
> }
>
> dev = qemu_chr_find(value);
> if (!dev) {
> error_report("tpm-emulator: tpm chardev '%s' not found.", value);
> goto err;
> }
>
> Your choice.
Ok, changed it to this.
>
>> @@ -925,6 +928,11 @@ static void tpm_emulator_shutdown(TPMEmulator *tpm_emu)
>> {
>> ptm_res res;
>>
>> + if (!tpm_emu->options->chardev) {
>> + /* was never properly initialized */
>> + return;
>> + }
>> +
> Is this still reachable? If yes, how?
tpm_emulator_inst_finalize still runs.
Sending V2. Thanks.
>
> If not, drop the hunk and have my
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
>> if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SHUTDOWN, &res, 0, sizeof(res)) < 0) {
>> error_report("tpm-emulator: Could not cleanly shutdown the TPM: %s",
>> strerror(errno));
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-24 14:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-24 13:30 [PATCH] tpm_emulator: Report an error if chardev is missing Stefan Berger
2020-07-24 13:58 ` Marc-André Lureau
2020-07-24 14:00 ` Markus Armbruster
2020-07-24 14:11 ` Stefan Berger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.