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
_______________________________________________
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
