On Tue, Dec 06, 2011 at 06:52:10PM +0100, Agata Murawska wrote:
> Introduction of instance policy - the required defaults and types are
> provided, along with the syntax check and changes to config writer.
> +# instance specs
> +MEM_COUNT_SPEC = "memory-count"
Hmm, this seems badly chosen (was it like this in the design?). I would
rather expect MEM_SIZE.
> +CPU_COUNT_SPEC = "cpu-count"
> +DISK_COUNT_SPEC = "disk-count"
> +DISK_SIZE_SPEC = "disk-size"
> +NIC_COUNT_SPEC = "nic-count"
> +
> +ISPECS_PARAMETER_TYPES = {
> + MEM_COUNT_SPEC: VTYPE_INT,
> + CPU_COUNT_SPEC: VTYPE_INT,
> + DISK_COUNT_SPEC: VTYPE_INT,
> + DISK_SIZE_SPEC: VTYPE_INT,
> + NIC_COUNT_SPEC: VTYPE_INT,
> + }
also, the _SPEC suffix is somewhat opposite to a more common SPEC_
prefix, but that's fine (at least it has a suffix).
> +IPOLICY_EMPTY = {
> + MIN_ISPECS: {},
> + MAX_ISPECS: {},
> + STD_ISPECS: {},
> + }
This is somewhat dangerous. Especially when you do this:
> + # instance policy added before 2.6
> + if self.ipolicy is None:
> + self.ipolicy = constants.IPOLICY_EMPTY
This should be constants.IPOLICY.copy(), but that's not enough since you
have embedded dicts.
I would suggest a different way, replace IPOLICY_EMPTY with
makeEmptyIPolicy: return dict(...). Similar to how we build defaults in
the lib/ht.py code.
thanks,
iustin
