[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Support reboots with the QEMU driver



On Fri, Jun 17, 2011 at 10:52:03AM +0800, Wen Congyang wrote:
> At 06/16/2011 01:06 AM, Daniel P. Berrange Write:
> > +
> > +
> >  static int
> >  qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
> >                            virDomainObjPtr vm)
> >  {
> > +    qemuDomainObjPrivatePtr priv = vm->privateData;
> 
> I think we should get vm->privateData after we lock vm.

Yep, good point.

> 
> > +    VIR_DEBUG("vm=%p", vm);
> > +
> >      virDomainObjLock(vm);
> > -    ((qemuDomainObjPrivatePtr) vm->privateData)->gotShutdown = true;
> > +    priv->gotShutdown = true;
> > +    if (priv->fakeReboot) {
> > +        virDomainObjRef(vm);
> > +        virThread th;
> > +        if (virThreadCreate(&th,
> > +                            false,
> > +                            qemuProcessFakeReboot,
> > +                            vm) < 0) {
> > +            VIR_ERROR("Failed to create reboot thread, killing domain");
> > +            qemuProcessKill(vm);
> 
> We should hold an extra reference the vm here. If the vm is not persistent
> and we destroy the vm before the thread runs, libvirtd may crash(I think, not test
> it).

I'm already taking an extra reference just before starting the
thread. I need to release that actually in virThreadCreate fails
to run.

> 
> > +        }
> > +    } else {
> > +        qemuProcessKill(vm);
> > +    }
> >      virDomainObjUnlock(vm);
> >  
> >      return 0;

Daiel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]