[libvirt] [PATCH] qemu: Forbid <memoryBacking><locked> without <memtune><hard_limit>

Andrea Bolognani posted 1 patch 7 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1486402199-13957-1-git-send-email-abologna@redhat.com
There is a newer version of this series
src/qemu/qemu_domain.c                               | 20 ++++++++++++++++++++
tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml     |  3 +++
....xml => qemuxml2argv-mlock-without-hardlimit.xml} |  0
tests/qemuxml2argvtest.c                             |  1 +
4 files changed, 24 insertions(+)
copy tests/qemuxml2argvdata/{qemuxml2argv-mlock-on.xml => qemuxml2argv-mlock-without-hardlimit.xml} (100%)
[libvirt] [PATCH] qemu: Forbid <memoryBacking><locked> without <memtune><hard_limit>
Posted by Andrea Bolognani 7 years, 2 months ago
In order for memory locking to work, the hard limit on memory
locking (and usage) has to be set appropriately by the user.

The documentation mentions the requirement already: with this
patch, it's going to be enforced by runtime checks as well.

Note that this will make existing guests that don't satisfy
the requirement disappear; that said, such guests have never
been able to start in the first place.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1316774
---
 src/qemu/qemu_domain.c                               | 20 ++++++++++++++++++++
 tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml     |  3 +++
 ....xml => qemuxml2argv-mlock-without-hardlimit.xml} |  0
 tests/qemuxml2argvtest.c                             |  1 +
 4 files changed, 24 insertions(+)
 copy tests/qemuxml2argvdata/{qemuxml2argv-mlock-on.xml => qemuxml2argv-mlock-without-hardlimit.xml} (100%)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c6ce090..bb29cfe 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2629,6 +2629,23 @@ qemuDomainDefVcpusPostParse(virDomainDefPtr def)
 
 
 static int
+qemuDomainDefMemoryLockingPostParse(virDomainDefPtr 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"));
+        return -1;
+    }
+
+    return 0;
+}
+
+
+static int
 qemuDomainDefPostParse(virDomainDefPtr def,
                        virCapsPtr caps,
                        unsigned int parseFlags,
@@ -2692,6 +2709,9 @@ qemuDomainDefPostParse(virDomainDefPtr def,
     if (qemuDomainDefVcpusPostParse(def) < 0)
         goto cleanup;
 
+    if (qemuDomainDefMemoryLockingPostParse(def) < 0)
+        goto cleanup;
+
     ret = 0;
  cleanup:
     virObjectUnref(qemuCaps);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml b/tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml
index 20a5eaa..2046663 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml
@@ -3,6 +3,9 @@
   <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>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml b/tests/qemuxml2argvdata/qemuxml2argv-mlock-without-hardlimit.xml
similarity index 100%
copy from tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml
copy to tests/qemuxml2argvdata/qemuxml2argv-mlock-without-hardlimit.xml
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 3532cb5..9b2fec5 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2129,6 +2129,7 @@ mymain(void)
     DO_TEST_FAILURE("mlock-on", NONE);
     DO_TEST("mlock-off", QEMU_CAPS_REALTIME_MLOCK);
     DO_TEST("mlock-unsupported", NONE);
+    DO_TEST_PARSE_ERROR("mlock-without-hardlimit", NONE);
 
     DO_TEST_PARSE_ERROR("pci-bridge-negative-index-invalid",
                         QEMU_CAPS_DEVICE_PCI_BRIDGE);
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Forbid <memoryBacking><locked> without <memtune><hard_limit>
Posted by Jiri Denemark 7 years, 2 months ago
On Mon, Feb 06, 2017 at 18:29:59 +0100, Andrea Bolognani wrote:
> In order for memory locking to work, the hard limit on memory
> locking (and usage) has to be set appropriately by the user.
> 
> The documentation mentions the requirement already: with this
> patch, it's going to be enforced by runtime checks as well.
> 
> Note that this will make existing guests that don't satisfy
> the requirement disappear; that said, such guests have never
> been able to start in the first place.

Yes, could not start or if they started they would just crash
afterwards. But with your patch they would just disappear and the user
would not even be able to fix the XML. I think the check should be done
prior to starting a domain to avoid this unfortunate behaviour.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Forbid <memoryBacking><locked> without <memtune><hard_limit>
Posted by Pavel Hrdina 7 years, 2 months ago
On Tue, Feb 07, 2017 at 08:43:56AM +0100, Jiri Denemark wrote:
> On Mon, Feb 06, 2017 at 18:29:59 +0100, Andrea Bolognani wrote:
> > In order for memory locking to work, the hard limit on memory
> > locking (and usage) has to be set appropriately by the user.
> > 
> > The documentation mentions the requirement already: with this
> > patch, it's going to be enforced by runtime checks as well.
> > 
> > Note that this will make existing guests that don't satisfy
> > the requirement disappear; that said, such guests have never
> > been able to start in the first place.
> 
> Yes, could not start or if they started they would just crash
> afterwards. But with your patch they would just disappear and the user
> would not even be able to fix the XML. I think the check should be done
> prior to starting a domain to avoid this unfortunate behaviour.
> 
> Jirka

Actually there is a third option, we have domainValidateCallback
that is what you are looking for.  This function is called only
when defining new XML and is skipped while parsing the XML from
disk on libvirtd startup or while restoring from snapshot and etc.

It is also called before domain is started.

Pavel

> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Forbid <memoryBacking><locked> without <memtune><hard_limit>
Posted by Andrea Bolognani 7 years, 2 months ago
On Tue, 2017-02-07 at 12:07 +0100, Pavel Hrdina wrote:
> Actually there is a third option, we have domainValidateCallback
> that is what you are looking for.  This function is called only
> when defining new XML and is skipped while parsing the XML from
> disk on libvirtd startup or while restoring from snapshot and etc.
> 
> It is also called before domain is started.

Oh, neat! I'll give it a go :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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