Re: [PATCH 7/7] util: Make client-side polkit work even with polkit disabled

On Tue, Nov 23, 2021 at 12:55:00PM +0100, Martin Kletzander wrote: > On Mon, Nov 22, 2021 at 10:07:39AM +0000, Daniel P. Berrangé wrote: > > On Mon, Nov 22, 2021 at 11:03:53AM +0100, Martin Kletzander wrote: > > > On Mon, Nov 22, 2021 at 09:23:01AM +0000, Daniel P. Berrangé wrote: > > > > On Sun, Nov 21, 2021 at 12:10:08AM +0100, Martin Kletzander wrote: > > > > > The reason for this is twofold: > > > > > > > > > > - the polkit build option is documented for UNIX socket access checks > > > > > > > > > > - there is no server-side change or dbus call done when enabling this as it only > > > > > starts a polkit agent on the client-side (actually only in virsh) and does not > > > > > need any requirements (starting is skipped if pkttyagent is not installed) > > > > > > > > > > Also move the conditional implementation to the bottom of the file so that it > > > > > does not look like the whole file is build conditionally and the common > > > > > functions are at the top. > > > > > > > > Does this still work correctly on Windows if we try by default ? > > > > > > > > > > Any call to virPolkitAgentAvailable() should return false on Windows > > > unless PKTTYAGENT (/usr/bin/pkttyagent) exists. While thinking about it > > > now I will change that virFileExists() call to virFileIsExecutable() to > > > catch even more possible issues. Anyway since that should return false > > > on Windows (and I hope my presumptions are correct) then > > > virPolkitAgentCreate() should report an error, just like it would > > > without this patch if connecting to polkit-guarded libvirtd socket > > > (e.g. through ssh). It should actually return a better error with this > > > patch applied. > > > > > > And ctermid() is a POSIX function, not sure what that returns on > > > windows, but it should not even get there as the first check is done > > > against the existence/executability of PKTTYAGENT. > > > > ctermid() doesn't exist in Windows, so it shouldn't even compile if we > > try to build with it ! A conditional willbe needed I expect. > > > > Yeah, with my limited (and mostly forgotten Windows experience) I was > too much reliant on our builds which all have allow_failure: true for > mingw builds. Sorry for that, I'll fix that up. We need to move at least one of our mingw builds off rawhide onto the stable Fedora, so we can remove the allow_failure flag.

Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|