[PATCH-for-5.1 v2 0/2] tpm: Improve error reporting

[PATCH-for-5.1 v2 0/2] tpm: Improve error reporting

[Philippe Mathieu-Daudé]

Improve error reporting by listing TPM backends.

Philippe Mathieu-Daudé (2):
  tpm: Display when no backend is available
  tpm: List the available TPM backends

 tpm.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

-- 
2.21.3

[PATCH-for-5.1 v2 1/2] tpm: Display when no backend is available

[Philippe Mathieu-Daudé]

Display "No TPM backend available in this binary." error when
no backend is available.

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tpm.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tpm.c b/tpm.c
index fe03b24858..e36803a64d 100644
--- a/tpm.c
+++ b/tpm.c
@@ -41,6 +41,22 @@ tpm_be_find_by_type(enum TpmType type)
     return TPM_BACKEND_CLASS(oc);
 }
 
+/*
+ * Walk the list of available TPM backend drivers and count them.
+ */
+static int tpm_backend_drivers_count(void)
+{
+    int count = 0, i;
+
+    for (i = 0; i < TPM_TYPE__MAX; i++) {
+        const TPMBackendClass *bc = tpm_be_find_by_type(i);
+        if (bc) {
+            count++;
+        }
+    }
+    return count;
+}
+
 /*
  * Walk the list of available TPM backend drivers and display them on the
  * screen.
@@ -87,6 +103,11 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp)
     TPMBackend *drv;
     int i;
 
+    if (!tpm_backend_drivers_count()) {
+        error_setg(errp, "No TPM backend available in this binary.");
+        return 1;
+    }
+
     if (!QLIST_EMPTY(&tpm_backends)) {
         error_setg(errp, "Only one TPM is allowed.");
         return 1;
-- 
2.21.3

[PATCH-for-5.1 v2 2/2] tpm: List the available TPM backends

[Philippe Mathieu-Daudé]

When an incorrect backend is selected, tpm_display_backend_drivers()
is supposed to list the available backends. However the error is
directly propagated, and we never display the list. The user only
gets "Parameter 'type' expects a TPM backend type" error.

Convert the fprintf(stderr,) calls to error hints propagated with
the error.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Since v1:
- Use g_assert_not_reached after processing 'help' in tpm_config_parse
---
 tpm.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/tpm.c b/tpm.c
index e36803a64d..f883340d1a 100644
--- a/tpm.c
+++ b/tpm.c
@@ -58,23 +58,21 @@ static int tpm_backend_drivers_count(void)
 }
 
 /*
- * Walk the list of available TPM backend drivers and display them on the
- * screen.
+ * Walk the list of available TPM backend drivers and list them as Error hint.
  */
