[Spice-devel] [spice-server v2] sound: do not change volume or mute state on migration

[Frediano Ziglio]

> 
> From: Victor Toso <me at victortoso.com>
> 
> On migration, Qemu notify spice-server with the current Guest volume
> and mute state values which currently is handled forwarding these
> values to the client.
> 
> This patch is a complement of f10de4bc084fcc - Here, volume was
> jumping regardless of guest's volume value.
> 
> Resolves: rhbz#1425443
> Signed-off-by: Victor Toso <victortoso at redhat.com>
> ---
>  server/sound.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/server/sound.c b/server/sound.c
> index b1bfaaaa..fc3d8f4a 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -823,6 +823,7 @@ static void snd_channel_set_volume(SndChannel *channel,
>  {
>      SpiceVolumeState *st = &channel->volume;
>      SndChannelClient *client = snd_channel_get_client(channel);
> +    RedChannelClient *rcc;
>  
>      st->volume_nchannels = nchannels;
>      g_free(st->volume);
> @@ -831,6 +832,10 @@ static void snd_channel_set_volume(SndChannel *channel,
>      if (!client || nchannels == 0)
>          return;
>  
> +    rcc = RED_CHANNEL_CLIENT(client);
> +    if
> (red_client_during_migrate_at_target(red_channel_client_get_client(rcc)))
> +        return;
> +
>      snd_set_command(client, SND_VOLUME_MASK);
>      snd_send(client);
>  }
> @@ -846,12 +851,17 @@ static void snd_channel_set_mute(SndChannel *channel,
> uint8_t mute)
>  {
>      SpiceVolumeState *st = &channel->volume;
>      SndChannelClient *client = snd_channel_get_client(channel);
> +    RedChannelClient *rcc;
>  
>      st->mute = mute;
>  
>      if (!client)
>          return;
>  
> +    rcc = RED_CHANNEL_CLIENT(client);
> +    if
> (red_client_during_migrate_at_target(red_channel_client_get_client(rcc)))
> +        return;
> +
>      snd_set_command(client, SND_MUTE_MASK);
>      snd_send(client);
>  }

I'll test today.
As a minor style (I can fix, no issues), we always wants brackets for if
statement, even for a single "return" line.

OT: Looking at this code and considering other channels why we should
care if RedClient or RedChannel is migrating and what does it means that
a RedChannel is migrating? If RedClient is migrating means there are some
channels that have to be migrated. In this case won't make more sense to
ask if this specific RedChannelClient is still migrating?
In case of RedChannel (like common_graphics_channel_get_during_target_migrate)
why this should be common to all related RedChannelClient? If I have a client
migrating and another just connected should not the behaviour be different
on the 2 clients? I know, we don't support multiple client but still looks
in theory wrong to me.

Frediano