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

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

 



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

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]