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

Re: [libvirt] [PATCH 09/10] Inhibit desktop shutdown while any virtual machines are running



On Tue, Nov 27, 2012 at 04:20:49PM -0500, Eric Blake wrote:
> > Use the freedesktop inhibition DBus service to prevent host
> > shutdown or session logout while any VMs are running.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange redhat com>
> > ---
> >  src/rpc/virnetserver.c | 110
> >  +++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 110 insertions(+)
> > 
> 
> > +++ b/src/rpc/virnetserver.c
> > @@ -27,6 +27,10 @@
> >  #include <string.h>
> >  #include <fcntl.h>
> >  
> > +#ifdef HAVE_DBUS
> > +# include <dbus/dbus.h>
> 
> Do we really need this header...

No, its obsolete.

> 
> > +#endif
> > +
> >  #include "virnetserver.h"
> >  #include "logging.h"
> >  #include "memory.h"
> > @@ -37,6 +41,7 @@
> >  #include "virfile.h"
> >  #include "event.h"
> >  #include "virnetservermdns.h"
> > +#include "virdbus.h"
> 
> ...or is this local header sufficient?  (That is, should you rework
> this patch to put the raw dbus_* calls isolated into virdbus.[ch],
> rathar than having this file have to use conditional compilation)?

Correct.

> >  void virNetServerAddShutdownInhibition(virNetServerPtr srv)
> >  {
> >      virNetServerLock(srv);
> >      srv->autoShutdownInhibitions++;
> > +
> > +    VIR_DEBUG("srv=%p inhibitions=%zu", srv,
> > srv->autoShutdownInhibitions);
> 
> Should this debug line be hoisted into the earlier patch that introduced
> inhibition callbacks?
> 
> > @@ -728,6 +830,12 @@ void
> > virNetServerRemoveShutdownInhibition(virNetServerPtr srv)
> >  {
> >      virNetServerLock(srv);
> >      srv->autoShutdownInhibitions--;
> > +
> > +    VIR_DEBUG("srv=%p inhibitions=%zu", srv,
> > srv->autoShutdownInhibitions);
> 
> Again, should this debug be hoisted into an earlier patch?

I don't think it really matters - this patch is the first time that it
is interesting to see the debug info :-)

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]