Re: [PATCH v2 3/9] admin: Introduce virAdmConnectIsAlive

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

 



On Wed, Nov 04, 2015 at 01:56:40PM +0100, Erik Skultety wrote:
+int
+virAdmConnectIsAlive(virAdmConnectPtr conn)
+{
+    bool ret;
+    remoteAdminPrivPtr priv = conn->privateData;
+
+    VIR_DEBUG("conn=%p", conn);
+
+    virResetLastError();
+
+    virObjectLock(priv);
+    ret = virNetClientIsOpen(priv->client);
+    virObjectUnlock(priv);
+
+    if (ret)
+        return 1;
+    else
+        return 0;

return ret; is fine here.

Yep, thanks.


+}
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 cc33b7b..1bc10b0 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -171,7 +171,7 @@ vshAdmConnectionHandler(vshControl *ctl)
    if (!priv->conn || disconnected)
        vshAdmReconnect(ctl);

-    if (!priv->conn) {
+    if (!priv->conn || !virAdmConnectIsAlive(priv->conn)) {

I know we don't do that in virsh, so it doesn't seem obvious here, but
shouldn't we do reconnect also when the connection is not alive?  I,
personally, would add it into the previous condition as well.

Well, you're right. But things are more interesting than that. If you
look at it closely (and I must have been blind or something until now),
disconnect and virConnectIsAlive (standard libvirt) symbolize the very
same thing, so the presence of disconnect variable in virsh is imho due
to historical reasons (as virConnectIsAlive has been present since
0.9.X). In virt-admin however, virAdmConnectIsAlive API will be
introduced at the same time as the client itself, so having that
variable in the code will only contribute to more confusion. And I also
need to have a look at vsh module which I added recently as
'disconnected' sneaked into that one as well and I'm not sure yet it is
necessary to have it there at all.


Oh, cool, I didn't go to such details.  It would certainly be nice to
have this cleaned up.


ACK.

        vshError(ctl, "%s", _("no valid connection"));
        return NULL;
    }
--
2.4.3

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

Attachment: signature.asc
Description: PGP signature

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