On Mon, Jan 22, 2018 at 06:16:04PM +0100, Pavel Hrdina wrote:
> If the connection dies for some reason between D-Bus calls we need
> to properly reconnect because the current connection is not usable
> anymore. Without this the only solution would be to restart the
> libvirt-dbus daemon.
>
> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> ---
> src/connect.c | 37 +++++++++++++++++++++++++++----------
> 1 file changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/src/connect.c b/src/connect.c
> index 9de764c..10183f3 100644
> --- a/src/connect.c
> +++ b/src/connect.c
> @@ -4,6 +4,7 @@
> #include "util.h"
>
> #include <errno.h>
> +#include <stdbool.h>
> #include <stdlib.h>
>
> static int virtDBusConnectCredType[] = {
> @@ -34,12 +35,34 @@ static virConnectAuth virtDBusConnectAuth = {
> NULL,
> };
>
> +static void
> +virtDBusConnectClose(virtDBusConnect *connect,
> + bool deregisterEvents)
> +{
> +
> + for (int i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i += 1) {
> + if (connect->callback_ids[i] >= 0) {
> + if (deregisterEvents) {
> + virConnectDomainEventDeregisterAny(connect->connection,
> + connect->callback_ids[i]);
> + }
> + connect->callback_ids[i] = -1;
> + }
> + }
> +
> + virConnectClose(connect->connection);
I think it is prudent to set connect->connection = NULL at this
point.
> static int
> virtDBusConnectOpen(virtDBusConnect *connect,
> sd_bus_error *error)
> {
> - if (connect->connection)
> - return 0;
> + if (connect->connection) {
> + if (virConnectIsAlive(connect->connection))
This means that every single dbus call runs an extra RPC call to ping
the server for liveliness.
That's not a huge problem, but at some point I'd recommend that
just use the close callback to immediately detect connection failure
and close the connection & run background job to re-open it.
Presumably you're going to issue dbus signals for domain lifecycle
events. Using the close callback & job to re-open means your
lifecycle events will start working again in the shortest amount
of time, instead of waiting for the next methd call to detect
the failure.
> + return 0;
> + else
> + virtDBusConnectClose(connect, false);
> + }
>
> virtDBusConnectAuth.cbdata = error;
>
> @@ -254,14 +277,8 @@ virtDBusConnectFree(virtDBusConnect *connect)
> if (connect->bus)
> sd_bus_unref(connect->bus);
>
> - if (connect->connection) {
> - for (int i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i += 1) {
> - if (connect->callback_ids[i] >= 0)
> - virConnectDomainEventDeregisterAny(connect->connection, connect->callback_ids[i]);
> - }
> -
> - virConnectClose(connect->connection);
> - }
> + if (connect->connection)
> + virtDBusConnectClose(connect, true);
>
> free(connect);
With the 1 small change
Reviewed-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
