On Fri, Feb 12, 2016 at 10:04:58AM -0500, John Ferlan wrote: > > > 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. Oh, I'm an idiot in suggesting this route. So, even when logging in via ssh, chances are that there *is* an agent running - one run by GNOME in your desktop login. The problem is that agent isn't able to display prompts on the ssh text console. So I think we would have to go back to the approach of returning a special error code to let the client detect the problem. So when libvirtd is able to auth due to missing agent, then we should return a new code VIR_ERR_AUTH_UNAVAILABLE, and virsh can check for that to see if it should launch pkttyagent. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list