Re: [libvirt] [PATCH v3 03/11] admin: Introduce virAdmConnectIsAlive

[Martin Kletzander]

On Fri, Nov 06, 2015 at 12:46:18PM +0100, Erik Skultety wrote:

Since most of our APIs rely on an acive functional connection to a daemon and
we have such a mechanism in libvirt already, there's need to have such a way in
libvirt-admin as well. By introducing a new public API, this patch provides
support to check for an active connection.
---
include/libvirt/libvirt-admin.h |  1 +
src/libvirt-admin.c             | 29 +++++++++++++++++++++++++++++
src/libvirt_admin_public.syms   |  1 +
tools/virt-admin.c              |  4 ++--
4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index 9997cc2..72671c6 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -52,6 +52,7 @@ virAdmConnectPtr virAdmConnectOpen(const char *name, unsigned 
int flags);
int virAdmConnectClose(virAdmConnectPtr conn);

int virAdmConnectRef(virAdmConnectPtr conn);
+int virAdmConnectIsAlive(virAdmConnectPtr conn);

# ifdef __cplusplus
}
diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
index 5a4fc48..105727f 100644
--- a/src/libvirt-admin.c
+++ b/src/libvirt-admin.c
@@ -386,3 +386,32 @@ virAdmConnectRef(virAdmConnectPtr conn)

    return 0;
}
+
+/**
+ * virAdmConnectIsAlive:
+ * @conn: connection to admin server
+ *
+ * Decide whether the connection to the admin server is alive or not.
+ * Connection is considered alive if the channel it is running over is not
+ * closed.
+ *
+ * Returns 1, if the connection is alive, 0 if the channel has already
+ * been closed or -1 on error.
+ */
+int
+virAdmConnectIsAlive(virAdmConnectPtr conn)
+{
+    bool ret;
+    remoteAdminPrivPtr priv = conn->privateData;
+

I know it wouldn't be consistent with virConnectIsAlive(), but we
could handle NULLs nicely, e.g. just return 0.  I guess -1 is OK as
well because everyone will check for '== 1' anyway (if at all ever).

+    VIR_DEBUG("conn=%p", conn);
+
+    virResetLastError();
+    virCheckAdmConnectReturn(conn, -1);
+
+    virObjectLock(priv);
+    ret = virNetClientIsOpen(priv->client);
+    virObjectUnlock(priv);
+
+    return ret;
+}
diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms
index d9e3c0b..16cfd42 100644
--- a/src/libvirt_admin_public.syms
+++ b/src/libvirt_admin_public.syms
@@ -15,4 +15,5 @@ LIBVIRT_ADMIN_1.3.0 {
        virAdmConnectOpen;
        virAdmConnectClose;
        virAdmConnectRef;
+        virAdmConnectIsAlive;
};
diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index ddfba91..6b009ea 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -159,10 +159,10 @@ vshAdmConnectionHandler(vshControl *ctl)
{
    vshAdmControlPtr priv = ctl->privData;

-    if (!priv->conn)
+    if (!priv->conn || !virAdmConnectIsAlive(priv->conn))
        vshAdmReconnect(ctl);

you could then drop the '!priv->conn' here...

ACK either way if nobody else has an opinion on that for some time.
Just remember that, you need to adjust the documentation for
virAdmConnectIsAlive() if you change its behaviour.

-    if (!priv->conn) {
+    if (!priv->conn || !virAdmConnectIsAlive(priv->conn)) {
        vshError(ctl, "%s", _("no valid connection"));
        return NULL;
    }
--
2.4.3

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

signature.asc
Description: PGP signature

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