Re: [libvirt] [PATCH 5/5] read saved vm status on libvirtd startup

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.

> + 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.

> /** > * 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.