Re: [libvirt] [PATCH] qemu: make advice from numad available when building commandline

2014-10-31 Thread Martin Kletzander

On Thu, Oct 30, 2014 at 06:21:28PM +0100, Michal Privoznik wrote:

On 30.10.2014 07:40, Martin Kletzander wrote:

Particularly in qemuBuildNumaArgStr(), there was a need for the advice
due to memory backing, which needs to know the nodeset it will be pinned
to.  With newer qemu this caused the following error when starting
domain:

   error: internal error: Advice from numad is needed in case of
   automatic numa placement

even when starting perfectly valid domain, e.g.:

   ...
   vcpu placement='auto'4/vcpu
   numatune
 memory mode='strict' placement='auto'/
   /numatune
   cpu
 numa
   cell id='0' cpus='0' memory='524288'/
   cell id='1' cpus='1' memory='524288'/
 /numa
   /cpu
   ...

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138545

Signed-off-by: Martin Kletzander mklet...@redhat.com


Would it be possible to add a test case? Maybe you'd need to mock some
functions but we already have qemuxml2argvmock.c. Otherwise the code
looks okay and with test case I'd ACK it for the freeze.



Well, I can add a xm2argv test that formats -object memory-ram with
some nodeset, but given that the nodeset needs to be passed as a
parameter of the function that gets called in the test itself, it
doesn't make much of a sense, does it.  No need to have it in the
release, I'd push it after release anyway, so thanks for the review.

Martin


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] qemu: make advice from numad available when building commandline

2014-10-30 Thread Martin Kletzander
Particularly in qemuBuildNumaArgStr(), there was a need for the advice
due to memory backing, which needs to know the nodeset it will be pinned
to.  With newer qemu this caused the following error when starting
domain:

  error: internal error: Advice from numad is needed in case of
  automatic numa placement

even when starting perfectly valid domain, e.g.:

  ...
  vcpu placement='auto'4/vcpu
  numatune
memory mode='strict' placement='auto'/
  /numatune
  cpu
numa
  cell id='0' cpus='0' memory='524288'/
  cell id='1' cpus='1' memory='524288'/
/numa
  /cpu
  ...

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138545

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/qemu/qemu_command.c  | 10 ++
 src/qemu/qemu_command.h  |  3 ++-
 src/qemu/qemu_driver.c   |  3 ++-
 src/qemu/qemu_process.c  |  3 ++-
 tests/qemuxml2argvtest.c |  3 ++-
 tests/qemuxmlnstest.c|  2 +-
 6 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2e5af4f..207e2b0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6636,7 +6636,8 @@ static int
 qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 const virDomainDef *def,
 virCommandPtr cmd,
-virQEMUCapsPtr qemuCaps)
+virQEMUCapsPtr qemuCaps,
+virBitmapPtr nodeset)
 {
 size_t i, j;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -6796,7 +6797,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,

 virBufferAsprintf(buf, ,size=%dM,id=ram-node%zu, cellmem, i);

-if (virDomainNumatuneMaybeFormatNodeset(def-numatune, NULL,
+if (virDomainNumatuneMaybeFormatNodeset(def-numatune, nodeset,
 nodemask, i)  0)
 goto cleanup;

@@ -7764,7 +7765,8 @@ qemuBuildCommandLine(virConnectPtr conn,
  virNetDevVPortProfileOp vmop,
  qemuBuildCommandLineCallbacksPtr callbacks,
  bool standalone,
- bool enableFips)
+ bool enableFips,
+ virBitmapPtr nodeset)
 {
 virErrorPtr originalError = NULL;
 size_t i, j;
@@ -7992,7 +7994,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 }

 if (def-cpu  def-cpu-ncells)
-if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps)  0)
+if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset)  0)
 goto error;

 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_UUID))
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index aa40c9e..f7d3c2d 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -79,7 +79,8 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
virNetDevVPortProfileOp vmop,
qemuBuildCommandLineCallbacksPtr callbacks,
bool forXMLToArgv,
-   bool enableFips)
+   bool enableFips,
+   virBitmapPtr nodeset)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(11);

 /* Generate '-device' string for chardev device */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 373daab..5ac638a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6472,7 +6472,8 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr 
conn,
  VIR_NETDEV_VPORT_PROFILE_OP_NO_OP,
  buildCommandLineCallbacks,
  true,
- qemuCheckFips(
+ qemuCheckFips(),
+ NULL)))
 goto cleanup;

 ret = virCommandToString(cmd);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ef258cf..6cbd608 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4362,7 +4362,8 @@ int qemuProcessStart(virConnectPtr conn,
  priv-monJSON, priv-qemuCaps,
  migrateFrom, stdin_fd, snapshot, vmop,
  buildCommandLineCallbacks, false,
- qemuCheckFips(
+ qemuCheckFips(),
+ nodemask)))
 goto cleanup;

 /* now that we know it is about to start call the hook if present */
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0e9fab9..9b8e3e2 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -362,7 +362,8 @@ static int testCompareXMLToArgvFiles(const char *xml,
  migrateFrom, migrateFd, NULL,
  

Re: [libvirt] [PATCH] qemu: make advice from numad available when building commandline

2014-10-30 Thread Michal Privoznik

On 30.10.2014 07:40, Martin Kletzander wrote:

Particularly in qemuBuildNumaArgStr(), there was a need for the advice
due to memory backing, which needs to know the nodeset it will be pinned
to.  With newer qemu this caused the following error when starting
domain:

   error: internal error: Advice from numad is needed in case of
   automatic numa placement

even when starting perfectly valid domain, e.g.:

   ...
   vcpu placement='auto'4/vcpu
   numatune
 memory mode='strict' placement='auto'/
   /numatune
   cpu
 numa
   cell id='0' cpus='0' memory='524288'/
   cell id='1' cpus='1' memory='524288'/
 /numa
   /cpu
   ...

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138545

Signed-off-by: Martin Kletzander mklet...@redhat.com


Would it be possible to add a test case? Maybe you'd need to mock some 
functions but we already have qemuxml2argvmock.c. Otherwise the code 
looks okay and with test case I'd ACK it for the freeze.


Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list