[libvirt] [PATCH v2 3/9] admin: Introduce virAdmConnectIsAlive
Martin Kletzander
mkletzan at redhat.com
Wed Nov 4 14:35:34 UTC 2015
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>
More information about the libvir-list
mailing list