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

[Eric Blake]

> 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 at 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...

> +#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)?

> @@ -391,6 +398,7 @@ virNetServerPtr virNetServerNew(size_t
> min_workers,
>      srv->clientPrivFree = clientPrivFree;
>      srv->clientPrivOpaque = clientPrivOpaque;
>      srv->privileged = geteuid() == 0 ? true : false;

Pre-existing, but I find 'some_bool_expr ? true : false' to be
overkill, compared to 'some_bool_expr'.

> +
> +/* As per: http://www.freedesktop.org/wiki/Software/systemd/inhibit
> */
> +static void virNetServerCallInhibit(virNetServerPtr srv,
> +                                    const char *what,
> +                                    const char *who,
> +                                    const char *why,
> +                                    const char *mode)
> +{
> +    DBusMessage *message;
> +    DBusPendingCall *pendingReply;
> +    DBusConnection *systemBus;
> +
> +    VIR_DEBUG("srv=%p what=%s who=%s why=%s mode=%s",

syntax-check didn't complain about mode=%s?  (I know it complains
about mode=%d, and favors mode=%o instead; but I guess a string
mode should not be an error).

>  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?

Overall, looks pretty slick!