On Wed, Jan 30, 2019 at 02:39:54PM +0100, Erik Skultety wrote: > > > > though, we need a #ifdef check for existance of PR_CAP_AMBIENT > > > > > > > > > An alternative question I've been playing ever since we exchanged the > > > > > last few > > > > > emails is that can't we wait until the ioctls are compared against > > > > > permissions > > > > > in kernel so that upstream libvirt (and downstream too for that > > > > > matter) doesn't > > > > > have to work around it and stick with that workaround for eternity? > > > > > > > > IIUC, the SEV feature has already shipped with distros, so we'd > > > > effectively > > > > be saying that what we already shipped is unusable to libvirt. This > > > > doesn't > > > > feel like a desirable story to me. > > > > > > It was, but it never worked, it always has been broken in this way. When > > > we > > > were merging this upstream, we had a terrible shortage of machines and we > > > had > > > to share, so the first person to provision the machine had already taken > > > care > > > of the permissions in order to test so that led to this issue having been > > > overlooked until now. If it ever worked as expected and then we broke it, > > > then > > > any fix from our side would make sense but otherwise I believe we should > > > fix > > > this bottom up. > > > > Well technically it would work if libvirt was configured to run as > > root:root, but yes, that is not a normal or recommended configuration. > > > > Personally I have a preference for userspace solutions, as those are > > pretty straightforward to roll out to people as patches in existing > > releases. Deploying kernel updates is a higher bar to cross for an > > existing release. > > So, can you compile the prctl stuff in kernel conditionally? If so, then > that's > a problem because you may end up with a platform where SEV is supported within > kernel, but you don't have the ambient stuff we have to conditionally compile > in libvirt, so you end up with broken SEV support anyway, I wanted to argue > with centos 7, but the ambient set support was backported to 3.10, so the only > distro where we'd have a problem from userspace POV would be debian 8, but > then > again the kernel there is so old that neither SEV is supported there. > > I understand your point, but it also sounds very agile and I don't think that > compensating with "something that is fast" for "something that is right" is > the > way to go in the long term. Especially since we almost never deprecate stuff > and we can't break compatibility. Trying to work around every issue coming > from your dependencies in your project is highly unsustainable.
With the launching of VMs we've got to a place where libvirt is pretty robust about being able to grant access regardless of what the host OS has done for permissions in /dev. I think its desirable that this same flexibility extends to capabilities probing, which is somethign the dac_override approach allows. IOW, even if the kernel changes /dev/sev as previously discussed, I would keep the dac_override stuff for probing capabilities forever. This makes sure we'll work even if the distro in question has strictly locked down permissions on /dev/kvm or /dev/sev, diverging from the default udev settup 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
