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

[Erik Skultety]

>> +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.

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