On 10/01/2015 08:10 AM, Martin Kletzander wrote:
Of course this will be used only in case we don't need the memory-backend-file backend, so it should not fire until later.
So if I'm reading things right... when building the numa string, if it's determine that "-object" and ",memdev=ram-node%zu" will be required, then it's also required to have "-mem-alloc -mem-path ..." (furrowing through the qemu docs was more difficult than I thought)
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 4 ++++ 1 file changed, 4 insertions(+)
Perhaps "outside" the domain of these changes; however, I note that the call to qemuBuildNumaArgStr is from qemuBuildCommandLine as follows: if (virDomainNumaGetNodeCount(def->numa)) { if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0) one of the first things done in qemuBuildNumaArgStr: size_t ncells = virDomainNumaGetNodeCount(def->numa); oh and of course the pesky const long system_page_size = virGetSystemPageSizeKB(); I mentioned previously BTW: Not noted by Coverity, but interestingly later in the code there's: if (ncells) { /* Fortunately, we allow only guest NUMA nodes to be continuous * starting from zero. */ pos = ncells - 1; } Which if I'm reading things correct - cannot happen. Just a note - you don't have to change/fix this, but well if you want to... ACK to what's here as it seems reasonable - perhaps a better reason in the commit message would help John
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 17e5cfd71702..321a5e931350 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8132,6 +8132,10 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, } }
+ if (!needBackend && + qemuBuildMemPathStr(cfg, def, qemuCaps, cmd) < 0) + goto cleanup; + for (i = 0; i < ncells; i++) { VIR_FREE(cpumask); if (!(cpumask = virBitmapFormat(virDomainNumaGetNodeCpumask(def->numa, i))))