[...]
+ err = virGetLastError(); + if (err && strstr(err->message, + _("no agent is available to authenticate"))) {
+ if (!pkagent) { + if (!(pkagent = virPolkitAgentCreate())) + goto cleanup; + } + agentstart++; + } else if (err && strstr(err->message, _("authentication failed:"))) {
String matching is pretty unpleasant. I think we can match on err->domain == VIR_FROM_POLKIT && err->code == VIR_ERR_AUTH_FAILED for this.
Using VIR_ERR_AUTH_FAILED I cannot distinguish between the failure of available agent or access denied by policy from virPolkitCheckAuth. Adjusting what virPolkitCheckAuth returns means more code modification since the assumption is -2 has 3 possible issues of which 2 currently are tested by a err->message comparison. I would think in this case, I wouldn't want to create a text agent if access is denied by policy. So should I bite the bullet and adjust the return value checking? Or should I add a new error code "VIR_ERR_AUTH_DENY" and likewise adjust the code/tests to use that rather than the current string comparisons. John
Also instead of trying to match for the agent message, you can just do
if (!virDBusIsServiceRegistered('....polkit service name....'))
to decide whether to then start the agent after an auth failure