Open Stack

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