-static void tpm_display_backend_drivers(void)
+static void tpm_list_backend_drivers_hint(Error **errp)
 {
     int i;
 
-    fprintf(stderr, "Supported TPM types (choose only one):\n");
+    error_append_hint(errp, "Supported TPM types (choose only one):\n");
 
     for (i = 0; i < TPM_TYPE__MAX; i++) {
         const TPMBackendClass *bc = tpm_be_find_by_type(i);
         if (!bc) {
             continue;
         }
-        fprintf(stderr, "%12s   %s\n", TpmType_str(i), bc->desc);
+        error_append_hint(errp, "%12s   %s\n", TpmType_str(i), bc->desc);
     }
-    fprintf(stderr, "\n");
 }
 
 /*
@@ -97,6 +95,7 @@ TPMBackend *qemu_find_tpm_be(const char *id)
 
 static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp)
 {
+    ERRP_GUARD();
     const char *value;
     const char *id;
     const TPMBackendClass *be;
@@ -122,7 +121,7 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp)
     value = qemu_opt_get(opts, "type");
     if (!value) {
         error_setg(errp, QERR_MISSING_PARAMETER, "type");
-        tpm_display_backend_drivers();
+        tpm_list_backend_drivers_hint(errp);
         return 1;
     }
 
@@ -131,7 +130,7 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp)
     if (be == NULL) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
                    "a TPM backend type");
-        tpm_display_backend_drivers();
+        tpm_list_backend_drivers_hint(errp);
         return 1;
     }
 
@@ -184,8 +183,8 @@ int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
     QemuOpts *opts;
 
     if (!strcmp(optarg, "help")) {
-        tpm_display_backend_drivers();
-        return -1;
+        tpm_list_backend_drivers_hint(&error_fatal);
+        g_assert_not_reached(); /* Using &error_fatal triggers exit(1). */
     }
     opts = qemu_opts_parse_noisily(opts_list, optarg, true);
     if (!opts) {
-- 
2.21.3

Re: [PATCH-for-5.1 v2 2/2] tpm: List the available TPM backends

[Philippe Mathieu-Daudé]

On 7/23/20 12:39 PM, Philippe Mathieu-Daudé wrote:
> When an incorrect backend is selected, tpm_display_backend_drivers()
> is supposed to list the available backends. However the error is
> directly propagated, and we never display the list. The user only
> gets "Parameter 'type' expects a TPM backend type" error.
> 
> Convert the fprintf(stderr,) calls to error hints propagated with
> the error.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Since v1:
> - Use g_assert_not_reached after processing 'help' in tpm_config_parse
> ---
>  tpm.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/tpm.c b/tpm.c
> index e36803a64d..f883340d1a 100644
> --- a/tpm.c
> +++ b/tpm.c
> @@ -58,23 +58,21 @@ static int tpm_backend_drivers_count(void)
>  }
>  
>  /*
> - * Walk the list of available TPM backend drivers and display them on the
> - * screen.
> + * Walk the list of available TPM backend drivers and list them as Error hint.
>   */
> -static void tpm_display_backend_drivers(void)
> +static void tpm_list_backend_drivers_hint(Error **errp)
>  {
>      int i;
>  
> -    fprintf(stderr, "Supported TPM types (choose only one):\n");
> +    error_append_hint(errp, "Supported TPM types (choose only one):\n");
>  
>      for (i = 0; i < TPM_TYPE__MAX; i++) {
>          const TPMBackendClass *bc = tpm_be_find_by_type(i);
>          if (!bc) {
>              continue;
>          }
> -        fprintf(stderr, "%12s   %s\n", TpmType_str(i), bc->desc);
> +        error_append_hint(errp, "%12s   %s\n", TpmType_str(i), bc->desc);
>      }
> -    fprintf(stderr, "\n");
>  }
>  
>  /*
> @@ -97,6 +95,7 @@ TPMBackend *qemu_find_tpm_be(const char *id)
>  
>  static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp)
>  {
> +    ERRP_GUARD();
>      const char *value;
>      const char *id;
>      const TPMBackendClass *be;
> @@ -122,7 +121,7 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp)
>      value = qemu_opt_get(opts, "type");
>      if (!value) {
>          error_setg(errp, QERR_MISSING_PARAMETER, "type");
> -        tpm_display_backend_drivers();
> +        tpm_list_backend_drivers_hint(errp);
>          return 1;
>      }
>  
> @@ -131,7 +130,7 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp)
>      if (be == NULL) {
>          error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
>                     "a TPM backend type");
> -        tpm_display_backend_drivers();
> +        tpm_list_backend_drivers_hint(errp);
>          return 1;
>      }
>  
> @@ -184,8 +183,8 @@ int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
>      QemuOpts *opts;
>  
>      if (!strcmp(optarg, "help")) {
> -        tpm_display_backend_drivers();
> -        return -1;
> +        tpm_list_backend_drivers_hint(&error_fatal);
> +        g_assert_not_reached(); /* Using &error_fatal triggers exit(1). */

Maybe tpm_config_parse() should take an Error** parameter instead?
And in vl.c:

-- >8 --
 #ifdef CONFIG_TPM
             case QEMU_OPTION_tpmdev:
-                if (tpm_config_parse(qemu_find_opts("tpmdev"), optarg)
< 0) {
-                    exit(1);
-                }
+                tpm_config_parse(qemu_find_opts("tpmdev"), optarg,
+                                 &error_fatal);
                 break;
 #endif
---

>      }
>      opts = qemu_opts_parse_noisily(opts_list, optarg, true);
>      if (!opts) {
> 

Re: [PATCH-for-5.1 v2 0/2] tpm: Improve error reporting

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Improve error reporting by listing TPM backends.

Let's first decide whether to revert commit d10e05f15d5.