[libvirt] [PATCH 1/8] Revert "qemu: Forbid <memoryBacking><locked> without <memtune><hard_limit>"

Andrea Bolognani posted 8 patches 8 years, 10 months ago
There is a newer version of this series
[libvirt] [PATCH 1/8] Revert "qemu: Forbid <memoryBacking><locked> without <memtune><hard_limit>"
Posted by Andrea Bolognani 8 years, 10 months ago
This reverts commit c2e60ad0e5124482942164e5fec088157f5e716a.

Turns out this check is excessively strict: there are ways
other than <memtune><hard_limit> to raise the memory locking
limit for QEMU processes, one prominent example being
tweaking /etc/security/limits.conf.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1431793
---
 src/qemu/qemu_domain.c                           | 10 ----------
 tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml |  3 ---
 2 files changed, 13 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c239a06..8fa43f2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2919,16 +2919,6 @@ qemuDomainDefValidate(const virDomainDef *def,
         }
     }
 
-    /* Memory locking can only work properly if the memory locking limit
-     * for the QEMU process has been raised appropriately: the default one
-     * is extrememly low, so there's no way the guest will fit in there */
-    if (def->mem.locked && !virMemoryLimitIsSet(def->mem.hard_limit)) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("Setting <memoryBacking><locked> requires "
-                         "<memtune><hard_limit> to be set as well"));
-        goto cleanup;
-    }
-
     if (qemuDomainDefValidateVideo(def) < 0)
         goto cleanup;
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml b/tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml
index 2046663..20a5eaa 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml
@@ -3,9 +3,6 @@
   <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
   <memory unit='KiB'>219136</memory>
   <currentMemory unit='KiB'>219136</currentMemory>
-  <memtune>
-    <hard_limit unit='KiB'>256000</hard_limit>
-  </memtune>
   <memoryBacking>
     <locked/>
   </memoryBacking>
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/8] Revert "qemu: Forbid <memoryBacking><locked> without <memtune><hard_limit>"
Posted by Luiz Capitulino 8 years, 10 months ago
On Thu, 23 Mar 2017 19:16:40 +0100
Andrea Bolognani <abologna@redhat.com> wrote:

> This reverts commit c2e60ad0e5124482942164e5fec088157f5e716a.
> 
> Turns out this check is excessively strict: there are ways
> other than <memtune><hard_limit> to raise the memory locking
> limit for QEMU processes, one prominent example being
> tweaking /etc/security/limits.conf.

Actually, it seems that limits.conf doesn't work with libvirt
as mentioned by Daniel in another thread. I didn't know this
myself btw.

This makes this series even more important because only through
libvirt we can set this limit to infinity.

> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1431793
> ---
>  src/qemu/qemu_domain.c                           | 10 ----------
>  tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml |  3 ---
>  2 files changed, 13 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index c239a06..8fa43f2 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2919,16 +2919,6 @@ qemuDomainDefValidate(const virDomainDef *def,
>          }
>      }
>  
> -    /* Memory locking can only work properly if the memory locking limit
> -     * for the QEMU process has been raised appropriately: the default one
> -     * is extrememly low, so there's no way the guest will fit in there */
> -    if (def->mem.locked && !virMemoryLimitIsSet(def->mem.hard_limit)) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("Setting <memoryBacking><locked> requires "
> -                         "<memtune><hard_limit> to be set as well"));
> -        goto cleanup;
> -    }
> -
>      if (qemuDomainDefValidateVideo(def) < 0)
>          goto cleanup;
>  
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml b/tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml
> index 2046663..20a5eaa 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml
> @@ -3,9 +3,6 @@
>    <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
>    <memory unit='KiB'>219136</memory>
>    <currentMemory unit='KiB'>219136</currentMemory>
> -  <memtune>
> -    <hard_limit unit='KiB'>256000</hard_limit>
> -  </memtune>
>    <memoryBacking>
>      <locked/>
>    </memoryBacking>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/8] Revert "qemu: Forbid <memoryBacking><locked> without <memtune><hard_limit>"
Posted by Andrea Bolognani 8 years, 10 months ago
On Fri, 2017-03-24 at 13:36 -0400, Luiz Capitulino wrote:
> > Turns out this check is excessively strict: there are ways
> > other than <memtune><hard_limit> to raise the memory locking
> > limit for QEMU processes, one prominent example being
> > tweaking /etc/security/limits.conf.
> 
> Actually, it seems that limits.conf doesn't work with libvirt
> as mentioned by Daniel in another thread. I didn't know this
> myself btw.
> 
> This makes this series even more important because only through
> libvirt we can set this limit to infinity.

Well, it *does* work if you set it up properly, eg. raise the
memory locking limit for the user under which libvirtd will
run instead of the user under which QEMU processes will run.

Doing so is very counter-intuitive, though.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/8] Revert "qemu: Forbid <memoryBacking><locked> without <memtune><hard_limit>"
Posted by Luiz Capitulino 8 years, 10 months ago
On Mon, 27 Mar 2017 10:33:52 +0200
Andrea Bolognani <abologna@redhat.com> wrote:

> On Fri, 2017-03-24 at 13:36 -0400, Luiz Capitulino wrote:
> > > Turns out this check is excessively strict: there are ways
> > > other than <memtune><hard_limit> to raise the memory locking
> > > limit for QEMU processes, one prominent example being
> > > tweaking /etc/security/limits.conf.  
> > 
> > Actually, it seems that limits.conf doesn't work with libvirt
> > as mentioned by Daniel in another thread. I didn't know this
> > myself btw.
> > 
> > This makes this series even more important because only through
> > libvirt we can set this limit to infinity.  
> 
> Well, it *does* work if you set it up properly, eg. raise the
> memory locking limit for the user under which libvirtd will
> run instead of the user under which QEMU processes will run.

Doesn't libvirtd run as root?

> 
> Doing so is very counter-intuitive, though.
> 
> -- 
> Andrea Bolognani / Red Hat / Virtualization
> 


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/8] Revert "qemu: Forbid <memoryBacking><locked> without <memtune><hard_limit>"
Posted by Andrea Bolognani 8 years, 10 months ago
On Tue, 2017-03-28 at 09:27 -0400, Luiz Capitulino wrote:
> > Well, it *does* work if you set it up properly, eg. raise the
> > memory locking limit for the user under which libvirtd will
> > run instead of the user under which QEMU processes will run.
> 
> Doesn't libvirtd run as root?

Yes. You can set the memory locking limit to whatever you
want for the root user, and then QEMU processes spawned by
libvirtd will inherit it.

-- 
Andrea Bolognani / Red Hat / Virtualization

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