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

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