On Tue, Nov 27, 2012 at 04:31:16PM -0500, Eric Blake wrote:
> > When the session dies or when the system is going to be shut down
> > we issue a virStateStop() call to instruct drivers to prepare to
> > be stopped. This will remove any previously acquire inhibitions.
>
> s/acquire/acquired/
>
> >
> > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> > ---
> > daemon/libvirtd.c | 84
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 84 insertions(+)
> >
>
> > +++ b/daemon/libvirtd.c
> > @@ -98,6 +98,11 @@
> >
> > #include "configmake.h"
> >
> > +#ifdef HAVE_DBUS
> > +# include <dbus/dbus.h>
>
> Again, is this necessary,
>
> > +# include "virdbus.h"
>
> or should we be hiding the interface into virdbus.h, which
> can be unconditionally included?
Correct, it is bogus
> > +static void daemonStopWorker(void *opaque)
> > +{
> > + virNetServerPtr srv = opaque;
> > +
> > + VIR_DEBUG("Begin stop srv=%p", srv);
> > +
> > + ignore_value(virStateStop());
> > +
> > + VIR_DEBUG("Completed stop srv=%p", srv);
>
> I think the VIR_DEBUG should log the return value of virStateStop(),
> rather than blindly ignoring the possibility of a failed shutdown.
Good idea.
>
> > +static DBusHandlerResult handleSessionMessageFunc(DBusConnection
> > *connection ATTRIBUTE_UNUSED,
>
> Long lines; could be shortened by using:
>
> static DBusHandlerResult
> handleSessionMessageFunc(...)
>
> Again, looks slick; and my objection earlier in the series about
> qemu:///system vs. libvirt-guests init script may have been
> premature, seeing as how you are tying this inhibition handling
> only to sessions. Still, I'd be interested in your responses
> to my questions before granting ack.
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