[Spice-devel] [PATCH 07/15] Replace RedsPrivate::mig_wait_disconnect_clients with a GList

[Frediano Ziglio]

> 
> From: Christophe Fergeau <cfergeau at redhat.com>
> 
> The code was introducing an intermediate RedsMigWaitDisconnectClient type to
> hold linked list elements, resulting in a memory handling behaviour very
> similar to a GList. Using GList directly makes the code shorter and more
> readable.
> ---
>  server/reds-private.h |  8 ++------
>  server/reds.c         | 35 +++++++----------------------------
>  2 files changed, 9 insertions(+), 34 deletions(-)
> 
> diff --git a/server/reds-private.h b/server/reds-private.h
> index b2afc6b..205973a 100644
> --- a/server/reds-private.h
> +++ b/server/reds-private.h
> @@ -120,11 +120,6 @@ typedef struct RedsMigTargetClient {
>      Ring pending_links;
>  } RedsMigTargetClient;
>  
> -typedef struct RedsMigWaitDisconnectClient {
> -    RingItem link;
> -    RedClient *client;
> -} RedsMigWaitDisconnectClient;
> -
>  /* Intermediate state for on going monitors config message from a single
>   * client, being passed to the guest */
>  typedef struct RedsClientMonitorsConfig {
> @@ -160,9 +155,10 @@ struct RedsState {
>      int mig_wait_connect; /* src waits for clients to establish connection
>      to dest
>                               (before migration starts) */
>      int mig_wait_disconnect; /* src waits for clients to disconnect (after
>      migration completes) */
> -    Ring mig_wait_disconnect_clients; /* List of
> RedsMigWaitDisconnectClient. Holds the clients
> +    GList *mig_wait_disconnect_clients;/* List of
> RedsMigWaitDisconnectClient. Holds the clients
>                                           which the src waits for their
>                                           disconnection */
>  
> +
>      int mig_inprogress;
>      int expect_migrate;
>      int src_do_seamless_migrate; /* per migration. Updated after the
>      migration handshake
> diff --git a/server/reds.c b/server/reds.c
> index 0a2f6f9..ed559c4 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -2857,11 +2857,8 @@ static void reds_mig_fill_wait_disconnect(RedsState
> *reds)
>       * of clients that got connected to the src after migration
>       completion.*/
>      RING_FOREACH(client_item, &reds->clients) {
>          RedClient *client = SPICE_CONTAINEROF(client_item, RedClient, link);
> -        RedsMigWaitDisconnectClient *wait_client;
>  
> -        wait_client = spice_new0(RedsMigWaitDisconnectClient, 1);
> -        wait_client->client = client;
> -        ring_add(&reds->mig_wait_disconnect_clients, &wait_client->link);
> +        reds->mig_wait_disconnect_clients =
> g_list_append(reds->mig_wait_disconnect_clients, client);
>      }
>      reds->mig_wait_disconnect = TRUE;
>      reds->core->timer_start(reds->mig_timer, MIGRATE_TIMEOUT);
> @@ -2869,36 +2866,18 @@ static void reds_mig_fill_wait_disconnect(RedsState
> *reds)
>  
>  static void reds_mig_cleanup_wait_disconnect(RedsState *reds)
>  {
> -    RingItem *wait_client_item;
> -
> -    while ((wait_client_item =
> ring_get_tail(&reds->mig_wait_disconnect_clients))) {
> -        RedsMigWaitDisconnectClient *wait_client;
> -
> -        wait_client = SPICE_CONTAINEROF(wait_client_item,
> RedsMigWaitDisconnectClient, link);
> -        ring_remove(wait_client_item);
> -        free(wait_client);
> -    }
> +    g_list_free(reds->mig_wait_disconnect_clients);
>      reds->mig_wait_disconnect = FALSE;
>  }
>  
>  static void reds_mig_remove_wait_disconnect_client(RedsState *reds,
>  RedClient *client)
>  {
> -    RingItem *wait_client_item;
> -
> -    RING_FOREACH(wait_client_item, &reds->mig_wait_disconnect_clients) {
> -        RedsMigWaitDisconnectClient *wait_client;
> +    g_warn_if_fail(g_list_find(reds->mig_wait_disconnect_clients, client) !=
> NULL);
>  
> -        wait_client = SPICE_CONTAINEROF(wait_client_item,
> RedsMigWaitDisconnectClient, link);
> -        if (wait_client->client == client) {
> -            ring_remove(wait_client_item);
> -            free(wait_client);
> -            if (ring_is_empty(&reds->mig_wait_disconnect_clients)) {
> -                reds_mig_cleanup(reds);
> -            }
> -            return;
> -        }
> +    reds->mig_wait_disconnect_clients =
> g_list_remove(reds->mig_wait_disconnect_clients, client);
> +    if (reds->mig_wait_disconnect_clients == NULL) {
> +       reds_mig_cleanup(reds);
>      }
> -    spice_warning("client not found %p", client);
>  }
>  
>  static void reds_migrate_channels_seamless(RedsState *reds)
> @@ -3357,7 +3336,7 @@ static int do_spice_init(RedsState *reds,
> SpiceCoreInterface *core_interface)
>      ring_init(&reds->channels);
>      ring_init(&reds->mig_target_clients);
>      reds->char_devices = NULL;
> -    ring_init(&reds->mig_wait_disconnect_clients);
> +    reds->mig_wait_disconnect_clients = NULL;
>      reds->vm_running = TRUE; /* for backward compatibility */
>  
>      if (!(reds->mig_timer = reds->core->timer_add(reds->core,
>      migrate_timeout, reds))) {

Acked-by: Frediano Ziglio <fziglio at redhat.com>

Frediano