Re: [libvirt] [PATCH 3/6] Report secret usage error message similarly
On 08/16/2013 12:34 PM, Ján Tomko wrote:
On 08/08/2013 02:43 AM, John Ferlan wrote:
> 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.
This error is a failure to find the secret value (e.g. call to
'secretGetValue' to the secret driver) given the "secret key"
(virSecretPtr) which was obtained by using either the UUID or Usage value.
The username is mainly for convenience in the error message since it's
not used by the secret driver. The username would be passed to whatever
the code is authenticating to along with the secret value pulled from
the secret driver.
I suppose username could be removed from the message, but it just seemed
easier to reference the username as well since it's part of the xml that
would use the secret.
> + }
> 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'"?
This error message is the inability to find the "secret key"
(virSecretPtr) given either a UUID or Usage value (the output of that
virsh secret-list).
Is the following more appropriate?
change
_("%s username '%s' specified but secret for "
"uuid '%s' not found")
to be:
_("no secret matches uuid '%s'")
and
_("%s username '%s' specified but secret for "
"usage value '%s' not found"),
to be:
_("no secret matches usage value '%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);
>
I removed this since it's unrelated - that's just my desire to have 80
column displays.
John