> > On 11/25/2015 02:23 PM, Frediano Ziglio wrote: > >> > >> On Mon, 2015-11-23 at 17:01 +0000, Frediano Ziglio wrote: > >>> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx> > >>> > >>> Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx> > >> > >> As I said in my previous review, I think that using g_return_if_fail() > >> here > >> is > >> appropriate. > >> > >> > > > > Yes, actually the only caller ignore the result so false or true is the > > same. > > Also there is the same check outside so this path is never taken. > > Basically the code is handling messages for migration till migration is > > ended. > > red_channel_client_waits_for_migrate_data just returns if we are in a > > migration > > (TRUE) or not (FALSE). > > > >> > >>> --- > >>> server/display-channel.c | 4 ++-- > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/server/display-channel.c b/server/display-channel.c > >>> index 5e75019..a178cc9 100644 > >>> --- a/server/display-channel.c > >>> +++ b/server/display-channel.c > >>> @@ -861,10 +861,10 @@ int > >>> display_channel_wait_for_migrate_data(DisplayChannel > >>> *display) > >>> RedChannelClient *rcc; > >>> > >>> spice_debug(NULL); > >>> - spice_assert(channel->clients_num == 1); > >>> + spice_warn_if_fail(channel->clients_num == 1); > >>> > > > > I agree with this warning but here I'd like to see a TODO also. > > > > I won't kill the VM just because we have a single client. > > This line verifies that there is only a single client. > In other words, that migration with multi-client is not-supported, > and also if we got here there must be a client connected. > > > I don't think is so easy here to handle multiple clients and by the > > way is not supported. > > I agree, the code does not handle it yet. > > > > > The reason for the warning... > > > >>> rcc = SPICE_CONTAINEROF(ring_get_head(&channel->clients), > >>> RedChannelClient, channel_link); > > > > Is this! Only the first client is considered! > > The first and only client is considered. > > regards, > Uri. > > Is this a NACK or not? Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel