[libvirt] [PATCH 18/20] remote: Use domainClientEventCallbacks for remoteReplayConnectionClosedEvent

[Marc Hartmayer]

On Thu, Mar 15, 2018 at 07:56 PM +0100, John Ferlan <jferlan at redhat.com> wrote:
> On 03/08/2018 07:20 AM, Marc Hartmayer wrote:
>> This allows us to get rid of another usage of the global variable
>> remoteProgram.
>>
>> Signed-off-by: Marc Hartmayer <mhartmay at linux.vnet.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
>> ---
>>  src/remote/remote_daemon_dispatch.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>
>
> Reviewed-by: John Ferlan <jferlan at redhat.com>
>
> John

Thanks.

>
> (see my note below, I imagine you agree...)
>
>> diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
>> index 9580e854efbe..95940ffdeefe 100644
>> --- a/src/remote/remote_daemon_dispatch.c
>> +++ b/src/remote/remote_daemon_dispatch.c
>> @@ -1661,12 +1661,12 @@ remoteRelayDomainQemuMonitorEvent(virConnectPtr conn,
>>  static
>>  void remoteRelayConnectionClosedEvent(virConnectPtr conn ATTRIBUTE_UNUSED, int reason, void *opaque)
>>  {
>> -    virNetServerClientPtr client = opaque;
>> +    daemonClientEventCallbackPtr callback = opaque;
>>
>>      VIR_DEBUG("Relaying connection closed event, reason %d", reason);
>>
>>      remote_connect_event_connection_closed_msg msg = { reason };
>> -    remoteDispatchObjectEventSend(client, remoteProgram,
>> +    remoteDispatchObjectEventSend(callback->client, callback->program,
>>                                    REMOTE_PROC_CONNECT_EVENT_CONNECTION_CLOSED,
>>                                    (xdrproc_t)xdr_remote_connect_event_connection_closed_msg,
>>                                    &msg);
>> @@ -3814,6 +3814,7 @@ remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server ATTRIBUTE_UNUS
>>                                             virNetMessageErrorPtr rerr)
>>  {
>>      int rv = -1;
>> +    daemonClientEventCallbackPtr callback = NULL;
>>      struct daemonClientPrivate *priv =
>>          virNetServerClientGetPrivateData(client);
>>
>> @@ -3824,9 +3825,16 @@ remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server ATTRIBUTE_UNUS
>>          goto cleanup;
>>      }
>>
>> +    if (VIR_ALLOC(callback) < 0)
>> +        goto cleanup;
>> +    callback->client = virObjectRef(client);
>
> This one's scary because currently that means we currently call
> virConnectRegisterCloseCallback, but haven't been doing the
> virObjectRef(client) prior to using it as an opaque... Meaning
> remoteRelayConnectionClosedEvent could be called with client already
> free'd.

Yep. Or at least, I think so.

>
>
>> +    callback->program = virObjectRef(remoteProgram);
>> +    /* eventID, callbackID, and legacy are not used */
>> +    callback->eventID = -1;
>> +    callback->callbackID = -1;
>>      if (virConnectRegisterCloseCallback(priv->conn,
>>                                          remoteRelayConnectionClosedEvent,
>> -                                        client, NULL) < 0)
>> +                                        callback, remoteEventCallbackFree) < 0)
>>          goto cleanup;
>>
>>      priv->closeRegistered = true;
>> @@ -3834,8 +3842,10 @@ remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server ATTRIBUTE_UNUS
>>
>>   cleanup:
>>      virMutexUnlock(&priv->lock);
>> -    if (rv < 0)
>> +    if (rv < 0) {
>> +        remoteEventCallbackFree(callback);
>>          virNetMessageSaveError(rerr);
>> +    }
>>      return rv;
>>  }
>>
>>
>
--
Beste Grüße / Kind regards
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294