[libvirt] [PATCH 3/6] Report secret usage error message similarly

[Ján Tomko]

On 08/08/2013 02:43 AM, John Ferlan wrote:
> Each of the modules handled reporting error messages from the secret fetching
> slightly differently with respect to the error. Provide a similar message
> for each error case and provide as much data as possible.
> ---
>  src/qemu/qemu_command.c             | 28 ++++++++++++++++++++++------
>  src/storage/storage_backend_iscsi.c | 28 ++++++++++++++++++++++------
>  src/storage/storage_backend_rbd.c   | 35 ++++++++++++++++++++++++++++-------
>  3 files changed, 72 insertions(+), 19 deletions(-)
> 

...

> diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c
> index ee8dd2e..55bf544 100644
> --- a/src/storage/storage_backend_iscsi.c
> +++ b/src/storage/storage_backend_iscsi.c
> @@ -732,15 +732,31 @@ virStorageBackendISCSISetAuth(const char *portal,
>              conn->secretDriver->secretGetValue(secret, &secret_size, 0,
>                                                 VIR_SECRET_GET_VALUE_INTERNAL_CALL);
>          if (!secret_value) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR,
> -                           _("could not get the value of the secret "
> -                             "for username %s"), chap.username);
> +            if (chap.secret.uuidUsable) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("could not get the value of the secret "
> +                                 "for username %s using uuid '%s'"),
> +                                 chap.username, chap.secret.uuid);
> +            } else {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("could not get the value of the secret "
> +                                 "for username %s using usage value '%s'"),
> +                                 chap.username, chap.secret.usage);

This looks to me like the username plays a part in getting the secret value,
but I'm not sure how to rephrase it while still keeping the username there.

> +            }
>              goto cleanup;
>          }
>      } else {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("username '%s' specified but secret not found"),
> -                       chap.username);
> +        if (chap.secret.uuidUsable) {
> +            virReportError(VIR_ERR_NO_SECRET,
> +                           _("username '%s' specified but the secret for "
> +                             "uuid '%s' not found"),
> +                           chap.username, chap.secret.uuid);
> +        } else {
> +            virReportError(VIR_ERR_NO_SECRET,
> +                           _("username '%s' specified but the secret for "
> +                             "usage value '%s' not found"),
> +                           chap.username, chap.secret.usage);
> +        }
>          goto cleanup;

With VIR_ERR_NO_SECRET, this says "secret not found" twice:
error: Secret not found: username 'nahasapeemapetilon' specified but the
secret for usage value 'pwagmattasquarmsettport' not found

But that's minor. Maybe "... but no secret matches usage '%s'"?

>      }
>  
> diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
> index e3340f6..9c6d179 100644
> --- a/src/storage/storage_backend_rbd.c
> +++ b/src/storage/storage_backend_rbd.c
...
>  
> -        secret_value = conn->secretDriver->secretGetValue(secret, &secret_value_size, 0,
> +        secret_value = conn->secretDriver->secretGetValue(secret,
> +                                                          &secret_value_size, 0,
>                                                            VIR_SECRET_GET_VALUE_INTERNAL_CALL);
>  

Unrelated whitespace change.

ACK if you separate it from the rest of the patch.

Jan