pyghmi asks for this parameter with xraw_command and catches the completion
code for a bmc that does not have it, and folding aiohmi in renamed that call
to oldraw_command rather than raw_command, so the handler could no longer fire.
Answering the code out of the returned dictionary repaired the crash but kept
the call on the older contract, which is now the only one left in the tree.
Catch it again instead: raw_command puts the completion code on the exception
as ipmicode, and nothing here reads the payload of a reply that carries a code,
which is the one thing catching gives up.
No behaviour change, checked against the previous version over the same fake
session for a good reply, an empty one, 0x80 and 0xC9 with and without a stray
payload, four other completion codes, a timeout, a lost session and a reply
with no data at all: same return value, same exception type, text and ipmicode,
same bytes on the wire.
Completion code 203 on a sensor reading means the sensor is not present, which
is expected of an optional device, but the check for it sat after a call that
raises first, so one absent sensor ended the whole sensor sweep.
_supports_standard_ipv6 read rsp['code'] the same way, so it could only ever
answer True; a platform without the standard parameters raised instead. A
completion code is that platform's answer, while a timeout or a lost session is
not and must not be cached as one.
raw_command's docstring still described itself as the other call it was renamed
from, which is how these checks came to be written against the wrong contract.
get_user_name documents that it answers None when reading a slot fails, but
raw_command raises before the check that would return it, so that branch has
never run and one refused slot aborted the whole user list. Answer None where
the docstring says to, and let get_users drop the blanket except it grew to
work around it.
Only a completion code counts as the bmc answering about the slot. A timeout
carries the fabricated 0xffff from the session layer and a lost session carries
no code at all, and swallowing either of those reports a list truncated at the
point of failure as a complete one.
The top bit of the major revision byte of Get Device ID says the device is
still initialising or taking a firmware update. It was read as part of
the revision, so immediately after a bmc reset nodefirmware reported "BMC
Version: 131.11" for what is 3.11, and settled down only once the bit
cleared.
Mask it as sdr.py already does for the same byte, so the two agree about
the same field.
The oem lookup answers whether it found a handler for the vendor, and that
answer was being stored as whether the lookup had been done at all. On
anything the map does not name, which is every bmc that is not a Lenovo,
the flag stayed false and each oem_init issued another Get Device ID and
built another handler.
Almost everything goes through oem_init, so this is a round trip added to
almost every operation. Where those calls are close together it is far
worse than that: reading the sensor data records asks for the event
constants once per record, so a run of 172 records fired 176 Get Device ID
commands back to back, which was enough to make the bmc stop answering and
the read fail with a timeout. The same sequence now takes 3 commands.
Settling for the generic handler is an answer. The device id cannot
change within a session, so asking again buys nothing, and the handler it
throws away each time is the one holding the sensor names it had cached.
Reading the alert destinations of a bmc that has none reported "Unknown
code 0x80 encountered", which is the fallback text for a completion code
the library has no name for. 0x80 on this parameter is not a failure, it
is the platform saying it does not have alert destinations, and the
redfish side of the same resource has said so in words for a while.
The lan parameter fetch already knew how to tell those apart, so build the
alert reads on it rather than on a raw command that raises on any non-zero
code, and raise UnsupportedFunctionality with something to read. Both the
count and an individual destination are covered, so a platform that offers
one and not the other says the same thing instead of failing differently.
Splitting the completion code handling out of the parameter fetch is what
makes that reuse possible; the interpretation of the payload, and every
answer it gives, is unchanged.
The oem hook for the destination count was passing its byte through ord(),
which raises TypeError on the bytearray it is given. No handler in tree
implements the hook, so it had never been called; hand it the integer.
A bmc that does not implement a lan configuration parameter says so in the
completion code and answers with no data at all. The helper reached
straight into the payload, so such a parameter raised IndexError, and with
it went the whole of nodeconfig over ipmi: the bmc group, the plain, the
detailed, the extended and the advanced reads all ended in "bytearray
index out of range". It also took the attribute enumeration with it, so
the client then rejected names it should have accepted.
The guard that was there caught an exception carrying the completion code,
but oldraw_command reports the code in its response rather than raising on
it, so nothing was ever caught. Read the code from the response instead:
parameter not supported and parameter out of range mean the platform does
not have it, and anything else is a real failure that should say what it
was rather than be mistaken for absence.
The address configuration method was looked up in a table with no regard
for whether it had been read at all, so a bmc that does not report it
would have traded the IndexError for a KeyError. Answer None when it is
absent, as the address above it already does, and name the value when it
is present but unfamiliar.
There is no ipmi command for a bmc hostname, so get_hostname falls back to the
DCMI management controller identifier. A bmc that does not implement the DCMI
group at all rejects that with "invalid command", which was handed to the caller
as if the request had been bad: nodeconfig <node> bmc read seven fields
correctly and then reported "Error: Invalid command", and the api answered 500
Unexpected error.
Route every DCMI request through one helper that turns "invalid command" and
"command disabled or unavailable" into UnsupportedFunctionality, so the
identifier, the asset tag and the hostname all report a platform that cannot do
this rather than a fault. Where the caller asked about a hostname, say that
rather than naming DCMI.
The whole group read still ends in one error line, because an operation a
platform cannot perform and one that failed are the same message to the client.
That is worth separating, but not here.
handle_users iterated get_users with async for while the same call is awaited
a few lines below, so listing the users collection, and creating a user,
raised a TypeError about a coroutine having no __aiter__. list_inventory in
the redfish plugin had the mirror of it, awaiting an async generator.
get_extended_bmc_configuration is called with hideadvanced but the ipmi chain
never accepted it, so the extra and extra_advanced resources raised a
TypeError; thread the argument through instead.
A user slot the bmc refuses to describe no longer takes the whole user list
with it: one MegaRAC slot answered Invalid data field for good after an
account was deleted, which broke every user operation.
Housekeeper dates from when this work was a blocking select loop. Under
asyncio the event loop is already that place, and a thread around it takes a
captured loop reference, a scheduling step before the thread starts, and a
daemon flag, only to sit waiting on a coroutine that never returns with no way
to stop it. A task runs on the right loop by construction and cancels. The
loop is looked up before the coroutine is built, so calling this without one
raises rather than stranding it.
Nothing in this tree used the class. Out of tree callers need
start_housekeeping() instead, and gain the ability to stop it.
Command.eventloop called wait_for_rsp with no timeout. With nothing waiting or
being kept alive there is no deadline to derive one from, so it returns
without suspending and the loop runs flat out, measured at over 100000
iterations in two tenths of a second.
MAX_IDLE gives it something to wait on, as a ceiling rather than a fixed
delay: real deadlines still shorten it and an arriving packet still ends it
early.
Apply ruff's safe autofixes.
The changes are mechanical and behaviour-preserving. Issues fixed:
- F401: remove unused imports.
- F841: drop unused local variables and assignments, including discarded
await/return values, unused "except ... as e" bindings, and unused
"with ... as name" targets.
- F541: remove the f prefix from f-strings that contain no placeholders.
- E711: compare against None with "is"/"is not" instead of "=="/"!=".
- E712: test truthiness directly instead of comparing to True.
- E713: use "x not in y" instead of "not x in y".
- E714: use "is not" instead of "not ... is".
- E731: convert lambdas bound to a name into def statements.
- W291/W293: trim trailing whitespace on touched lines.
Await the channel access raw command, the TSMA remote media settings
requests, and the XCC3 volume creation responses. These calls returned
or unpacked coroutine objects, breaking set_channel_access, TSMA
virtual media attach, and RAID volume creation at runtime.
Drop python3-eventlet from the Ubuntu Noble build Dockerfile. Clean up
remaining greenthread/greenlet terminology in comments across aiohmi
IPMI modules, consoleserver, macmap, and the IPMI plugin. Remove a
commented-out GreenPool reference in macmap.
Jarrod Johnson
Markus Hilger
Jarrod Johnson
Vinícius Ferrão