[sos-devel] [PATCH] plugin/ipmitool : Add usb interface if available instead of using default interface
Bryn M. Reeves
bmr at redhat.com
Thu Dec 1 12:55:01 UTC 2016
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.
More information about the sos-devel
mailing list