[libvirt] [PATCH v2 3/3] virsh: Add support for text based polkit authentication

[John Ferlan]

[...]

>> +        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
> 
>