Re: [Kimchi-devel] [PATCH] [Kimchi 2/6] Implement the web serial console server

On 05-02-2016 14:20, Aline Manera wrote: > > > On 02/04/2016 05:47 PM, Jose Ricardo Ziviani wrote: >> - This server opens an unix socket server that listens to clients >> insterested in use a guest serial console. >> - Each connection is isolated in its own process, that is destroyed >> when the client disconnects (timeout/ctrl+q/browser tab closed). >> Such isolation avoids any block operation on the main process. >> - Websockify is the responsible to receive the connection from the >> client websocket to the local unix socket server. >> >> Signed-off-by: Jose Ricardo Ziviani <joserz(a)linux.vnet.ibm.com&gt; >> --- >> serial_console.py | 290 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 290 insertions(+) >> create mode 100644 serial_console.py >> >> diff --git a/serial_console.py b/serial_console.py >> new file mode 100644 >> index 0000000..b0659a5 >> --- /dev/null >> +++ b/serial_console.py > > License header is missing. > > I also suggest to rename the file to serialconsole.py (without > underscore) to have the imports listed properly then. > >> @@ -0,0 +1,290 @@ >> +import libvirt >> +import os >> +import socket >> +import sys >> +import threading >> +import time >> + >> +from multiprocessing import Process >> + >> + >> +from wok.utils import wok_log >> +from wok.plugins.kimchi import model >> + >> + >> +SOCKET_QUEUE_BACKLOG = 0 >> +DEFAULT_TIMEOUT = 120 # seconds >> +CTRL_Q = '\x11' >> + >> + >> +class socket_server(object): > > It is better to use SocketServer() to keep code styling consistent. OK > >> + """Unix socket server for guest console access. >> + >> + Implements a unix socket server for each guest, this server will >> receive >> + data from a particular client, forward that data to the guest >> console, >> + receive the response from the console and send the response back >> to the >> + client. >> + >> + Features: >> + - one socket server per client connection; >> + - server listens to unix socket; >> + - exclusive connection per guest; >> + - websockity handles the proxy between the client websocket >> to the >> + local unix socket; >> + >> + Note: >> + - old versions (< 0.6.0)of websockify don't handle their >> children >> + processes accordingly, leaving a zombie process behind (this >> also >> + happens with novnc). >> + """ >> + >> + def __init__(self, guest_name): >> + """Constructs a unix socket server. >> + >> + Listens to connections on /tmp/<guest name>. >> + """ >> + self._guest_name = guest_name > >> + self._server_addr = '/tmp/%s' % guest_name > > Why is it needed as websocket already has a controller file under > /var/lib/kimchi/vnc-tokens? Do you mean to save the socket file into /var/lib/kimchi/vnc-tokens instead of /tmp? My initial idea is that /tmp is always clean after any unexpected reboot or something. But I can change it.

