qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH spice-server 08/13] server: move the linking of


From: Yonit Halperin
Subject: Re: [Qemu-devel] [PATCH spice-server 08/13] server: move the linking of channels to a separate routine
Date: Sun, 25 Sep 2011 12:13:50 +0300
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.9) Gecko/20100921 Fedora/3.1.4-1.fc14 Thunderbird/3.1.4

On 09/22/2011 05:01 PM, Alon Levy wrote:
On Wed, Sep 21, 2011 at 06:51:18PM +0300, Yonit Halperin wrote:

Signed-off-by: Yonit Halperin<address@hidden>
---
  server/reds.c |   68 +++++++++++++++++++++++++++++++++++---------------------
  1 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index bea0eb0..e7388a0 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2612,12 +2612,48 @@ static void inputs_init()
      reds_register_channel(channel);
  }

+static void reds_send_input_channel_insecure_warn()
+{
+    RedsOutItem *item;
+    SpiceMsgNotify notify;
+    char *mess = "keyboard channel is insecure";
+    const int mess_len = strlen(mess);
+
+    item = new_out_item(SPICE_MSG_NOTIFY);
+
+    notify.time_stamp = get_time_stamp();
+    notify.severity = SPICE_NOTIFY_SEVERITY_WARN;
+    notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH;
+    notify.what = SPICE_WARN_GENERAL;
+    notify.message_len = mess_len;
+
+    spice_marshall_msg_notify(item->m,&notify);
+    spice_marshaller_add(item->m, (uint8_t *)mess, mess_len + 1);
+
+    reds_push_pipe_item(item);
+}
+
+static void reds_channel_do_link(Channel *channel, SpiceLinkMess *link_msg, 
RedsStream *stream)
+{
+    uint32_t *caps;
+
+    ASSERT(channel);
+    ASSERT(link_msg);
+    ASSERT(stream);
+
+    if (link_msg->channel_type == SPICE_CHANNEL_INPUTS&&  !stream->ssl) {
+        reds_send_input_channel_insecure_warn();
+    }
+    caps = (uint32_t *)((uint8_t *)link_msg + link_msg->caps_offset);
+    channel->link(channel, stream, FALSE, link_msg->num_common_caps,
+                  link_msg->num_common_caps ? caps : NULL, 
link_msg->num_channel_caps,
+                  link_msg->num_channel_caps ? caps + 
link_msg->num_common_caps : NULL);
+}
+
  static void reds_handle_other_links(RedLinkInfo *link)
  {
      Channel *channel;
-    RedsStream *stream;
      SpiceLinkMess *link_mess;
-    uint32_t *caps;

      link_mess = link->link_mess;

@@ -2635,36 +2671,16 @@ static void reds_handle_other_links(RedLinkInfo *link)
      }

      reds_send_link_result(link, SPICE_LINK_ERR_OK);
+    // TODO: if mig_target, where this call should be? here or only when the 
links are handled
Both? add a prefix argument to reds_show_new_channel maybe.

Hi, my concern was more about the red_channel_event called from reds_show_new_channel. I think it should be called only once, and at the same place it is in these patches - when the client makes the initial connection to the target, i.e., before migration.
      reds_show_new_channel(link, reds->link_id);
-    if (link_mess->channel_type == SPICE_CHANNEL_INPUTS&&  !link->stream->ssl) 
{
-        RedsOutItem *item;
-        SpiceMsgNotify notify;
-        char *mess = "keyboard channel is insecure";
-        const int mess_len = strlen(mess);
-
-        item = new_out_item(SPICE_MSG_NOTIFY);
-
-        notify.time_stamp = get_time_stamp();
-        notify.severity = SPICE_NOTIFY_SEVERITY_WARN;
-        notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH;
-        notify.what = SPICE_WARN_GENERAL;
-        notify.message_len = mess_len;
+    reds_stream_remove_watch(link->stream);

-        spice_marshall_msg_notify(item->m,&notify);
-        spice_marshaller_add(item->m, (uint8_t *)mess, mess_len + 1);
+    reds_channel_do_link(channel, link->link_mess, link->stream);

Would be nice to consistently use link_mess or link->link_mess, not both.

+    free(link_mess);

-        reds_push_pipe_item(item);
-    }
-    stream = link->stream;
-    reds_stream_remove_watch(stream);
      link->stream = NULL;
      link->link_mess = NULL;
      reds_link_free(link);
-    caps = (uint32_t *)((uint8_t *)link_mess + link_mess->caps_offset);
-    channel->link(channel, stream, reds->mig_target, 
link_mess->num_common_caps,
-                  link_mess->num_common_caps ? caps : NULL, 
link_mess->num_channel_caps,
-                  link_mess->num_channel_caps ? caps + 
link_mess->num_common_caps : NULL);
-    free(link_mess);
  }

  static void reds_handle_link(RedLinkInfo *link)
--
1.7.4.4





reply via email to

[Prev in Thread] Current Thread [Next in Thread]