From 42ae44d5bf6729a5b0d5af91e17e50f62268ea69 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Mon, 10 Aug 2026 04:39:28 +0200 Subject: [PATCH] Finish the server side SOL and cleanup paths The console awaits its output handler, but both sample BMCs supplied a plain function, and both dropped the send_data coroutine. virshbmc additionally receives its stream callback on a libvirt thread, so the send goes through run_coroutine_threadsafe against the loop captured at activation, called asyncloop because the class already has a loop method it uses as a thread target. ServerConsole asked the session layer to retry, which a ServerSession cannot do: it never runs Session.__init__, so it has no timeout, and its _timedout does nothing. IpmiServer.logout was synchronous and an argument short while _cleanup awaits logout(False). The boot options handler answered, then read an unbound name and answered again with 0xff. --- confluent_server/aiohmi/cmd/fakebmc.py | 4 ++-- confluent_server/aiohmi/cmd/virshbmc.py | 12 +++++++++--- confluent_server/aiohmi/ipmi/bmc.py | 4 +++- confluent_server/aiohmi/ipmi/console.py | 5 ++++- .../aiohmi/ipmi/private/serversession.py | 2 +- 5 files changed, 19 insertions(+), 8 deletions(-) diff --git a/confluent_server/aiohmi/cmd/fakebmc.py b/confluent_server/aiohmi/cmd/fakebmc.py index 5a222241..3ccbabf8 100755 --- a/confluent_server/aiohmi/cmd/fakebmc.py +++ b/confluent_server/aiohmi/cmd/fakebmc.py @@ -73,10 +73,10 @@ class FakeBmc(bmc.Bmc): def is_active(self): return self.powerstate == 'on' - def iohandler(self, data): + async def iohandler(self, data): print(data) if self.sol: - self.sol.send_data(data) + await self.sol.send_data(data) def main(): diff --git a/confluent_server/aiohmi/cmd/virshbmc.py b/confluent_server/aiohmi/cmd/virshbmc.py index d5e614e1..359ff97a 100755 --- a/confluent_server/aiohmi/cmd/virshbmc.py +++ b/confluent_server/aiohmi/cmd/virshbmc.py @@ -39,8 +39,11 @@ def stream_callback(stream, events, console): data = console.stream.recv(1024) except Exception: return - if console.sol: - console.sol.send_data(data) + if console.sol and console.asyncloop: + # libvirt calls this from its own event thread, and asyncio objects + # are not thread safe, so hand the send to the loop that owns them. + asyncio.run_coroutine_threadsafe(console.sol.send_data(data), + console.asyncloop) class LibvirtBmc(bmc.Bmc): @@ -54,6 +57,7 @@ class LibvirtBmc(bmc.Bmc): self.domain = self.conn.lookupByName(domain) self.state = self.domain.state(0) self.stream = None + self.asyncloop = None self.run_console = False self.conn.domainEventRegister(lifecycle_callback, self) self.sol_thread = None @@ -108,6 +112,8 @@ class LibvirtBmc(bmc.Bmc): return self.run_console async def activate_payload(self, request, session): + # captured for stream_callback, which runs on a libvirt thread + self.asyncloop = asyncio.get_running_loop() await super(LibvirtBmc, self).activate_payload(request, session) self.run_console = True self.sol_thread = threading.Thread(target=self.loop) @@ -118,7 +124,7 @@ class LibvirtBmc(bmc.Bmc): self.sol_thread.join() await super(LibvirtBmc, self).deactivate_payload(request, session) - def iohandler(self, data): + async def iohandler(self, data): if self.stream: self.stream.send(data) diff --git a/confluent_server/aiohmi/ipmi/bmc.py b/confluent_server/aiohmi/ipmi/bmc.py index 1a35bfa3..7ef57226 100644 --- a/confluent_server/aiohmi/ipmi/bmc.py +++ b/confluent_server/aiohmi/ipmi/bmc.py @@ -132,7 +132,9 @@ class Bmc(serversession.IpmiServer): try: bootdevice = self.get_boot_device() except NotImplementedError: - await session.send_ipmi_response(data=[1, 5, 0, 0, 0, 0, 0]) + # nothing more to say, and bootdevice below would be unbound + return await session.send_ipmi_response( + data=[1, 5, 0, 0, 0, 0, 0]) if (type(bootdevice) != int and bootdevice in ipmicommand.boot_devices): bootdevice = ipmicommand.boot_devices[bootdevice] diff --git a/confluent_server/aiohmi/ipmi/console.py b/confluent_server/aiohmi/ipmi/console.py index d204e126..c68ca774 100644 --- a/confluent_server/aiohmi/ipmi/console.py +++ b/confluent_server/aiohmi/ipmi/console.py @@ -541,9 +541,12 @@ class ServerConsole(Console): needskeepalive=False): while not (self.connected or self.broken): await session.Session.wait_for_rsp(timeout=10) + # retry is not passed on: a ServerSession has no retry timer, it + # never runs Session.__init__ and its _timedout does nothing, so + # asking for one only reaches for a timeout attribute it lacks. await self.ipmi_session.send_payload(payload, payload_type=payload_type, - retry=retry, + retry=False, needskeepalive=needskeepalive) def close(self): diff --git a/confluent_server/aiohmi/ipmi/private/serversession.py b/confluent_server/aiohmi/ipmi/private/serversession.py index a3288ec2..934ec0ca 100644 --- a/confluent_server/aiohmi/ipmi/private/serversession.py +++ b/confluent_server/aiohmi/ipmi/private/serversession.py @@ -412,5 +412,5 @@ class IpmiServer(object): # per table 5-2, completion code 0xc1 is 'unrecognized' await session.send_ipmi_response(code=0xc1) - def logout(self): + async def logout(self, sessionok=True): pass