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

On 02/12/2016 08:22 AM, Daniel P. Berrange wrote: > On Fri, Feb 12, 2016 at 07:53:40AM -0500, John Ferlan wrote: >> >> >> On 02/12/2016 06:57 AM, Daniel P. Berrange wrote: >>> On Fri, Feb 12, 2016 at 06:49:22AM -0500, John Ferlan wrote: >>>> [...] >>>> >>>>>> + 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. >>> >>> My point is that you don't actually need to distinguish those two >>> cases directly. You can do this: >>> >>> if (err && err->code == VIR_FROM_POLKIT && err->code == VIR_ER_AUTH_FAILED) { >>> if (!virDBusIsServiceRegistered(...polkit...)) { >> >> Including "virdbus.h" to get virDBusIsServiceRegistered from virsh.c >> sends me down the build system rabbit hole again: >> >> In file included from virsh.c:59:0: >> ../src/util/virdbus.h:27:25: fatal error: dbus/dbus.h: No such file or >> directory >> compilation terminated. >> >> Adding "$(DBUS_CFLAGS)" to the virsh_CLFAGS in Makefile.am still leaves >> me with: >> >> virsh-virsh.o: In function `virshConnect': >> /home/jferlan/git/libvirt.work/tools/virsh.c:183: undefined reference to >> `virDBusIsServiceRegistered' >> collect2: error: ld returned 1 exit status > > Oh we missed it from src/libvirt_private.syms > Ahh... that's it - didn't even consider that option... However, virDBusIsServiceRegistered: "Retruns 0 if service is registered, -1 on fatal error, or -2 if service is not registered" I found passing "org.freedesktop.PolicyKit1" returns 0 every time even whether or not virPolkitAgentCreate has been called... Feels like something like the machine name code that searches by pid would be what would work. As an alternative (since this is the I want to make sure the agent is running path), the pkttyagent also takes a --notify-fd fd parameter. I can work something up to use that.