> > Also using the guest_name is not a good idea as we support > internationalization and some special character may not be accepted. guest_name here is already encoded to utf-8 (.encode('utf-8')), but I'll double check if any issue could happen > >> + if os.path.exists(self._server_addr): >> + wok_log.error('Cannot connect to %s due to an existing ' >> + 'connection', guest_name) >> + raise RuntimeError('There is an existing connection to >> %s' % >> + guest_name) >> + >> + self._socket = socket.socket(socket.AF_UNIX, >> + socket.SOCK_STREAM) >> + self._socket.setsockopt(socket.SOL_SOCKET, >> + socket.SO_REUSEADDR, >> + 1) >> + self._socket.bind(self._server_addr) >> + self._socket.listen(SOCKET_QUEUE_BACKLOG) >> + self._stop = False >> + wok_log.info('socket server to guest %s created', guest_name) >> + >> + def _send_to_client(self, stream, event, opaque): >> + """Handles libvirt stream readable events. >> + >> + Each event will be send back to the client socket. >> + """ >> + try: >> + data = stream.recv(1024) >> + >> + except Exception as e: >> + wok_log.info('Error when reading from console: %s', >> e.message) >> + return >> + >> + # return if no data received or client socket(opaque) is not >> valid >> + if not data or not opaque: >> + return >> + >> + opaque.send(data) >> + >> + def libvirt_event_loop(self, guest, client): >> + """Runs libvirt event loop. >> + """ >> + # stop the event loop when the guest is not running >> + while guest.is_running(): >> + libvirt.virEventRunDefaultImpl() >> + >> + # shutdown the client socket to unblock the recv and stop the >> + # server as soon as the guest shuts down >> + client.shutdown(socket.SHUT_RD) >> + >> + def listen(self): >> + """Prepares the environment before starts to accept >> connections >> + >> + Initializes and destroy the resources needed to accept >> connection. >> + """ >> + libvirt.virEventRegisterDefaultImpl() >> + try: >> + guest = libvirt_guest(self._guest_name) >> + >> + except Exception as e: >> + wok_log.error('Cannot open the guest %s due to %s', >> + self._guest_name, e.message) >> + self._socket.close() >> + sys.exit(1) >> + >> + except (KeyboardInterrupt, SystemExit): >> + self._socket.close() >> + sys.exit(1) >> + >> + console = None >> + try: >> + console = guest.get_console() >> + self._listen(guest, console) >> + >> + # clear resources aquired when the process is killed >> + except (KeyboardInterrupt, SystemExit): >> + pass >> + >> + finally: >> + wok_log.info("Shutting down the socket server to %s >> console", >> + self._guest_name) >> + self._socket.close() >> + if os.path.exists(self._server_addr): >> + os.unlink(self._server_addr) >> + >> + try: >> + console.eventRemoveCallback() >> + >> + except Exception as e: >> + wok_log.info('Callback is probably removed: %s', >> e.message) >> + >> + guest.close() >> + >> + def _listen(self, guest, console): >> + """Accepts client connections. >> + >> + Each connection is directly linked to the desired guest >> console. Thus >> + any data received from the client can be send to the guest >> console as >> + well as any response from the guest console can be send back >> to the >> + client console. >> + """ >> + client, client_addr = self._socket.accept() >> + client.settimeout(DEFAULT_TIMEOUT) >> + wok_log.info('Client %s connected to %s', >> + str(client_addr), >> + self._guest_name) >> + >> + # register the callback to receive any data from the console >> + console.eventAddCallback(libvirt.VIR_STREAM_EVENT_READABLE, >> + self._send_to_client, >> + client) >> + >> + # start the libvirt event loop in a python thread >> + libvirt_loop = >> threading.Thread(target=self.libvirt_event_loop, >> + args=(guest, client)) >> + libvirt_loop.start() >> + >> + while True: >> + data = '' >> + try: >> + data = client.recv(1024) >> + >> + except Exception as e: >> + wok_log.info('Client %s disconnected from %s: %s', >> + str(client_addr), >> + self._guest_name, >> + e.message) >> + break >> + >> + if not data or data == CTRL_Q: >> + break >> + >> + # if the console can no longer be accessed, close >> everything >> + # and quits >> + try: >> + console.send(data) >> + >> + except: >> + wok_log.info('Console of %s is not accessible', >> + self._guest_name) >> + break >> + >> + # clear used resources when the connection is closed and, if >> possible, >> + # tell the client the connection was lost. >> + try: >> + client.send('\r\n\r\nClient disconnected\r\n') >> + >> + except: >> + pass >> +# socket_server >> + >> + >> +class libvirt_guest(object): >> + > > LibvirtGuest() OK > >> + def __init__(self, guest_name): >> + """ >> + Constructs a guest object that opens a connection to >> libvirt and >> + searchs for a particular guest, provided by the caller. >> + """ >> + try: >> + libvirt = model.libvirtconnection.LibvirtConnection( >> + 'qemu:///system') >> + self._guest = model.vms.VMModel.get_vm(guest_name, >> libvirt) >> + > > That way, we will open a new libvirt connection. We can reuse the same > along the whole application. > > And also, you are assuming the "qemu:///system" driver and it is not > true for MockModel (one more reason to do not open a new connection to > libvirt) > > You can receive it as a parameter for the LibvirtGuest() instance. Using the same libvirt connection under a new process doesn't work well. I believe that a reference is passed to the new process that, when finished, it closes that connection (and close Kimchi connection's to libvirt as well because both points to the same object). I'll investigate a bit more, if that's the case I could check on how to increase the refcount for such object and let you know then.

> >> + except Exception as e: >> + wok_log.error('Cannot open guest %s: %s', guest_name, >> e.message) >> + raise >> + >> + self._libvirt = libvirt.get() >> + self._name = guest_name >> + self._stream = None >> + >> + def get_name(self): >> + return self._name >> + >> + def is_running(self): >> + """ >> + Checks if this guest is currently in a running state. >> + """ >> + return self._guest.state(0)[0] == libvirt.VIR_DOMAIN_RUNNING >> or \ >> + self._guest.state(0)[0] == libvirt.VIR_DOMAIN_PAUSED >> + >> + def get_console(self): >> + """ >> + Opens a console to this guest and returns a reference to it. >> + Note: If another instance (eg: virsh) has an existing console >> opened >> + to this guest, this code will steal that console. >> + """ >> + # guest must be in a running state to get its console >> + counter = 10 >> + while not self.is_running(): >> + wok_log.info('Guest %s is not running, waiting for it', >> + self._name) >> + >> + counter -= 1 >> + if counter <= 0: >> + return None >> + >> + time.sleep(1) >> + >> + # attach a stream in the guest console so we can read >> from/write to it >> + if self._stream is None: >> + wok_log.info('Opening the console for guest %s', >> + self._name) >> + self._stream = >> self._libvirt.newStream(libvirt.VIR_STREAM_NONBLOCK) >> + self._guest.openConsole(None, >> + self._stream, >> + libvirt.VIR_DOMAIN_CONSOLE_FORCE | >> + libvirt.VIR_DOMAIN_CONSOLE_SAFE) >> + return self._stream >> + >> + def close(self): >> + """Closes the libvirt connection. >> + """ >> + self._libvirt.close() >> +# guest >> + >> + >> +def main(guest_name): >> + """Main entry point to create a socket server. >> + >> + Starts a new socket server to listen messages to/from the guest. >> + """ >> + server = None >> + try: >> + server = socket_server(guest_name) >> + >> + except Exception as e: >> + wok_log.error('Cannot create the socket server for %s due to >> %s', >> + guest_name, e.message) >> + raise >> + >> + proc = Process(target=server.listen) >> + proc.start() >> + return proc >> + >> + >> +if __name__ == '__main__': >> + """Executes a stand alone instance of the socket server. >> + >> + This may be useful for testing/debugging. >> + """ >> + argc = len(sys.argv) >> + if argc != 2: >> + print 'usage: ./%s <guest_name>' % sys.argv[0] >> + sys.exit(1) >> + >> + main(sys.argv[1]) >