7:50 a.m.
On 10/01/2015 08:10 AM, Martin Kletzander wrote:
We are using memory-backing-file even when it's not needed, for
example
if user requests hugepages for mamory backing, but does not specify any
^^^^^^
Different body part altogether ;-)
pagesize, neither memory node pinning. This causes migrations to
fail
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Doesn't read well - as in I'm not quite sure what you meant... Neither
and nor are usually paired together if that helps any
when migrating from older libvirt that did not do this. So similarly
to
commit 7832fac84741d65e851dbdbfaf474785cbfdcf3c which does it for
memory-backend-ram, this commit makes is more generic and
backend-agnostic, so the backend is not used if there is no specific
pagesize of hugepages requested, no nodeset the memory node should be
bound to, no memory access change required, and so on.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1266856
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/qemu/qemu_command.c | 36 ++++++++++------------
.../qemuxml2argv-hugepages-numa.args | 6 ++--
2 files changed, 19 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 321a5e931350..7ff3e535a543 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5120,12 +5120,6 @@ qemuBuildMemoryBackendStr(unsigned long long size,
}
if (pagesize || hugepage) {
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("this qemu doesn't support hugepage memory
backing"));
- goto cleanup;
- }
-
Wasn't clear to me why this was removed. The code that follows within
the if will create a memory-backend-file
I see below the check is made in the else case but there's no hugepage
the list of !x && !y && !z ...
Perhaps I'm being thrown by the use of "pagesize" and "hugepage"
conditions. The 'hugepage' only gets set if 'pagesize = 0'... If
'hugepage' is set, can pagesize be '0' (outside if pagesize ==
system_page_size)
Sorry - just trying to think through the logic...
I guess removing it is fine, but it's not obvious without digging in a
bit...
if (pagesize) {
/* Now lets see, if the huge page we want to use is even mounted
* and ready to use */
@@ -5204,29 +5198,31 @@ qemuBuildMemoryBackendStr(unsigned long long size,
goto cleanup;
}
- if (!hugepage && !pagesize) {
-
- if ((userNodeset || nodeSpecified || force) &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+ /* If none of the following is requested... */
+ if (!pagesize && !userNodeset && !memAccess &&
!nodeSpecified && !force) {
hmmm. force isn't documented - in the input args... I know different problem
+ /* report back that using the new backend is not necessary
+ * to achieve the desired configuration */
+ ret = 1;
+ } else {
+ /* otherwise check the required capability */
+ if (STREQ(*backendType, "memory-backend-file") &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("this qemu doesn't support the "
- "memory-backend-ram object"));
+ "memory-backend-file object"));
goto cleanup;
+ } else if (STREQ(*backendType, "memory-backend-ram") &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("this qemu doesn't support the "
+ "memory-backend-ram object"));
}
- /* report back that using the new backend is not necessary to achieve
- * the desired configuration */
- if (!userNodeset && !nodeSpecified) {
- *backendProps = props;
- props = NULL;
- ret = 1;
- goto cleanup;
- }
+ ret = 0;
As confusing as the diff is - it looks cleaner in it's final version.
Hopefully Michal or Peter can also look at the final product - it looks
good to me though
ACK with some adjustments
John
}
*backendProps = props;
props = NULL;
- ret = 0;
cleanup:
virJSONValueFree(props);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args
index 37511b109b6e..3496cf1a732d 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args
@@ -4,9 +4,9 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=spice \
-M pc-i440fx-2.3 \
-m size=1048576k,slots=16,maxmem=1099511627776k \
-smp 2 \
--object memory-backend-file,id=ram-node0,prealloc=yes,\
-mem-path=/dev/hugepages2M/libvirt/qemu,size=1073741824 \
--numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \
+-mem-prealloc \
+-mem-path /dev/hugepages2M/libvirt/qemu \
+-numa node,nodeid=0,cpus=0-1,mem=1024 \
-object memory-backend-file,id=memdimm0,prealloc=yes,\
mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=1-3,policy=bind \
-device pc-dimm,node=0,memdev=memdimm0,id=dimm0 \