Question: does 'SF_NODE' belong in 'VALID_STORAGE_FIELDS' ?
Let's take a look at two files referring to 'VALID_STORAGE_FIELDS' and
'SF_NODE' and some code snippets (I omitted some irrelevant files).
/lib/cmdlib/node.py:
Someone knows '_CheckOutputFields' well can perhaps enlighten why
'SF_NODE' is being passed as a static field instead of a dynamic
field. I looked inside '_CheckOutputFields' and static and dynamic
fields are merged together and compared against selected. I don't
see a reason why they should be passed separate. And
'_FIELDS_STATIC' is not used anywhere else and it is not part of any
of the superclasses. I put the code here so you can see for
yourselves.
_FIELDS_STATIC = utils.FieldSet(constants.SF_NODE)
...
def CheckArguments(self):
_CheckOutputFields(static=self._FIELDS_STATIC,
dynamic=utils.FieldSet(*constants.VALID_STORAGE_FIELDS),
selected=self.op.output_fields)
./qa/qa_node.py
So this one is just inconsistent: you can see that we are creating a
list containing 'VALID_STORAGE_FIELDS' plus 'SF_NODE' and 'SF_TYPE'.
This suggests these two last contants should not be a part of
'VALID_STORAGE_TYPES'. However, 'SF_TYPE' IS part of the
'VALID_STORAGE_FIELDS' and 'SF_NODE' is NOT, so the final list is
going to contain 'SF_TYPE' twice.
cmd = ["gnt-node", "list-storage", "--storage-type", storage_type,
"--output=%s" % ",".join(list(constants.VALID_STORAGE_FIELDS) +
[constants.SF_NODE, constants.SF_TYPE])]
So at this point, we can either (1) fix the QA node test to by
removing 'SF_TYPE' from the list; (2) or we can add 'SF_NODE' to
'VALID_STORAGE_TYPES', fix the LU by eliminating the '_STATIC_FIELDS',
and removing both 'SF_NODE' and 'SF_TYPE' from the QA node test.
If we go with number 2, we can also simplify '_CheckOutputArguments'
because it is only used in two places: in this one and another one
that is very similar. And we can stop this thing with 'static' and
'dynamic' fields and simply call it 'fields'.
Do you guys see any other problem I might have overlooked ?
Whay do you think ?
Jose
On Wed, Aug 21, 2013 at 02:54:44PM +0200, Guido Trotter wrote:
> On Tue, Aug 20, 2013 at 9:24 PM, <[email protected]> wrote:
> > Hi,
> >
> > In lib/constants.py, is 'VALID_STORAGE_FIELDS' missing 'SF_NODE' ?
> >
>
> Possibly, indeed. Can you investigate where it's used and report on
> whether it should indeed be added, or just dropped, perhaps?
>
> Thanks,
>
> Guido
