From b985624a4bc1de4e59a3b23fc42ea7e534a0d1dc Mon Sep 17 00:00:00 2001 From: Jarrod Johnson Date: Thu, 25 Jul 2013 14:19:09 -0400 Subject: [PATCH] Remove callback arguments from most functions. After some contemplation, I realized that given the underlying behavior of 'wait_for_rsp', that synchronous and asynchronous calls play well with each other. The pattern of considering an 'onlogon' function as running in a sort of greenthread works well and leads to much more comprehensible code for the average person. While removing those arguments, the module contents were reorganized to serve as an example of how more straightforward code written in this fashion may be. Change-Id: I9c83660e4d3c68ad8ae0c016a3377b2a0e7ee6ec --- ipmi/command.py | 263 +++++++++++++--------------------------- ipmi/private/session.py | 53 +++----- ipmictl.py | 48 +++++--- 3 files changed, 128 insertions(+), 236 deletions(-) diff --git a/ipmi/command.py b/ipmi/command.py index 1fa289e4..7bd4d939 100644 --- a/ipmi/command.py +++ b/ipmi/command.py @@ -49,14 +49,6 @@ power_states = { } -def _raiseorcall(callback, response, args=None): - if callback is None: - if 'error' in response: - raise Exception(response['error']) - else: - session.call_with_optional_args(callback, args) - - class Command(object): """Send IPMI commands to BMCs. @@ -74,18 +66,37 @@ class Command(object): :param bmc: hostname or ip address of the BMC :param userid: username to use to connect :param password: password to connect to the BMC + :param onlogon: function to run when logon completes in an asynchronous + fashion. This will result in a greenthread behavior. :param kg: Optional parameter to use if BMC has a particular Kg configured """ - def __init__(self, bmc, userid, password, kg=None): + def __init__(self, bmc, userid, password, onlogon=None, kg=None): # TODO(jbjohnso): accept tuples and lists of each parameter for mass # operations without pushing the async complexities up the stack - self.ipmi_session = session.Session(bmc=bmc, - userid=userid, - password=password, - kg=kg) + self.onlogon = onlogon + self.bmc = bmc + if onlogon is not None: + self.ipmi_session = session.Session(bmc=bmc, + userid=userid, + password=password, + onlogon=self.logged, + kg=kg) + else: + self.ipmi_session = session.Session(bmc=bmc, + userid=userid, + password=password, + kg=kg) - def get_bootdev(self, callback=None, callback_args=None): + def logged(self, response): + self.onlogon(response, self) + + @classmethod + def eventloop(cls): + while (session.Session.wait_for_rsp()): + pass + + def get_bootdev(self): """Get current boot device override information. Provides the current requested boot device. Be aware that not all IPMI @@ -93,29 +104,32 @@ class Command(object): BIOS or UEFI fail to honor it. This is usually only applicable to the next reboot. - :param callback: optional callback - :param callback_args: optional arguments to callback - :returns: dict or True -- If callback is not provided, the response - will be provided in the return + :returns: dict --The response will be provided in the return as a dict """ - self.commandcallback = callback - self.commandcallbackargs = callback_args - self.ipmi_session.raw_command(netfn=0, - command=9, - data=(5, 0, 0), - callback=self._got_bootdev) - return self._waitifsync() + response = self.ipmi_session.raw_command(netfn=0, + command=9, + data=(5, 0, 0)) + # interpret response per 'get system boot options' + if 'error' in response: + return response + # this should only be invoked for get system boot option complying to + # ipmi spec and targeting the 'boot flags' parameter + assert (response['command'] == 9 and + response['netfn'] == 1 and + response['data'][0] == 1 and + (response['data'][1] & 0b1111111) == 5) + if (response['data'][1] & 0b10000000 or + not response['data'][2] & 0b10000000): + return {'bootdev': 'default'} + else: # will consult data2 of the boot flags parameter for the data + bootnum = (response['data'][3] & 0b111100) >> 2 + bootdev = boot_devices[bootnum] + if (bootdev): + return {'bootdev': bootdev} + else: + return {'bootdev': bootnum} - def _waitifsync(self): - self.requestpending = True - if self.commandcallback is None: - while self.requestpending: - session.Session.wait_for_rsp() - return self.lastresponse - return True - - def set_power(self, powerstate, wait=False, callback=None, - callback_args=None): + def set_power(self, powerstate, wait=False): """Request power state change :param powerstate: @@ -127,79 +141,39 @@ class Command(object): * reset -- Request system reset without waiting for OS * boot -- If system is off, then 'on', else 'reset' - :param wait: If True, do not return or callback until system actually - completes requested state change - :param callback: optional callback - :param callback_args: optional arguments to callback - :returns: dict or True -- If callback is not provided, the response + :param wait: If True, do not return until system actually completes + requested state change + :returns: dict -- A dict describing the response retrieved """ - self.commandcallback = callback - self.commandcallbackargs = callback_args if powerstate not in power_states: - _raiseorcall(self.commandcallback, - {'error': - "Unknown power state %s requested" % powerstate}, - self.commandcallbackargs) + raise Exception("Unknown power state %s requested" % powerstate) self.newpowerstate = powerstate - self.wait_for_power = wait - self.ipmi_session.raw_command(netfn=0, - command=1, - callback=self._set_power_with_chassisinfo - ) - return self._waitifsync() - - def _set_power_with_chassisinfo(self, response): + response = self.ipmi_session.raw_command(netfn=0, command=1) if 'error' in response: - _raiseorcall( - self.commandcallback, response, self.commandcallbackargs) - return + return response self.powerstate = 'on' if (response['data'][0] & 1) else 'off' if self.newpowerstate == 'boot': self.newpowerstate = 'on' if self.powerstate == 'off' else 'reset' - self.ipmi_session.raw_command(netfn=0, - command=2, - data=[power_states[self.newpowerstate]], - callback=self._power_set) - - def _power_set(self, response): + response = self.ipmi_session.raw_command( + netfn=0, command=2, data=[power_states[self.newpowerstate]]) if 'error' in response: - _raiseorcall( - self.commandcallback, response, self.commandcallbackargs) - return + return response self.lastresponse = {'pendingpowerstate': self.newpowerstate} - if (self.wait_for_power and + if (wait and self.newpowerstate in ('on', 'off', 'shutdown', 'softoff')): if self.newpowerstate in ('softoff', 'shutdown'): self.waitpowerstate = 'off' else: self.waitpowerstate = self.newpowerstate - self.ipmi_session.raw_command(netfn=0, - command=1, - callback=self._power_wait) + currpowerstate = None + while currpowerstate != self.waitpowerstate: + response = self.ipmi_session.raw_command(netfn=0, command=1) + if 'error' in response: + return response + currpowerstate = 'on' if (response['data'][0] & 1) else 'off' + return {'powerstate': currpowerstate} else: - self.requestpending = False - if self.commandcallback: - session.call_with_optional_args(self.commandcallback, - self.lastresponse, - self.commandcallbackargs) - - def _power_wait(self, response): - if 'error' in response: - _raiseorcall( - self.commandcallback, response, self.commandcallbackargs) - return - self.powerstate = 'on' if (response['data'][0] & 1) else 'off' - if self.powerstate == self.waitpowerstate: - self.requestpending = False - self.lastresponse = {'powerstate': self.powerstate} - if self.commandcallback: - session.call_with_optional_args(self.commandcallback, - self.lastresponse, - self.commandcallbackargs) - return - self.ipmi_session.raw_command(netfn=0, - command=1, - callback=self._power_wait) + return self.lastresponse def set_bootdev(self, bootdev, @@ -232,10 +206,7 @@ class Command(object): self.commandcallback = callback self.commandcallbackargs = callback_args if bootdev not in boot_devices: - _raiseorcall(self.commandcallback, - {'error': "Unknown bootdevice %s requested" % - bootdev}, - self.commandcallbackargs) + return {'error': "Unknown bootdevice %s requested" % bootdev} self.bootdev = boot_devices[bootdev] self.persistboot = persist self.uefiboot = uefiboot @@ -243,21 +214,11 @@ class Command(object): # then move on to set chassis capabilities self.requestpending = True # Set System Boot Options is netfn=0, command=8, data - self.ipmi_session.raw_command(netfn=0, - command=8, data=(3, 8), - callback=self._bootdev_timer_disabled) - if callback is None: - while self.requestpending: - session.Session.wait_for_rsp() - return self.lastresponse - - def _bootdev_timer_disabled(self, response): - self.requestpending = False + response = self.ipmi_session.raw_command(netfn=0, command=8, + data=(3, 8)) self.lastresponse = response if 'error' in response: - _raiseorcall( - self.commandcallback, response, self.commandcallbackargs) - return + return response bootflags = 0x80 if self.uefiboot: bootflags = bootflags | 1 << 5 @@ -266,18 +227,12 @@ class Command(object): if self.bootdev == 0: bootflags = 0 data = (5, bootflags, self.bootdev, 0, 0, 0) - self.ipmi_session.raw_command(netfn=0, - command=8, - data=data, - callback=self.commandcallback, - callback_args=self.commandcallbackargs) + response = self.ipmi_session.raw_command(netfn=0, command=8, data=data) + if 'error' in response: + return response + return {'bootdev': bootdev} - def raw_command(self, - netfn, - command, - data=(), - callback=None, - callback_args=None): + def raw_command(self, netfn, command, data=()): """Send raw ipmi command to BMC This allows arbitrary IPMI bytes to be issued. This is commonly used @@ -288,74 +243,22 @@ class Command(object): :param netfn: Net function number :param command: Command value :param data: Command data as a tuple or list - :param callback: optional callback - :param callback_args: optional arguments to callback - :returns: dict or True -- If callback is not provided, the response + :returns: dict -- The response from IPMI device """ - response = self.ipmi_session.raw_command(netfn=0, - command=1, - callback=callback, - callback_args=callback_args) - if response: # this means there was no callback - if 'error' in response: - raise Exception(response['error']) - return response + return self.ipmi_session.raw_command(netfn=netfn, command=command) - def _got_bootdev(self, response): - # interpret response per 'get system boot options' - self.requestpending = False - if 'error' in response: - _raiseorcall( - self.commandcallback, response, self.commandcallbackargs) - return - # this should only be invoked for get system boot option complying to - # ipmi spec and targeting the 'boot flags' parameter - assert (response['command'] == 9 and - response['netfn'] == 1 and - response['data'][0] == 1 and - (response['data'][1] & 0b1111111) == 5) - if (response['data'][1] & 0b10000000 or - not response['data'][2] & 0b10000000): - self.lastresponse = {'bootdev': 'default'} - else: # will consult data2 of the boot flags parameter for the data - bootnum = (response['data'][3] & 0b111100) >> 2 - bootdev = boot_devices[bootnum] - if (bootdev): - self.lastresponse = {'bootdev': bootdev} - else: - self.lastresponse = {'bootdev': bootnum} - if self.commandcallback: - session.call_with_optional_args(self.commandcallback, - self.lastresponse, - self.commandcallbackargs) - - def get_power(self, callback=None, callback_args=None): + def get_power(self): """Get current power state of the managed system The response, if successful, should contain 'powerstate' key and either 'on' or 'off' to indicate current state. - :param callback: optional callback - :param callback_args: optional arguments to callback - :returns: dict or True -- If callback is not provided, the response + :returns: dict -- {'powerstate': value} """ - self.commandcallback = callback - self.commandcallbackargs = callback_args - self.ipmi_session.raw_command(netfn=0, - command=1, - callback=self._got_power) - return self._waitifsync() - - def _got_power(self, response): - self.requestpending = False + response = self.ipmi_session.raw_command(netfn=0, command=1) if 'error' in response: - _raiseorcall( - self.commandcallback, response, self.commandcallbackargs) + raise Exception(response['error']) return assert(response['command'] == 1 and response['netfn'] == 1) self.powerstate = 'on' if (response['data'][0] & 1) else 'off' - self.lastresponse = {'powerstate': self.powerstate} - if self.commandcallback: - session.call_with_optional_args(self.commandcallback, - self.lastresponse, - self.commandcallbackargs) + return {'powerstate': self.powerstate} diff --git a/ipmi/private/session.py b/ipmi/private/session.py index c2332733..a688d076 100644 --- a/ipmi/private/session.py +++ b/ipmi/private/session.py @@ -194,8 +194,7 @@ class Session: password, port=623, kg=None, - onlogon=None, - onlogonargs=None): + onlogon=None): self.lastpayload = None self.bmc = bmc self.userid = userid @@ -207,7 +206,6 @@ class Session: else: self.kg = password self.port = port - self.onlogonargs = onlogonargs if (onlogon is None): self.async = False self.onlogon = self._sync_login @@ -464,7 +462,7 @@ class Session: def _got_channel_auth_cap(self, response): if 'error' in response: - call_with_optional_args(self.onlogon, response, self.onlogonargs) + self.onlogon(response) return if response['code'] == 0xcc and self.ipmi15only is not None: # tried ipmi 2.0 against a 1.5 which should work, but some bmcs @@ -474,9 +472,7 @@ class Session: mysuffix = " while trying to get channel authentication capabalities" errstr = get_ipmi_error(response, suffix=mysuffix) if errstr: - call_with_optional_args(self.onlogon, - {'error': errstr}, - self.onlogonargs) + self.onlogon({'error': errstr}) return data = response['data'] self.currentchannel = data[0] @@ -484,11 +480,9 @@ class Session: self.ipmiversion = 2.0 if self.ipmiversion == 1.5: if not (data[1] & 0b100): - call_with_optional_args( - self.onlogon, + self.onlogon( {'error': - "MD5 required but not enabled/available on target BMC"}, - self.onlogonargs) + "MD5 required but not enabled/available on target BMC"}) return self._get_session_challenge() elif self.ipmiversion == 2.0: @@ -498,9 +492,7 @@ class Session: errstr = get_ipmi_error(response, suffix=" while getting session challenge") if errstr: - call_with_optional_args(self.onlogon, - {'error': errstr}, - self.onlogonargs) + self.onlogon({'error': errstr}) return data = response['data'] self.sessionid = struct.unpack("= 5: args = sys.argv[4:] -ipmicmd = command.Command(bmc=bmc, userid=userid, password=password) -if cmmand == 'power': - if args: - print ipmicmd.set_power(args[0], wait=True) - else: - print ipmicmd.get_power() -elif cmmand == 'bootdev': - if args: - print ipmicmd.set_bootdev(args[0]) - else: - print ipmicmd.get_bootdev() -elif cmmand == 'raw': - netfn = args[0] - cmmand = args[1] - data = args[2:] - print ipmicmd.raw_command(netfn=netfn, command=cmmand, data=data) + +ipmicmd = None + + +def docommand(result, ipmisession): + cmmand = sys.argv[3] + print "Logged into %s" % ipmisession.bmc + if 'error' in result: + print result['error'] + return + if cmmand == 'power': + if args: + print ipmisession.set_power(args[0], wait=True) + else: + value = ipmisession.get_power() + print "%s: %s" % (ipmisession.bmc, value['powerstate']) + elif cmmand == 'bootdev': + if args: + print ipmisession.set_bootdev(args[0]) + else: + print ipmisession.get_bootdev() + elif cmmand == 'raw': + print ipmisession.raw_command(netfn=args[0], + command=args[1], data=args[2:]) + +bmcs = string.split(bmc, ",") +for bmc in bmcs: + ipmicmd = command.Command(bmc=bmc, userid=userid, password=password, + onlogon=docommand) +ipmicmd.eventloop()