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

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

 




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@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
> ---
>  src/remote/remote_daemon_dispatch.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 

Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>

John

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


> +    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;
>  }
>  
> 

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

  Powered by Linux