On Mon, Dec 15, 2008 at 11:27:27AM +0000, Daniel P. Berrange wrote:
NACK, to these 3 lines - since we go to trouble of creating the statefile, we should store the FD numbers in the statefile too, instead of relying in the linux specific /proc code. This patch actually monitors things in /proc which is not very portable. I'll think about something better. Let's leave out this (and the daemonizing patch for now) until I've cleaned this up.
+ if (vm->stdin_fd == -1 || vm->stdout_fd == -1 || vm->stderr_fd == -1) + goto next_error; + + if (((vm->stdout_watch = virEventAddHandle(vm->stdout_fd, + VIR_EVENT_HANDLE_READABLE | + VIR_EVENT_HANDLE_ERROR | + VIR_EVENT_HANDLE_HANGUP, + qemudDispatchVMEvent, + driver, NULL)) < 0) || + ((vm->stderr_watch = virEventAddHandle(vm->stderr_fd, + VIR_EVENT_HANDLE_READABLE | + VIR_EVENT_HANDLE_ERROR | + VIR_EVENT_HANDLE_HANGUP, + qemudDispatchVMEvent, + driver, NULL)) < 0)) {
It is actually not neccessary to add the _ERROR or _HANGUP flags when registering a handle. Those two conditions are implict for every read or write watch. Yes, other code in QEMU driver adding them is bogus too. Ahh, good to know.
/** * qemudStartup: * @@ -316,6 +454,7 @@ qemudStartup(void) { qemu_driver->autostartDir, NULL, NULL) < 0) goto error; + qemudReconnectVMs(qemu_driver); qemudAutostartConfigs(qemu_driver);
qemuDriverUnlock(qemu_driver); @@ -418,11 +557,14 @@ qemudShutdown(void) {
/* shutdown active VMs */ for (i = 0 ; i < qemu_driver->domains.count ; i++) { + /* FIXME: don't shutdown VMs on daemon shutdown for now */ +#if 0 virDomainObjPtr dom = qemu_driver->domains.objs[i]; virDomainObjLock(dom); if (virDomainIsActive(dom)) qemudShutdownVMDaemon(NULL, qemu_driver, dom); virDomainObjUnlock(dom); +#endif
Yep, just remove this block of code entirely - we dont want to shutdown VMs anymore. If someone wants to explicitly save or shutdown VMs during machine shutdown, they're probably better off explicitly using virsh to do it from an initscript. Great, I'll simply remove it. -- Guido