On Fri, Oct 26, 2018 at 06:57:54AM +0200, Bjoern Walk wrote: > Daniel P. Berrangé <berrange@xxxxxxxxxx> [2018-10-25, 06:32PM +0100]: > > On Thu, Oct 25, 2018 at 01:47:26PM +0200, Bjoern Walk wrote: > > > Daniel P. Berrangé <berrange@xxxxxxxxxx> [2018-10-24, 10:43PM +0100]: > > > > We could optimize this by jcalling virFileAccessibleAs > > > > once and storing the result in a global. Then just do a > > > > plain stat() call in process to check the st_ctime field > > > > for changes. We only need re-run the heavy virFileAccessibleAs > > > > check if st_ctime has changed (indicating a owner/group/acl > > > > change). > > > > > > But can't access permission change outside of changing the actual device > > > file (e.g. cgroups or other OS capabilities)? Isn't that the whole > > > purpose of the virFileAccessibleAs gymnastics? > > > > Yes, cgroups could restrict access to /dev/kvm, but virFileAccessibleAs > > does not use cgroups, it only cares about using the correct user + group > > membership. > > Sorry, but then I don't understand the purpose of this function at all. > Why would you EVER check permissions like that? A simple stat(2) call > should give you the exact same information, no? The capabilities probing runs as the 'qemu' user and 'qemu' group. If libvirtd stat()s the /dev/kvm device, it will merely find out if the *root* user can access it. We need to chcek if the qemu/qemu user/group can access it, and if you try todo via a stat() then you have to reinvent the kernel logic for checking supplementary group memberships and file ACL membership. The only sensible way is for the process checking to actually be running as the qemu/qemu user and group, hence we fork and change to this user/group pair. 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
