Re: [PATCH v2 2/3] qemu: hotplug: Fix mlock limit handling on memory hotplug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 09, 2015 at 13:11:16 -0500, John Ferlan wrote:
> On 11/09/2015 07:50 AM, Peter Krempa wrote:
> > If mlock is required either due to use of VFIO hostdevs or due to the
> > fact that it's enabled it needs to be tweaked prior to adding new memory
> > or after removing a module. Add a helper to determine when it's
> > necessary and reuse it both on hotplug and hotunplug.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1273491
> > ---
> >  src/qemu/qemu_domain.c  | 27 +++++++++++++++++++++++++++
> >  src/qemu/qemu_domain.h  |  1 +
> >  src/qemu/qemu_hotplug.c | 32 +++++++++++++++++++++++++++++---
> >  3 files changed, 57 insertions(+), 3 deletions(-)
> > 

[...]

> > @@ -1802,16 +1803,25 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
> >          goto cleanup;
> >      }
> > 
> > +    mlock = qemuDomainRequiresMlock(vm->def);
> > +
> > +    if (mlock &&
> > +        virProcessSetMaxMemLock(vm->pid,
> > +                                qemuDomainGetMlockLimitBytes(vm->def)) < 0) {
> > +        mlock = false;
> 
> Coverity found a need for:
> 
>     virJSONValueFree(props);
> 
> 
> ACK - with the adjustment
> 
> John
> 
> BTW: If you think it matters - if we got the LimitBytes before inserting
> vm->mem to the domain def, then we could know whether it changed anda

In that case I'd rather make qemuDomainRequiresMlock less generic (it'd
include "memory hotplug" in some form in the name) which would return
false in an additional case when 'hard limit' was specified. I was
actually thinking of doing it this way but I've opted for the more
generic approach.

> whether we needed to Set it here and the error path. One would like to
> believe an initial setting to hard_limit (such as 4G) wouldn't have a
> failure if someone passed 4G again...

This actually should not happen (TM) while libvirtd is privileged.

Peter

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]