[libvirt] [BUG] mlock support breakage

[Luiz Capitulino]

On Wed, 15 Mar 2017 08:59:34 +0100
Jiri Denemark <jdenemar at redhat.com> wrote:

> On Tue, Mar 14, 2017 at 15:54:25 -0400, Luiz Capitulino wrote:
> > On Tue, 14 Mar 2017 20:28:12 +0100
> > Andrea Bolognani <abologna at redhat.com> wrote:
> >   
> > > On Tue, 2017-03-14 at 14:54 -0400, Luiz Capitulino wrote:  
> > > > > It's unfortunate that the current, buggy behavior made
> > > > > it look like you didn't necessarily have to worry about
> > > > > this. If we fix it, existing guests will fail to start
> > > > > right away instead of possibly crashing in the future:
> > > > > while that's going to be very annoying in the short run,    
> > > > 
> > > > It breaks existing guests, so it's beyond annoying.  
> 
> Because they were relying on a bug in our code rather than following the
> documentation.

The documentation was strictly followed. The documentation strongly
advises against using <hard_limit>. We did that, everything worked
as expected.

I don't think it's a bug that things just worked. That's how
it's supposed to be and that's how it's for VFIO support.

The bug is that it's not working now.

> > > Existing guests are already broken, it's just that the
> > > breakage has not hit them yet ;)  
> > 
> > We should prevent that from happening.  
> 
> The only way we could achieve this is to set the memory locking limit to
> "unlimited" by default, computing the limit is impossible unless the
> result will be bigger than host's memory which is effectively equivalent
> to "unlimited".

I agree. I also suggested setting mlock limit to infinity when
libvirt sees <locked/> in the XML.

> > > > > Luiz mentioned the fact that you can't set the memory
> > > > > locking limit to "unlimited" with the current <hard_limit>
> > > > > element: that's something we can, and should, address.
> > > > > With that implemented, the administrator will have full
> > > > > control on the memory limit and will be able to implement
> > > > > the policy that best suits the use case at hand.    
> > > > 
> > > > Asking <locked/> users to set <hard_limit> to "unlimited"
> > > > is a much worse solution than automatically setting the
> > > > memory lock limit to infinity in libvirt, for the reasons
> > > > I outlined in my first email.  
> 
> Memory locking limit and memory hard limit are separate things even
> though they can be set by a single value in domain XML. Hard limit is by
> default unlimited and a domain will just be killed if it consumes too
> much memory. However setting memory locking limit to unlimited may cause
> an OOM condition inside the host kernel, which is why it is not and
> shouldn't be the default value. However, ...

Well, I was wondering if OOM killer (and Linux's memory reclaim
logic) would work the same way (except for swapping pages, of
course) for processes with infinity mlock limit. If the answer
is yes, then a malicious guest trying a DDoS attack would only
be partially successful.

> > > Removing all memory locking limits should be something that
> > > admins very carefully opt-in into, because of the potential
> > > host DoS consequences. Certainly not the default.  
> > 
> > There's no opt-in with <locked/>, it is mandatory to increase
> > the mlock limit. Asking users to do this themselves is only
> > adding an extra step that's causing breakage right now.  
> 
> ... we could consider <locked/> to be the explicit request for
> setting an infinite memory locking limit and letting users set a lower
> limit with hard_limit if they want.

That's exactly how I see it! It seems we're total agreement.

Now, something has just occurred to me: shouldn't VFIO have
the same problem? It's the same hard limit that's set.