[libvirt] [PATCH v2] Return more error output if policykit auth fails.
Several not uncommon issues can be diagnosed through pkcheck output, like lack of/malfunctioning desktop agent, or lack of/malfunctioning polkit dbus agent. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- v2: Don't print (null) for empty output daemon/remote.c | 12 ++++++++++-- 1 files changed, 10 insertions(+), 2 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 4725896..d2150bf 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2472,7 +2472,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, int status = -1; char *ident = NULL; bool authdismissed = 0; - char *pkout = NULL; + char *pkout = NULL, *pkerr = NULL; struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); virCommandPtr cmd = NULL; @@ -2484,6 +2484,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, cmd = virCommandNewArgList(PKCHECK_PATH, "--action-id", action, NULL); virCommandSetOutputBuffer(cmd, &pkout); + virCommandSetErrorBuffer(cmd, &pkerr); VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(client)); if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_POLKIT) { @@ -2537,15 +2538,22 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, error: virCommandFree(cmd); VIR_FREE(ident); - VIR_FREE(pkout); virResetLastError(); + if (authdismissed) { virNetError(VIR_ERR_AUTH_CANCELLED, "%s", _("authentication cancelled by user")); + } else if (pkout || pkerr) { + virNetError(VIR_ERR_AUTH_FAILED, "%s %s", + pkerr ? pkerr : "", + pkout ? pkout : ""); } else { virNetError(VIR_ERR_AUTH_FAILED, "%s", _("authentication failed")); } + + VIR_FREE(pkout); + VIR_FREE(pkerr); virNetMessageSaveError(rerr); virMutexUnlock(&priv->lock); return -1; -- 1.7.7.5
On 01/27/2012 03:02 PM, Cole Robinson wrote:
Several not uncommon issues can be diagnosed through pkcheck output, like lack of/malfunctioning desktop agent, or lack of/malfunctioning polkit dbus agent.
Signed-off-by: Cole Robinson <crobinso@redhat.com> ---
+ } else if (pkout || pkerr) { + virNetError(VIR_ERR_AUTH_FAILED, "%s %s", + pkerr ? pkerr : "", + pkout ? pkout : "");
Always printing both is also okay, but now we run into the issue that you might have a trailing space if there is no pkerr. This almost argues that we should have a mode in virCommand where you can request that stdout and stderr are collected into the same string (basically, by redirecting stderr onto stdout before doing string collection); I'll play with that idea. In the meantime, ACK. A trailing space isn't the end of the world, especially if I can get the virCommand changes to work and avoid it in that way. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/27/2012 05:32 PM, Eric Blake wrote:
On 01/27/2012 03:02 PM, Cole Robinson wrote:
Several not uncommon issues can be diagnosed through pkcheck output, like lack of/malfunctioning desktop agent, or lack of/malfunctioning polkit dbus agent.
Signed-off-by: Cole Robinson <crobinso@redhat.com> ---
+ } else if (pkout || pkerr) { + virNetError(VIR_ERR_AUTH_FAILED, "%s %s", + pkerr ? pkerr : "", + pkout ? pkout : "");
Always printing both is also okay, but now we run into the issue that you might have a trailing space if there is no pkerr.
This almost argues that we should have a mode in virCommand where you can request that stdout and stderr are collected into the same string (basically, by redirecting stderr onto stdout before doing string collection); I'll play with that idea.
Yeah I actually thought about sending a patch to add just that, but just stuck it at the end of my todo list :)
In the meantime, ACK. A trailing space isn't the end of the world, especially if I can get the virCommand changes to work and avoid it in that way.
Thanks Eric, pushed now. - Cole
participants (2)
-
Cole Robinson -
Eric Blake