On Mon, Nov 12, 2018 at 12:43:11PM +0100, Marc Hartmayer wrote:
On Thu, Nov 01, 2018 at 10:18 AM +0100, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
On Thu, Nov 01, 2018 at 09:31:50AM +0100, Martin Kletzander wrote:
On Tue, Oct 30, 2018 at 03:07:59PM +0000, Daniel P. Berrangé wrote:
On Tue, Oct 30, 2018 at 03:45:36PM +0100, Michal Privoznik wrote:
On 10/30/2018 02:46 PM, Michal Privoznik wrote:
It is kernel problem. In my testing, the moment I call:
ioctl(kvm, KVM_CREATE_VM, 0);
Okay, I have to retract this claim. 'udevadm monitor' shows some events:
KERNEL[3631.129645] change /devices/virtual/misc/kvm (misc) UDEV [3631.130816] change /devices/virtual/misc/kvm (misc)
and stopping udevd leaves all three times untouched. So it is udev after all. I just don't know how to find the rule that is causing the issue. Anyway, as for this patch, I think we can merge it in the end, can't we?
Not really. Udev is in use everywhere, so this behaviour makes the patch useless in practice, even though it is technically right in theory :-(
Does it? With this behaviour we still do the "expensive" work after any machine has started. But for one machine starting it still has the effect of running it only once. And we *need* to run it for each machine unless we also cache the result per (at least) user:group of that machine as every machine can run under different user:group.
Yes, with the current patch it still rechecks it for each VM startup
But that happens only because of udev (see Michal’s answer).
, but I was going to suggest the caching of this is simply put in a global static variable as there's no reason to record this device permissions state in the per VM caps cache.
I’m not sure I understand you correctly, but this patch doesn’t use the VM/QEMU capabilities cache (that would be 'struct _virQEMUCaps') for the caching. Instead it uses 'struct _virQEMUCapsCachePriv' and this struct is initialized only once when initializing the QEMU driver. This should be pretty similar to your proposal with the global static variable.
Oh yes, I missed that. I think this is fine then, and I don't think we need to deal with checking against arbitrary user ids 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 :|