On 12/04/2014 04:46 AM, Daniel P. Berrange wrote:
On Thu, Dec 04, 2014 at 09:25:00AM +0100, Nikola ─Éipanov wrote:
On 12/04/2014 05:30 AM, Michael Still wrote:
Hi,

so just having read a bunch of the libvirt driver numa code, I have a
concern. At first I thought it was a little thing, but I am starting
to think its more of a big deal...

We use the term "cells" to describe numa cells. However, that term has
a specific meaning in nova, and I worry that overloading the term is
confusing.

(Yes, I know the numa people had it first, but hey).

So, what do people think about trying to move the numa code to use
something like "numa cell" or "numacell" based on context?


Seeing that "node" is also not exactly unambiguous in this space - I am
fine with both with either "numanode" or "numacell" with a slight
preference for "numacell".

A small issue will be renaming it in objects though - as this will
require adding a new field for use in Kilo while still remaining
backwards compatible with Juno, resulting in even more compatibility
code (we already added some for the slightly different data format). The
whole name is quite in context there, but we would use it like:

   for cell in numa_topology.cells:
      # awesome algo here with cell :(

but if we were to rename it just in places where it's used to:

   for numacell in numa_topology.cells:
      # awesome algo here with numacell :)

I think renaming local variables like this is really a solution
in search of a problem. It is pretty blindingly obvious the 'cell'
variable refers to a NUMA cell here, without having to spell it
out as 'numacell'. Likewise I think the object property name is
just fine as 'cell' because the context again makes it obvious
what it is referring to

+1 I don't think this is a problem in the current code.

-jay


_______________________________________________
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Reply via email to