Richard W.M. Jones wrote:

beth kon wrote:

[PATCH 1/2] - add capability to access free memory information on each NUMA cell.


+
+/* number of cells in the node will be saved for later use */
+int nbNodeCells;

This should be static I think.

As the code stands now, I don't think so. It is defined in xend_internal.c but used in xen_internal.c

A bigger problem here is that I think the call to xenDaemonNodeGetInfo should happen inside xen_internal.c:xenHypervisorNodeGetCellsFreeMemory, something like this:

xenHypervisorNodeGetCellsFreeMemory (virConnectPtr conn, ....)
{
  static int nb_nodes = -1;

  if (nb_nodes == -1) {
    virNodeInfo info;
    if (xenDaemonNodeGetInfo (conn, &info) == -1) {
      ... (error) ...
    }
    nb_nodes = info->nodes;
  }

  ...
}

Note also the explicit test for == -1

I like this solution. I thought that Daniel wanted it out of the xenHypervisorNodeGetCellsFreeMemory call, and in some init code. Daniel? Do you have an opinion on this?


The rest of the patch looks OK to me.

Rich.



--
Elizabeth Kon (Beth)
IBM Linux Technology Center
Open Hypervisor Team
email: [EMAIL PROTECTED]

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

Reply via email to