[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



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

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

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


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