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

Re: [libvirt] [PATCH 10/10] Shut down session libvirtd cleanly on host shutdown/user logout



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 redhat com>
> > ---
> >  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 :|


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