On Tue, Feb 12, 2013 at 04:47:12PM -0700, Eric Blake wrote:
> On 02/11/2013 09:47 AM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> >
> > With the majority of fields in the virQEMUDriverPtr struct
> > now immutable or self-locking, there is no need for practically
> > any methods to be using the QEMU driver lock. Only a handful
> > of helper APIs in qemu_conf.c now need it
>
> I agree with squashing in Martin's cleanups. Beyond that, the locking
> changes looked sane.
>
> ACK. Let's get this in and stress-tested before the 1.0.3 release.
>
> > +++ b/src/qemu/qemu_driver.c
> > @@ -153,16 +153,9 @@ virQEMUDriverPtr qemu_driver = NULL;
> >
> >
> > static void
> > -qemuVMDriverLock(void) {
> > - qemuDriverLock(qemu_driver);
> > -};
> > -
> > -
> > +qemuVMDriverLock(void) {}
> > static void
> > -qemuVMDriverUnlock(void) {
> > - qemuDriverUnlock(qemu_driver);
> > -};
> > -
> > +qemuVMDriverUnlock(void) {}
>
> Do we need to keep these functions any more, or can we get rid of them
> in a followup patch?
We have to provide these stubs for the nwfilter driver, because the
LXC driver still needs it. At some point I'll change the LXC driver
to also use this new approach to locking. When that's done we can
kill this callback from the nwfilter driver.
> > cleanup:
> > - if (watchdogEvent || lifecycleEvent) {
> > - qemuDriverLock(driver);
> > - if (watchdogEvent)
> > - qemuDomainEventQueue(driver, watchdogEvent);
> > - if (lifecycleEvent)
> > - qemuDomainEventQueue(driver, lifecycleEvent);
> > - qemuDriverUnlock(driver);
> > - }
> > + if (watchdogEvent)
> > + qemuDomainEventQueue(driver, watchdogEvent);
> > + if (lifecycleEvent)
> > + qemuDomainEventQueue(driver, lifecycleEvent);
>
> Is it worth a followup patch to qemuDomainEventQueue that becomes a
> no-op when passed a NULL event, so we can let the callers avoid the 'if'?
I think we can probably optimize in a different way. The reason why
we created the events early in the method, but didn't queue them
until late in the method is because we couldn't lock the QEMU
driver at the point we create the events. Now that the QEMU driver
lock is gone, we can probably just immediately queue the events as
we create them, rather than delaying until the cleanup: block. This
would remove the need for the conditional tests entirely.
Daniel
--
|: 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list