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

[Martin Kletzander]

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 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/20151104/09c84ef0/attachment-0001.sig>