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
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list