[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 at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151116/b5e78887/attachment-0001.sig>