From ddf4d9751177dbb849892e0a071188cb4444844b Mon Sep 17 00:00:00 2001 From: modeco80 Date: Thu, 11 Jul 2024 20:33:50 -0400 Subject: [PATCH] qemu: more refactoring qmp client is now much more robust (and doesn't fight over itself as much). this should hopefully completely eliminate the case where display connects but qmp is half connected. i also forgot QemuDisplay actually emits an event on connection, so we can just use that to wait for when the display connects. which allows us to set the started state there instead of praying when the qmp client connects that we are connected to the display roughly at the same time. i also gated some stuff to require vm state in the server. this is a bit rickity, but does seem to work. --- cvmts/src/CollabVMServer.ts | 80 ++++++++++++++++++++++--------------- cvmts/src/VM.ts | 2 + cvmts/src/VNCVM/VNCVM.ts | 7 +++- qemu/src/QemuVM.ts | 77 ++++++++++++++++++----------------- 4 files changed, 96 insertions(+), 70 deletions(-) diff --git a/cvmts/src/CollabVMServer.ts b/cvmts/src/CollabVMServer.ts index d9a2aa2..d972b32 100644 --- a/cvmts/src/CollabVMServer.ts +++ b/cvmts/src/CollabVMServer.ts @@ -106,12 +106,11 @@ export default class CollabVMServer { this.indefiniteTurn = null; this.ModPerms = Utilities.MakeModPerms(this.Config.collabvm.moderatorPermissions); - let initSize = vm.GetDisplay().Size() || { + // No size initially, since the + this.OnDisplayResized({ width: 0, height: 0 - }; - - this.OnDisplayResized(initSize); + }); this.VM = vm; @@ -119,7 +118,8 @@ export default class CollabVMServer { let self = this; if (config.vm.type == 'qemu') { (vm as QemuVM).on('statechange', (newState: VMState) => { - if(newState == VMState.Started) { + if (newState == VMState.Started) { + self.logger.Info("VM started"); // well aware this sucks but whatever self.VM.GetDisplay().on('resize', (size: Size) => self.OnDisplayResized(size)); self.VM.GetDisplay().on('rect', (rect: Rect) => self.OnDisplayRectangle(rect)); @@ -214,12 +214,11 @@ export default class CollabVMServer { // Set username if (client.countryCode !== null && client.noFlag) { // privacy - for (let cl of this.clients.filter(c => c !== client)) { + for (let cl of this.clients.filter((c) => c !== client)) { cl.sendMsg(cvm.guacEncode('remuser', '1', client.username!)); } this.renameUser(client, res.username, false); - } - else this.renameUser(client, res.username, true); + } else this.renameUser(client, res.username, true); // Set rank client.rank = res.rank; if (client.rank === Rank.Admin) { @@ -241,18 +240,28 @@ export default class CollabVMServer { } break; case 'noflag': { - if (client.connectedToNode) // too late + if (client.connectedToNode) + // too late return; client.noFlag = true; } case 'list': - client.sendMsg(cvm.guacEncode('list', this.Config.collabvm.node, this.Config.collabvm.displayname, this.screenHidden ? this.screenHiddenThumb : await this.getThumbnail())); + if (this.VM.GetState() == VMState.Started) { + client.sendMsg(cvm.guacEncode('list', this.Config.collabvm.node, this.Config.collabvm.displayname, this.screenHidden ? this.screenHiddenThumb : await this.getThumbnail())); + } break; case 'connect': if (!client.username || msgArr.length !== 2 || msgArr[1] !== this.Config.collabvm.node) { client.sendMsg(cvm.guacEncode('connect', '0')); return; } + + // Don't allow connecting if the VM hasn't started + if (this.VM.GetState() != VMState.Started) { + client.sendMsg(cvm.guacEncode('connect', '0')); + return; + } + client.connectedToNode = true; client.sendMsg(cvm.guacEncode('connect', '1', '1', this.VM.SnapshotsSupported() ? '1' : '0', '0')); if (this.ChatHistory.size !== 0) client.sendMsg(this.getChatHistoryMsg()); @@ -275,6 +284,12 @@ export default class CollabVMServer { return; } + // similar rationale to 'connect' + if (this.VM.GetState() != VMState.Started) { + client.sendMsg(cvm.guacEncode('connect', '0')); + return; + } + switch (msgArr[2]) { case '0': client.viewMode = 0; @@ -443,20 +458,21 @@ export default class CollabVMServer { } this.sendVoteUpdate(); break; - case "cap": { + case 'cap': { if (msgArr.length < 2) return; // Capabilities can only be announced before connecting to the VM if (client.connectedToNode) return; var caps = []; - for (const cap of msgArr.slice(1)) switch(cap) { - case "bin": { - if (caps.indexOf("bin") !== -1) break; - client.Capabilities.bin = true; - caps.push("bin"); - break; + for (const cap of msgArr.slice(1)) + switch (cap) { + case 'bin': { + if (caps.indexOf('bin') !== -1) break; + client.Capabilities.bin = true; + caps.push('bin'); + break; + } } - } - client.sendMsg(cvm.guacEncode("cap", ...caps)); + client.sendMsg(cvm.guacEncode('cap', ...caps)); } case 'admin': if (msgArr.length < 2) return; @@ -719,10 +735,11 @@ export default class CollabVMServer { if (announce) this.clients.forEach((c) => c.sendMsg(cvm.guacEncode('rename', '1', oldname, client.username!, client.rank.toString()))); } else { this.logger.Info(`Rename ${client.IP.address} to ${client.username}`); - if (announce) this.clients.forEach((c) => { - c.sendMsg(cvm.guacEncode('adduser', '1', client.username!, client.rank.toString())); - if (client.countryCode !== null) c.sendMsg(cvm.guacEncode('flag', client.username!, client.countryCode)); - }); + if (announce) + this.clients.forEach((c) => { + c.sendMsg(cvm.guacEncode('adduser', '1', client.username!, client.rank.toString())); + if (client.countryCode !== null) c.sendMsg(cvm.guacEncode('flag', client.username!, client.countryCode)); + }); } } @@ -733,9 +750,9 @@ export default class CollabVMServer { return cvm.guacEncode(...arr); } - getFlagMsg() : string { + getFlagMsg(): string { var arr = ['flag']; - for (let c of this.clients.filter(cl => cl.countryCode !== null && cl.username && (!cl.noFlag || cl.rank === Rank.Unregistered))) { + for (let c of this.clients.filter((cl) => cl.countryCode !== null && cl.username && (!cl.noFlag || cl.rank === Rank.Unregistered))) { arr.push(c.username!, c.countryCode!); } return cvm.guacEncode(...arr); @@ -814,14 +831,14 @@ export default class CollabVMServer { private async OnDisplayRectangle(rect: Rect) { let encoded = await this.MakeRectData(rect); - let encodedb64 = encoded.toString("base64"); - let bmsg : CollabVMProtocolMessage = { + let encodedb64 = encoded.toString('base64'); + let bmsg: CollabVMProtocolMessage = { type: CollabVMProtocolMessageType.rect, rect: { x: rect.x, y: rect.y, data: encoded - }, + } }; var encodedbin = msgpack.encode(bmsg); this.clients @@ -860,7 +877,7 @@ export default class CollabVMServer { client.sendMsg(cvm.guacEncode('size', '0', displaySize.width.toString(), displaySize.height.toString())); if (client.Capabilities.bin) { - let msg : CollabVMProtocolMessage = { + let msg: CollabVMProtocolMessage = { type: CollabVMProtocolMessageType.rect, rect: { x: 0, @@ -870,7 +887,7 @@ export default class CollabVMServer { }; client.socket.sendBinary(msgpack.encode(msg)); } else { - client.sendMsg(cvm.guacEncode('png', '0', '0', '0', '0', encoded.toString("base64"))); + client.sendMsg(cvm.guacEncode('png', '0', '0', '0', '0', encoded.toString('base64'))); } } @@ -879,8 +896,7 @@ export default class CollabVMServer { let displaySize = display.Size(); // TODO: actually throw an error here - if(displaySize.width == 0 && displaySize.height == 0) - return Buffer.from("no") + if (displaySize.width == 0 && displaySize.height == 0) return Buffer.from('no'); let encoded = await JPEGEncoder.Encode(display.Buffer(), displaySize, rect); diff --git a/cvmts/src/VM.ts b/cvmts/src/VM.ts index dbdf4bb..d2a8657 100644 --- a/cvmts/src/VM.ts +++ b/cvmts/src/VM.ts @@ -1,3 +1,4 @@ +import { VMState } from '@cvmts/qemu'; import VMDisplay from './VMDisplay.js'; export default interface VM { @@ -7,5 +8,6 @@ export default interface VM { Reset(): Promise; MonitorCommand(command: string): Promise; GetDisplay(): VMDisplay; + GetState(): VMState; SnapshotsSupported(): boolean; } diff --git a/cvmts/src/VNCVM/VNCVM.ts b/cvmts/src/VNCVM/VNCVM.ts index c89ec61..6cf2976 100644 --- a/cvmts/src/VNCVM/VNCVM.ts +++ b/cvmts/src/VNCVM/VNCVM.ts @@ -4,7 +4,7 @@ import VM from '../VM'; import VMDisplay from '../VMDisplay'; import { Clamp, Logger, Rect, Size, Sleep } from '@cvmts/shared'; import { VncClient } from '@computernewb/nodejs-rfb'; -import { BatchRects } from '@cvmts/qemu'; +import { BatchRects, VMState } from '@cvmts/qemu'; import { execaCommand } from 'execa'; export default class VNCVM extends EventEmitter implements VM, VMDisplay { @@ -125,6 +125,11 @@ export default class VNCVM extends EventEmitter implements VM, VMDisplay { return this; } + GetState(): VMState { + // for now! + return VMState.Started; + } + SnapshotsSupported(): boolean { return true; } diff --git a/qemu/src/QemuVM.ts b/qemu/src/QemuVM.ts index d0553fb..ab99930 100644 --- a/qemu/src/QemuVM.ts +++ b/qemu/src/QemuVM.ts @@ -51,14 +51,14 @@ class SocketWriter implements IQmpClientWriter { export class QemuVM extends EventEmitter { private state = VMState.Stopped; + // QMP stuff. private qmpInstance: QmpClient = new QmpClient(); private qmpSocket: Socket | null = null; - private qmpConnected = false; private qmpFailCount = 0; private qemuProcess: ExecaChildProcess | null = null; - private display: QemuDisplay | null; + private display: QemuDisplay | null = null; private definition: QemuVmDefinition; private addedAdditionalArguments = false; @@ -69,8 +69,6 @@ export class QemuVM extends EventEmitter { this.definition = def; this.logger = new Shared.Logger(`CVMTS.QEMU.QemuVM/${this.definition.id}`); - this.display = new QemuDisplay(this.GetVncPath()); - let self = this; // Handle the STOP event sent when using -no-shutdown @@ -86,12 +84,16 @@ export class QemuVM extends EventEmitter { self.VMLog().Info('QMP ready'); this.display = new QemuDisplay(this.GetVncPath()); - self.display?.Connect(); - // QMP has been connected so the VM is ready to be considered started + self.display?.on('connected', () => { + // The VM can now be considered started + self.VMLog().Info("Display connected"); + self.SetState(VMState.Started); + }) + + // now that we've connected to VNC, connect to the display self.qmpFailCount = 0; - self.qmpConnected = true; - self.SetState(VMState.Started); + self.display?.Connect(); }); } @@ -170,6 +172,10 @@ export class QemuVM extends EventEmitter { return this.display!; } + GetState() { + return this.state; + } + /// Private fun bits :) private VMLog() { @@ -223,7 +229,6 @@ export class QemuVM extends EventEmitter { // Disconnect from the display and QMP connections. await self.DisconnectDisplay(); - await self.DisconnectQmp(); // Remove the sockets for VNC and QMP. try { @@ -262,8 +267,10 @@ export class QemuVM extends EventEmitter { private async ConnectQmp() { let self = this; - if (this.qmpConnected) { - this.VMLog().Error('Already connected to QMP!'); + if (this.qmpSocket) { + // This isn't really a problem (since we gate it) + // but I'd like to see if i could eliminate this + this.VMLog().Warning('QemuVM.ConnectQmp(): Already connected to QMP socket!'); return; } @@ -271,20 +278,20 @@ export class QemuVM extends EventEmitter { this.qmpSocket = connect(this.GetQmpPath()); this.qmpSocket.on('close', async () => { - if (self.qmpConnected) { - await self.DisconnectQmp(); + self.qmpSocket?.removeAllListeners(); + self.qmpSocket = null; - // If we aren't stopping, then we should care QMP disconnected - if (self.state != VMState.Stopping) { - if (self.qmpFailCount++ < kMaxFailCount) { - self.VMLog().Error(`Failed to connect to QMP ${self.qmpFailCount} times.`); - await Shared.Sleep(500); - await self.ConnectQmp(); - } else { - self.VMLog().Error(`Reached max retries, giving up.`); - await self.Stop(); - return; - } + // If we aren't stopping (i.e: disconnection wasn't because we disconnected), + // then we should care QMP disconnected + if (self.state != VMState.Stopping) { + if (self.qmpFailCount++ < kMaxFailCount) { + self.VMLog().Error(`Failed to connect to QMP ${self.qmpFailCount} times.`); + await Shared.Sleep(500); + await self.ConnectQmp(); + } else { + self.VMLog().Error(`Reached max retries, giving up.`); + await self.Stop(); + return; } } }); @@ -293,10 +300,15 @@ export class QemuVM extends EventEmitter { self.VMLog().Error('QMP socket error: {0}', e.message); }); - // Setup the QMP client. - let writer = new SocketWriter(this.qmpSocket, this.qmpInstance); - this.qmpInstance.reset(); - this.qmpInstance.setWriter(writer); + this.qmpSocket.on('connect', () => { + self.VMLog().Info("Connected to QMP socket"); + + // Setup the QMP client. + let writer = new SocketWriter(self.qmpSocket!, self.qmpInstance); + self.qmpInstance.reset(); + self.qmpInstance.setWriter(writer); + }) + } private async DisconnectDisplay() { @@ -307,13 +319,4 @@ export class QemuVM extends EventEmitter { // oh well lol } } - - private async DisconnectQmp() { - if (!this.qmpConnected) return; - this.qmpConnected = false; - - if (this.qmpSocket == null) return; - this.qmpSocket?.end(); - this.qmpSocket = null; - } }