Open Stack

On 10/09/2014 10:16 AM, Daniel P. Berrange wrote:
> On Thu, Oct 09, 2014 at 01:45:43PM +0000, Murray, Paul (HP Cloud) wrote:
>> Hi All,
>>
>> The following question relates to this change:
>>
>> https://review.openstack.org/#/c/125091/
>>
>> This change adds a field to the ComputeNode object called "supported_instances". It also adds an object called "SupportedInstance" that has fields called
>> "arch", "hvtype" and "vm_mode".
>>
>> All these names already exist in the nova code, but when put together in
>> these objects they seem a little odd (i.e. supported_instances may be a
>> little misleading, hvtype has no hyphen but vm_mode does). This is where
>> they come from:
>>
>> -          supported_instances is the name of the corresponding field of
>>   the compute_nodes database table. The supported_instances field actually
>>   contains a the list of architecture, hypervisor type and vm_mode
>>   combinations supported by the compute node. It is also the existing
>>   field name used in a dict provided by the virt drivers to report this
>>   list to nova.
>>
>>
>> -          arch, hvtype and vm_mode are the names used by variables
>>    throughout nova that refer to the relevant data obtained contained
>>    in supported_instances.
>>
>> The question is: are these the names we actually want to use?
>
> I'd like to just kill the underscore in 'vm_mode' as I don't think it
> adds any real value.

The value it adds (and that an underscore would add in hvtype -> 
hv_type) is that the name would match the naming style for the vast 
majority of everything else in OpenStack. See, for examples:

  We use vm_state not vmstate. power_state, not powerstate, port_status 
not portstatus, disk_container not diskcontainer, etc.

>> Let me know opinions and I will fix the patch accordingly.
>
> I don't think we want to block your patch on this item. We can just do a
> cleanup afterwards, rather than mixing it in with your patch.

As mentioned in the review, I disagree on this point, since "doing a 
cleanup afterwards" would mean having to increment the 
nova.objects.SupportedInstance model VERSION as soon as it went into 
master. I say let's make the quick change now and avoid having to write 
code like this in the next patchset:

  if client_version <= (1,0):
     # We renamed hvtype to hv_type in 1.1
     self.hv_type = fields.get('hvtype')

Best,
-jay