Change behaviour of OpDiagnoseOS w.r.t. 'valid'

This patch changes the behaviour of OpDiagnoseOS with regards to the
'valid' field to be similar to the one for the hidden/blacklisted
fields: unless this field is requested, invalid OSes are filtered out.

The rationale is that, except for the gnt-os info/diagnose, all other
users of this opcode are requesting the valid field just to filter out
invalid OSes, and not for any other use. Thus, changing this behaviour
makes these callers simpler.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index e22e4e7..d1ffec2 100644 (file)
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -3122,8 +3122,9 @@ class LUDiagnoseOS(NoHooksLU):
   REQ_BGL = False
   _HID = "hidden"
   _BLK = "blacklisted"
+  _VLD = "valid"
   _FIELDS_STATIC = utils.FieldSet()
-  _FIELDS_DYNAMIC = utils.FieldSet("name", "valid", "node_status", "variants",
+  _FIELDS_DYNAMIC = utils.FieldSet("name", _VLD, "node_status", "variants",
                                    "parameters", "api_versions", _HID, _BLK)
 
   def CheckArguments(self):
@@ -3215,13 +3216,14 @@ class LUDiagnoseOS(NoHooksLU):
       is_hid = os_name in cluster.hidden_oss
       is_blk = os_name in cluster.blacklisted_oss
       if ((self._HID not in self.op.output_fields and is_hid) or
-          (self._BLK not in self.op.output_fields and is_blk)):
+          (self._BLK not in self.op.output_fields and is_blk) or
+          (self._VLD not in self.op.output_fields and not valid)):
         continue
 
       for field in self.op.output_fields:
         if field == "name":
           val = os_name
-        elif field == "valid":
+        elif field == self._VLD:
           val = valid
         elif field == "node_status":
           # this is just a copy of the dict

diff --git a/lib/rapi/rlib2.py b/lib/rapi/rlib2.py
index eed9640..0d8367a 100644 (file)
--- a/lib/rapi/rlib2.py
+++ b/lib/rapi/rlib2.py
@@ -148,8 +148,7 @@ class R_2_os(baserlib.R_Generic):
 
     """
     cl = baserlib.GetClient()
-    op = opcodes.OpDiagnoseOS(output_fields=["name", "valid", "variants"],
-                              names=[])
+    op = opcodes.OpDiagnoseOS(output_fields=["name", "variants"], names=[])
     job_id = baserlib.SubmitJob([op], cl)
     # we use custom feedback function, instead of print we log the status
     result = cli.PollJob(job_id, cl, feedback_fn=baserlib.FeedbackFn)
@@ -159,9 +158,8 @@ class R_2_os(baserlib.R_Generic):
       raise http.HttpBadGateway(message="Can't get OS list")
 
     os_names = []
-    for (name, valid, variants) in diagnose_data:
-      if valid:
-        os_names.extend(cli.CalculateOSNames(name, variants))
+    for (name, variants) in diagnose_data:
+      os_names.extend(cli.CalculateOSNames(name, variants))
 
     return os_names
 

diff --git a/scripts/gnt-instance b/scripts/gnt-instance
index 6015875..d921ca0 100755 (executable)
--- a/scripts/gnt-instance
+++ b/scripts/gnt-instance
@@ -524,8 +524,7 @@ def ReinstallInstance(opts, args):
 
   # second, if requested, ask for an OS
   if opts.select_os is True:
-    op = opcodes.OpDiagnoseOS(output_fields=["name", "valid", "variants"],
-                              names=[])
+    op = opcodes.OpDiagnoseOS(output_fields=["name", "variants"], names=[])
     result = SubmitOpCode(op, opts=opts)
 
     if not result:
@@ -535,12 +534,11 @@ def ReinstallInstance(opts, args):
     ToStdout("Available OS templates:")
     number = 0
     choices = []
-    for (name, valid, variants) in result:
-      if valid:
-        for entry in CalculateOSNames(name, variants):
-          ToStdout("%3s: %s", number, entry)
-          choices.append(("%s" % number, entry, entry))
-          number += 1
+    for (name, variants) in result:
+      for entry in CalculateOSNames(name, variants):
+        ToStdout("%3s: %s", number, entry)
+        choices.append(("%s" % number, entry, entry))
+        number += 1
 
     choices.append(('x', 'exit', 'Exit gnt-instance reinstall'))
     selected = AskUser("Enter OS template number (or x to abort):",

diff --git a/scripts/gnt-os b/scripts/gnt-os
index 59cae51..9392b38 100755 (executable)
--- a/scripts/gnt-os
+++ b/scripts/gnt-os
@@ -44,8 +44,7 @@ def ListOS(opts, args):
   @return: the desired exit code
 
   """
-  op = opcodes.OpDiagnoseOS(output_fields=["name", "valid", "variants"],
-                            names=[])
+  op = opcodes.OpDiagnoseOS(output_fields=["name", "variants"], names=[])
   result = SubmitOpCode(op, opts=opts)
 
   if not result:
@@ -58,9 +57,8 @@ def ListOS(opts, args):
     headers = None
 
   os_names = []
-  for (name, valid, variants) in result:
-    if valid:
-      os_names.extend([[n] for n in CalculateOSNames(name, variants)])
+  for (name, variants) in result:
+    os_names.extend([[n] for n in CalculateOSNames(name, variants)])
 
   data = GenerateTable(separator=None, headers=headers, fields=["name"],
                        data=os_names, units=None)

diff --git a/tools/burnin b/tools/burnin
index e5cce50..3ce4152 100755 (executable)
--- a/tools/burnin
+++ b/tools/burnin
@@ -510,16 +510,16 @@ class Burner(object):
       Err(msg, exit_code=err_code)
     self.nodes = [data[0] for data in result if not (data[1] or data[2])]
 
-    op_diagnose = opcodes.OpDiagnoseOS(output_fields=["name", "valid",
-                                                      "variants"], names=[])
+    op_diagnose = opcodes.OpDiagnoseOS(output_fields=["name", "variants"],
+                                       names=[])
     result = self.ExecOp(True, op_diagnose)
 
     if not result:
       Err("Can't get the OS list")
 
     found = False
-    for (name, valid, variants) in result:
-      if valid and self.opts.os in cli.CalculateOSNames(name, variants):
+    for (name, variants) in result:
+      if self.opts.os in cli.CalculateOSNames(name, variants):
         found = True
         break