Re: [PATCH 13/18] display: replace some dubious asserts

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

 



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.

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]