From e255cf02b2a24d19412d9bf08dfa654150d9a31b Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Tue, 8 Aug 2017 16:51:30 +0200 Subject: [PATCH] qemuBuildMemoryBackendStr: Handle one more corner case https://bugzilla.redhat.com/show_bug.cgi?id=1458638 This code is so complicated because we allow enabling the same bits at many places. Just like in this case: huge pages can be enabled by global element under or on per basis. To complicate things a bit more, users are allowed to omit the page size which case the default page size is used. And this is what is causing this bug. If no page size is specified, @pagesize is keeping value of zero throughout whole function. Therefore we need yet another boolean to hold [use, don't use] information as we can't sue @pagesize for that. Signed-off-by: Michal Privoznik Reviewed-by: Martin Kletzander --- src/qemu/qemu_command.c | 22 +++++-- .../qemuxml2argv-hugepages-pages7.args | 35 ++++++++++ .../qemuxml2argv-hugepages-pages7.xml | 65 +++++++++++++++++++ tests/qemuxml2argvtest.c | 2 + .../qemuxml2xmlout-hugepages-pages7.xml | 1 + tests/qemuxml2xmltest.c | 1 + 6 files changed, 120 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-hugepages-pages7.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6046505963..68fc137069 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3250,8 +3250,6 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps, virBitmapPtr autoNodeset, bool force) { - virDomainHugePagePtr master_hugepage = NULL; - virDomainHugePagePtr hugepage = NULL; virDomainNumatuneMemMode mode; const long system_page_size = virGetSystemPageSizeKB(); virDomainMemoryAccess memAccess = mem->access; @@ -3264,6 +3262,13 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps, bool nodeSpecified = virDomainNumatuneNodeSpecified(def->numa, mem->targetNode); unsigned long long pagesize = mem->pagesize; bool needHugepage = !!pagesize; + bool useHugepage = !!pagesize; + + /* The difference between @needHugepage and @useHugepage is that the latter + * is true whenever huge page is defined for the current memory cell. + * Either directly, or transitively via global domain huge pages. The + * former is true whenever "memory-backend-file" must be used to satisfy + * @useHugepage. */ *backendProps = NULL; *backendType = NULL; @@ -3290,6 +3295,8 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps, mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT; if (pagesize == 0) { + virDomainHugePagePtr master_hugepage = NULL; + virDomainHugePagePtr hugepage = NULL; bool thisHugepage = false; /* Find the huge page size we want to use */ @@ -3327,8 +3334,10 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps, hugepage = master_hugepage; } - if (hugepage) + if (hugepage) { pagesize = hugepage->size; + useHugepage = true; + } } if (pagesize == system_page_size) { @@ -3336,17 +3345,18 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps, * of regular system page size, it's as if they * hasn't specified any huge pages at all. */ pagesize = 0; - hugepage = NULL; + needHugepage = false; + useHugepage = false; } if (!(props = virJSONValueNewObject())) return -1; - if (pagesize || mem->nvdimmPath || memAccess || + if (useHugepage || mem->nvdimmPath || memAccess || def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) { *backendType = "memory-backend-file"; - if (pagesize) { + if (useHugepage) { if (qemuGetDomainHupageMemPath(def, cfg, pagesize, &memPath) < 0) goto cleanup; prealloc = true; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.args new file mode 100644 index 0000000000..e4229dce6e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.args @@ -0,0 +1,35 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name fedora \ +-S \ +-M pc-i440fx-2.3 \ +-m size=1048576k,slots=16,maxmem=1099511627776k \ +-smp 2,sockets=2,cores=1,threads=1 \ +-mem-prealloc \ +-mem-path /dev/hugepages2M/libvirt/qemu/-1-fedora \ +-numa node,nodeid=0,cpus=0-1,mem=1024 \ +-object memory-backend-file,id=memdimm0,prealloc=yes,\ +mem-path=/dev/hugepages1G/libvirt/qemu/-1-fedora,size=1073741824,\ +host-nodes=1-3,policy=bind \ +-device pc-dimm,node=0,memdev=memdimm0,id=dimm0,slot=0 \ +-object memory-backend-file,id=memdimm1,prealloc=yes,\ +mem-path=/dev/hugepages2M/libvirt/qemu/-1-fedora,share=no,size=536870912 \ +-device pc-dimm,node=0,memdev=memdimm1,id=dimm1,slot=1 \ +-uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-fedora/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-boot c \ +-usb \ +-drive file=/var/lib/libvirt/images/fedora.qcow2,format=qcow2,if=none,\ +id=drive-virtio-disk0 \ +-device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.xml b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.xml new file mode 100644 index 0000000000..d75cf5afa3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.xml @@ -0,0 +1,65 @@ + + fedora + 63840878-0deb-4095-97e6-fc444d9bc9fa + 1099511627776 + 2621439 + 2621439 + + + + 2 + + hvm + + + + + + + + + + + + + + destroy + restart + restart + + /usr/bin/qemu-system-x86_64 + + + + +
+ + +
+ + + + + +
+ + + + 1-3 + 1048576 + + + 1048576 + 0 + +
+ + + + 524287 + 0 + +
+ + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 655c89bc3e..9938052310 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -844,6 +844,8 @@ mymain(void) QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("hugepages-pages5", QEMU_CAPS_MEM_PATH); DO_TEST("hugepages-pages6", NONE); + DO_TEST("hugepages-pages7", QEMU_CAPS_MEM_PATH, + QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("hugepages-memaccess", QEMU_CAPS_OBJECT_MEMORY_FILE, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_NUMA); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-hugepages-pages7.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hugepages-pages7.xml new file mode 120000 index 0000000000..9aafa6e958 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hugepages-pages7.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-hugepages-pages7.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 0d549adecf..311b713565 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -433,6 +433,7 @@ mymain(void) DO_TEST("hugepages-pages4", NONE); DO_TEST("hugepages-pages5", NONE); DO_TEST("hugepages-pages6", NONE); + DO_TEST("hugepages-pages7", NONE); DO_TEST("hugepages-shared", NONE); DO_TEST("hugepages-memaccess", NONE); DO_TEST("hugepages-memaccess2", NONE);