Thanks Bryn.
Will come up with another version.

-Mukesh


On Thursday 01 December 2016 06:25 PM, Bryn M. Reeves wrote:
On Thu, Dec 01, 2016 at 06:16:14PM +0530, Mukesh Ojha wrote:
+import subprocess
  from sos.plugins import Plugin, RedHatPlugin, DebianPlugin
Please don't invent new ways to call programs using subprocess - use the
existing wrappers in the base Plugin class and the utilities module.

For this use you probably want Plugin.get_command_output() (just test
the 'status' value stored in the returned dictionary).

+        p = subprocess.Popen('ipmitool -I usb mc info', shell=True,
There's no need for shell=True here and using a shell can introduce
unwanted security and behavioural consequences - this is a main reason
for using the provided interfaces in plugin code.

+        isusbIntfEnable = p.returncode
We don't use camelCase for local identifiers in sos - suggest
'have_usb', 'usb_available' etc. instead.

+
+        if not isusbIntfEnable:
+            cmd = "ipmitool -I usb"
+        else:
+            cmd = "ipmitool"
+
          self.add_cmd_output([
-            "ipmitool sel info",
-            "ipmitool sel list",
-            "ipmitool sensor list",
-            "ipmitool chassis status",
-            "ipmitool fru print",
-            "ipmitool mc info",
-            "ipmitool sdr info"
+            "%s sel info" % cmd,
+            "%s sel list" % cmd,
+            "%s sensor list" % cmd,
+            "%s chassis status" % cmd,
+            "%s fru print" % cmd,
+            "%s mc info" % cmd,
+            "%s sdr info" % cmd
Ack.

Should be no problem merging this with those few items taken care of.

Regards,
Bryn.


_______________________________________________
sos-devel mailing list
sos-devel@redhat.com
https://www.redhat.com/mailman/listinfo/sos-devel

Reply via email to