From d22dfef7565baa9e0901fcf4712bacd20d3fb67b Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Tue, 21 Sep 2010 10:07:12 +0200
Subject: [PATCH] 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>
---
 lib/cmdlib.py        |  8 +++++---
 lib/rapi/rlib2.py    |  8 +++-----
 scripts/gnt-instance | 14 ++++++--------
 scripts/gnt-os       |  8 +++-----
 tools/burnin         |  8 ++++----
 5 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index e22e4e75f..d1ffec28f 100644
--- 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 eed96400e..0d8367ae4 100644
--- 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 601587591..d921ca0da 100755
--- 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 59cae51d0..9392b3809 100755
--- 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 e5cce5089..3ce415241 100755
--- 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
 
-- 
GitLab