[Spice-devel] [spice-server PATCH v1] snd_worker: do not send volume/mute on migration

[Victor Toso]

Hi,

On Fri, Apr 24, 2015 at 01:25:56PM +0200, Marc-André Lureau wrote:
> Hi
> 
> After this patch, I still get wrong volume when migration finishes.

Odd, It does fix it here.

> My test:
> 1) qemu-kvm -m 1024 -spice
> port=5967,disable-ticketing,seamless-migration=on -monitor stdio -vga qxl
> -device AC97  -snapshot ~/VirtualMachines/f21.img
> 2) SPICE_DEBUG=1 gtk/spicy -p 5967
> 3) qemu-kvm -m 1024 -spice
> port=5968,disable-ticketing,seamless-migration=on -vga qxl -device AC97
> -snapshot ~/VirtualMachines/f21.img -monitor stdio -incoming tcp::4445
> 4) on 1) monitor:
> client_migrate_info spice 127.0.0.1 5968
> migrate -d tcp:localhost:4445
> 5) observe volume change in client after migration
>
> Furthermore, how would this patch let the client sync with valid guest
> volume changes? Migration may take a while, the client could still be
> running the guest and volume control should keep working.

As I said in the cover-letter, I've tested as well with a script doing
volume-changes every one second in a slow migration, limited by
bandwidth and it worked well.

But your solution is cleaner and smaller.

> On Fri, Apr 24, 2015 at 12:27 PM, Victor Toso <victortoso at redhat.com> wrote:
>
> > During migration we should not allow volume-changes. QEMU does it
> > internally and it end up sending the current guest volume on migration.
> >
> > We track on the client all volume-changes that happen in the guest and
> > we change the volume of the client application accordingly. But this
> > volume-change on migration is not done by the user and should be
> > ignored.
> >
> > Resolve: https://bugzilla.redhat.com/show_bug.cgi?id=1012868
> > ---
> >  server/snd_worker.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/server/snd_worker.c b/server/snd_worker.c
> > index a1223b4..e1f70d1 100644
> > --- a/server/snd_worker.c
> > +++ b/server/snd_worker.c
> > @@ -1015,6 +1015,10 @@ SPICE_GNUC_VISIBLE void
> > spice_server_playback_set_volume(SpicePlaybackInstance *
> >      if (!channel || nchannels == 0)
> >          return;
> >
> > +    if
> > (red_client_during_migrate_at_target(channel->channel_client->client)) {
> > +        spice_debug("Do not set playback volume during migration");
> > +        return;
> > +    }
> >      snd_playback_send_volume(playback_channel);
> >  }
> >
> > @@ -1029,6 +1033,10 @@ SPICE_GNUC_VISIBLE void
> > spice_server_playback_set_mute(SpicePlaybackInstance *si
> >      if (!channel)
> >          return;
> >
> > +    if
> > (red_client_during_migrate_at_target(channel->channel_client->client)) {
> > +        spice_debug("Do not set playback mute during migration");
> > +        return;
> > +    }
> >      snd_playback_send_mute(playback_channel);
> >  }
> >
> > @@ -1270,6 +1278,10 @@ SPICE_GNUC_VISIBLE void
> > spice_server_record_set_volume(SpiceRecordInstance *sin,
> >      if (!channel || nchannels == 0)
> >          return;
> >
> > +    if
> > (red_client_during_migrate_at_target(channel->channel_client->client)) {
> > +        spice_debug("Do not set record volume during migration");
> > +        return;
> > +    }
> >      snd_record_send_volume(record_channel);
> >  }
> >
> > @@ -1284,6 +1296,10 @@ SPICE_GNUC_VISIBLE void
> > spice_server_record_set_mute(SpiceRecordInstance *sin, u
> >      if (!channel)
> >          return;
> >
> > +    if
> > (red_client_during_migrate_at_target(channel->channel_client->client)) {
> > +        spice_debug("Do not set record mute during migration");
> > +        return;
> > +    }
> >      snd_record_send_mute(record_channel);
> >  }
> >
> > --
> > 2.1.0
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
> 
> 
> 
> -- 
> Marc-André Lureau