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

Andrea Bolognani posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1486467303-23556-1-git-send-email-abologna@redhat.com
There is a newer version of this series
src/qemu/qemu_command.c                          | 9 +++++++++
tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml | 3 +++
2 files changed, 12 insertions(+)
[libvirt] [PATCH v2] qemu: Forbid <memoryBacking><locked> without <memtune><hard_limit>
Posted by Andrea Bolognani 7 years, 1 month 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,
by forbidding a non-compliant guest from starting at all.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1316774
---
Changes from [v1]

  * Address review feeback:
    - check in BuildCommandLine rather than in PostParse,
      so that non-compliant guests will merely fail to
      start rather than disappear completely.

[v1] https://www.redhat.com/archives/libvir-list/2017-February/msg00180.html

 src/qemu/qemu_command.c                          | 9 +++++++++
 tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml | 3 +++
 2 files changed, 12 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1396661..ca3bcdc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7340,6 +7340,15 @@ qemuBuildMemCommandLine(virCommandPtr cmd,
         qemuBuildMemPathStr(cfg, def, qemuCaps, cmd) < 0)
         return -1;
 
+    /* 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;
+    }
     if (def->mem.locked && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_REALTIME_MLOCK)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("memory locking not supported by QEMU binary"));
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>
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Forbid <memoryBacking><locked> without <memtune><hard_limit>
Posted by Pavel Hrdina 7 years, 1 month ago
On Tue, Feb 07, 2017 at 12:35:03PM +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,
> by forbidding a non-compliant guest from starting at all.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1316774
> ---
> Changes from [v1]
> 
>   * Address review feeback:
>     - check in BuildCommandLine rather than in PostParse,
>       so that non-compliant guests will merely fail to
>       start rather than disappear completely.
> 
> [v1] https://www.redhat.com/archives/libvir-list/2017-February/msg00180.html

NACK, see my review for v1.  This should be implemented in
qemuDomainDefValidate.

Pavel

> 
>  src/qemu/qemu_command.c                          | 9 +++++++++
>  tests/qemuxml2argvdata/qemuxml2argv-mlock-on.xml | 3 +++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 1396661..ca3bcdc 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7340,6 +7340,15 @@ qemuBuildMemCommandLine(virCommandPtr cmd,
>          qemuBuildMemPathStr(cfg, def, qemuCaps, cmd) < 0)
>          return -1;
>  
> +    /* 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;
> +    }
>      if (def->mem.locked && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_REALTIME_MLOCK)) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                         _("memory locking not supported by QEMU binary"));
> 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>
> -- 
> 2.7.4
> 
> --
> 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