Re: [PATCH v2 09/16] numatune: add support for per-node memory bindings in private APIs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 11, 2014 at 05:11:02PM +0200, Michal Privoznik wrote:
On 08.07.2014 13:50, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
---
  src/conf/numatune_conf.c | 111 ++++++++++++++++++++++++++++++++++++++++-------
  src/conf/numatune_conf.h |  14 ++++--
  src/libvirt_private.syms |   1 +
  src/lxc/lxc_cgroup.c     |   3 +-
  src/qemu/qemu_cgroup.c   |   2 +-
  src/qemu/qemu_driver.c   |  10 ++---
  src/util/virnuma.c       |   6 +--
  7 files changed, 117 insertions(+), 30 deletions(-)

diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 67fc799..60b6867 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -63,6 +63,18 @@ struct _virDomainNumatune {
  };


+static inline bool
+numa_cell_specified(virDomainNumatunePtr numatune,

Whoa, when I met this function call I thought to myself that this is
some libnuma function. Please name it differently to match our
virSomethingShiny pattern.


I wanted this function to stand out as it is a macro (or static
inline) and I've seen it somewhere else in the code.  I changed it to
virDomainNumatuneNodeSpecified(ewww, gross) and all the calls too
(with proper wrapping as well).

+                    int cellid)
+{
+    if (numatune &&
+        cellid >= 0 &&
+        cellid < numatune->nmem_nodes)
+        return numatune->mem_nodes[cellid].nodeset;
+
+    return false;
+}
+
  static int
  virDomainNumatuneNodeParseXML(virDomainDefPtr def,
                                xmlXPathContextPtr ctxt)
@@ -312,6 +324,7 @@ void
  virDomainNumatuneFree(virDomainNumatunePtr numatune)
  {
      size_t i = 0;
+

This change is spurious. Either move it to 8/16 or drop this one.


Done.

[...]
@@ -469,6 +500,35 @@ virDomainNumatuneSet(virDomainDefPtr def,
      return ret;
  }

+static bool
+virDomainNumatuneNodesEqual(virDomainNumatunePtr n1,
+                            virDomainNumatunePtr n2)
+{
+    size_t i = 0;
+
+    if (n1->nmem_nodes != n2->nmem_nodes)
+        return false;
+
+    for (i = 0; i < n1->nmem_nodes; i++) {
+        virDomainNumatuneNodePtr nd1 = &n1->mem_nodes[i];
+        virDomainNumatuneNodePtr nd2 = &n2->mem_nodes[i];
+
+        if (!nd1->nodeset && !nd2->nodeset)
+            continue;

So if both are missing nodeset, they are considered equal? What if they
differ in mode?


Yes, because !nd->nodeset means there was no <memnode/> with that
cellid.  Therefore mode doesn't make sense (and is not set anyway).

Martin

Attachment: signature.asc
Description: Digital signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]