[Spice-devel] [RFC v4 20/62] server/main_channel: support multiple clients
Marc-André Lureau
marcandre.lureau at gmail.com
Mon May 2 16:52:31 PDT 2011
The ref system is not clear, why is it heap allocated? Some
documentation about that Refs type would help.
Having explicit _ref() and _unref() methods would make things more
clear. Can it be pushed higher in the object hierarchy instead of
having new Refs types?
I am a bit skeptical reading:
switch (base_item->type)
....
case SPICE_MSG_MAIN_AGENT_DATA:
AgentDataPipeItem *data = (AgentDataPipeItem*)base;
if (!--data->refs->refs)
....
case SPICE_MSG_MAIN_MIGRATE_SWITCH_HOST:
RefsPipeItem *data = (RefsPipeItem*)base;
if (!--*(data->refs)) {
Passing item creation arguments into Info structs doesn't make things
easier to read, and duplicate code (more assignments, more type
declaration with similar fields, more things to instanciate...). It
should be possible to simplify PipeItem creation, for example, I don't
see why we are passing RedChannelClient or RedChannel.
There is many unused "num" arguments.
On Tue, Apr 26, 2011 at 12:54 PM, Alon Levy <alevy at redhat.com> wrote:
> Notable TODOs:
> * migration testing
Simple migration seems to work.
> * agent tokens are wrongly sent (or did I fix that? check)
In which case? I have been doing basic tests with a single client, it
seems to work fine.
> ---
> server/main_channel.c | 392 ++++++++++++++++++++++++++++---------------------
> server/main_channel.h | 8 +-
> server/reds.c | 7 +-
> server/reds.h | 2 +-
> 4 files changed, 231 insertions(+), 178 deletions(-)
>
> diff --git a/server/main_channel.c b/server/main_channel.c
> index 8ba0978..74c7f40 100644
> --- a/server/main_channel.c
> +++ b/server/main_channel.c
> @@ -61,6 +61,11 @@ struct RedsOutItem {
> PipeItem base;
> };
>
> +typedef struct RefsPipeItem {
> + PipeItem base;
> + int *refs;
> +} RefsPipeItem;
> +
> typedef struct PingPipeItem {
> PipeItem base;
> int size;
> @@ -77,8 +82,13 @@ typedef struct TokensPipeItem {
> int tokens;
> } TokensPipeItem;
>
> +typedef struct AgentDataPipeItemRefs {
> + int refs;
> +} AgentDataPipeItemRefs;
> +
> typedef struct AgentDataPipeItem {
> PipeItem base;
> + AgentDataPipeItemRefs *refs;
> uint8_t* data;
> size_t len;
> spice_marshaller_item_free_func free_data;
> @@ -152,25 +162,27 @@ static void main_disconnect(MainChannel *main_chan)
> red_channel_destroy(&main_chan->base);
> }
>
> +#define MAIN_FOREACH(_link, _main, _mcc) \
> + if ((_main) && ((_mcc) = \
> + SPICE_CONTAINEROF((_main)->base.rcc, MainChannelClient, base)))
Used only once, perhaps we don't need a macro for a single usage.
_main != NULL should be checked before,
_link is unused
> RedClient *main_channel_get_client_by_link_id(MainChannel *main_chan, uint32_t link_id)
> {
> MainChannelClient *mcc;
>
> - if (!main_chan || !main_chan->base.rcc) {
> - return NULL;
> - }
> - mcc = SPICE_CONTAINEROF(main_chan->base.rcc, MainChannelClient, base);
> - if (mcc->link_id == link_id) {
> - return mcc->base.client;
> + MAIN_FOREACH(link, main_chan, mcc) {
> + if (mcc->link_id == link_id) {
> + return mcc->base.client;
> + }
> }
> return NULL;
> }
>
> -static int main_channel_client_push_ping(MainChannelClient *rcc, int size);
> +static int main_channel_client_push_ping(MainChannelClient *mcc, int size);
>
> -void main_channel_start_net_test(MainChannelClient *mcc)
> +void main_channel_client_start_net_test(MainChannelClient *mcc)
> {
> - if (!mcc) {
> + if (!mcc || mcc->net_test_id) {
> return;
> }
> if (main_channel_client_push_ping(mcc, NET_TEST_WARMUP_BYTES)
> @@ -181,67 +193,89 @@ void main_channel_start_net_test(MainChannelClient *mcc)
> }
> }
>
> -static RedsOutItem *main_pipe_item_new(RedChannelClient *rcc, int type)
> +static PipeItem *main_pipe_item_new_broadcast(RedChannelClient *rcc, void *data, int num)
> +{
> + RedsOutItem *item = spice_malloc(sizeof(RedsOutItem));
> +
> + red_channel_pipe_item_init(rcc->channel, &item->base, (uint64_t)data);
> + return &item->base;
> +}
> +
so what is num for?
Why do we have
main_pipe_item_new() with: int type
main_pipe_item_new_broadcast() with: void *data that is cast to (uint64_t)
Could we have just one fonction with a "bool broadcast" instead?
> +static PipeItem *main_pipe_item_new(MainChannelClient *mcc, int type)
> {
> RedsOutItem *item = spice_malloc(sizeof(RedsOutItem));
>
> - red_channel_pipe_item_init(rcc->channel, &item->base, type);
> - return item;
> + red_channel_pipe_item_init(mcc->base.channel, &item->base, type);
> + return &item->base;
> }
>
> -static MouseModePipeItem *main_mouse_mode_item_new(RedChannelClient *rcc,
> - int current_mode, int is_client_mouse_allowed)
> +typedef struct MainMouseModeItemInfo {
> + int current_mode;
> + int is_client_mouse_allowed;
> +} MainMouseModeItemInfo;
> +
> +static PipeItem *main_mouse_mode_item_new(RedChannelClient *rcc, void *data, int num)
> {
> MouseModePipeItem *item = spice_malloc(sizeof(MouseModePipeItem));
> + MainMouseModeItemInfo *info = data;
>
> red_channel_pipe_item_init(rcc->channel, &item->base,
> SPICE_MSG_MAIN_MOUSE_MODE);
> - item->current_mode = current_mode;
> - item->is_client_mouse_allowed = is_client_mouse_allowed;
> - return item;
> + item->current_mode = info->current_mode;
> + item->is_client_mouse_allowed = info->is_client_mouse_allowed;
> + return &item->base;
> }
>
> -static PingPipeItem *main_ping_item_new(RedChannelClient *rcc, int size)
> +static PipeItem *main_ping_item_new(MainChannelClient *mcc, int size)
> {
> PingPipeItem *item = spice_malloc(sizeof(PingPipeItem));
>
> - red_channel_pipe_item_init(rcc->channel, &item->base, SPICE_MSG_PING);
> + red_channel_pipe_item_init(mcc->base.channel, &item->base, SPICE_MSG_PING);
> item->size = size;
> - return item;
> + return &item->base;
> }
>
> -static TokensPipeItem *main_tokens_item_new(RedChannelClient *rcc, int tokens)
> +static PipeItem *main_tokens_item_new(RedChannelClient *rcc, void *data, int num)
> {
> TokensPipeItem *item = spice_malloc(sizeof(TokensPipeItem));
>
> red_channel_pipe_item_init(rcc->channel, &item->base,
> SPICE_MSG_MAIN_AGENT_TOKEN);
> - item->tokens = tokens;
> - return item;
> + item->tokens = (uint64_t)data;
> + return &item->base;
> }
>
> -static AgentDataPipeItem *main_agent_data_item_new(RedChannelClient *rcc,
> - uint8_t* data, size_t len,
> - spice_marshaller_item_free_func free_data, void *opaque)
> +typedef struct MainAgentDataItemInfo {
> + uint8_t* data;
> + size_t len;
> + spice_marshaller_item_free_func free_data;
> + void *opaque;
> + AgentDataPipeItemRefs *refs;
> +} MainAgentDataItemInfo;
> +
> +static PipeItem *main_agent_data_item_new(RedChannelClient *rcc, void *data, int num)
> {
> + MainAgentDataItemInfo *info = data;
> AgentDataPipeItem *item = spice_malloc(sizeof(AgentDataPipeItem));
>
> - red_channel_pipe_item_init(rcc->channel, &item->base, SPICE_MSG_MAIN_AGENT_DATA);
> - item->data = data;
> - item->len = len;
> - item->free_data = free_data;
> - item->opaque = opaque;
> - return item;
> + red_channel_pipe_item_init(rcc->channel, &item->base,
> + SPICE_MSG_MAIN_AGENT_DATA);
> + item->refs = info->refs;
> + item->data = info->data;
> + item->len = info->len;
> + item->free_data = info->free_data;
> + item->opaque = info->opaque;
> + return &item->base;
> }
>
> -static InitPipeItem *main_init_item_new(RedChannelClient *rcc,
> +static PipeItem *main_init_item_new(MainChannelClient *mcc,
> int connection_id, int display_channels_hint, int current_mouse_mode,
> int is_client_mouse_allowed, int multi_media_time,
> int ram_hint)
> {
> InitPipeItem *item = spice_malloc(sizeof(InitPipeItem));
>
> - red_channel_pipe_item_init(rcc->channel, &item->base,
> + red_channel_pipe_item_init(mcc->base.channel, &item->base,
> SPICE_MSG_MAIN_INIT);
> item->connection_id = connection_id;
> item->display_channels_hint = display_channels_hint;
> @@ -249,57 +283,70 @@ static InitPipeItem *main_init_item_new(RedChannelClient *rcc,
> item->is_client_mouse_allowed = is_client_mouse_allowed;
> item->multi_media_time = multi_media_time;
> item->ram_hint = ram_hint;
> - return item;
> + return &item->base;
> }
>
> -static NotifyPipeItem *main_notify_item_new(RedChannelClient *rcc,
> - uint8_t *mess, const int mess_len)
> +typedef struct NotifyPipeInfo {
> + uint8_t *mess;
> + int mess_len;
> +} NotifyPipeInfo;
> +
> +static PipeItem *main_notify_item_new(RedChannelClient *rcc, void *data, int num)
> {
> NotifyPipeItem *item = spice_malloc(sizeof(NotifyPipeItem));
> + NotifyPipeInfo *info = data;
>
> red_channel_pipe_item_init(rcc->channel, &item->base,
> SPICE_MSG_NOTIFY);
> - item->mess = mess;
> - item->mess_len = mess_len;
> - return item;
> + item->mess = info->mess;
> + item->mess_len = info->mess_len;
> + return &item->base;
> }
>
> -static MigrateBeginPipeItem *main_migrate_begin_item_new(
> - RedChannelClient *rcc, int port, int sport,
> - char *host, uint16_t cert_pub_key_type, uint32_t cert_pub_key_len,
> - uint8_t *cert_pub_key)
> +typedef struct MigrateBeginItemInfo {
> + int port;
> + int sport;
> + char *host;
> + uint16_t cert_pub_key_type;
> + uint32_t cert_pub_key_len;
> + uint8_t *cert_pub_key;
> +} MigrateBeginItemInfo;
> +
> +// TODO: MC: migration is not tested at all with multiclient.
> +static PipeItem *main_migrate_begin_item_new(
> + RedChannelClient *rcc, void *data, int num)
> {
> MigrateBeginPipeItem *item = spice_malloc(sizeof(MigrateBeginPipeItem));
> + MigrateBeginItemInfo *info = data;
>
> red_channel_pipe_item_init(rcc->channel, &item->base,
> SPICE_MSG_MAIN_MIGRATE_BEGIN);
> - item->port = port;
> - item->sport = sport;
> - item->host = host;
> - item->cert_pub_key_type = cert_pub_key_type;
> - item->cert_pub_key_len = cert_pub_key_len;
> - item->cert_pub_key = cert_pub_key;
> - return item;
> + item->port = info->port;
> + item->sport = info->sport;
> + item->host = info->host;
> + item->cert_pub_key_type = info->cert_pub_key_type;
> + item->cert_pub_key_len = info->cert_pub_key_len;
> + item->cert_pub_key = info->cert_pub_key;
> + return &item->base;
> }
>
> -static MultiMediaTimePipeItem *main_multi_media_time_item_new(
> - RedChannelClient *rcc, int time)
> +static PipeItem *main_multi_media_time_item_new(
> + RedChannelClient *rcc, void *data, int num)
> {
> - MultiMediaTimePipeItem *item;
> + MultiMediaTimePipeItem *item, *info = data;
>
> item = spice_malloc(sizeof(MultiMediaTimePipeItem));
> red_channel_pipe_item_init(rcc->channel, &item->base,
> SPICE_MSG_MAIN_MULTI_MEDIA_TIME);
> - item->time = time;
> - return item;
> + item->time = info->time;
> + return &item->base;
> }
>
> -static void main_channel_push_channels(RedChannelClient *rcc)
> +static void main_channel_push_channels(MainChannelClient *mcc)
> {
> - RedsOutItem *item;
> + PipeItem *item = main_pipe_item_new(mcc, SPICE_MSG_MAIN_CHANNELS_LIST);
>
> - item = main_pipe_item_new(rcc, SPICE_MSG_MAIN_CHANNELS_LIST);
> - red_channel_client_pipe_add_push(rcc, &item->base);
> + red_channel_client_pipe_add_push(&mcc->base, item);
> }
>
> static void main_channel_marshall_channels(SpiceMarshaller *m)
> @@ -315,20 +362,14 @@ static void main_channel_marshall_channels(SpiceMarshaller *m)
>
> int main_channel_client_push_ping(MainChannelClient *mcc, int size)
> {
> - PingPipeItem *item;
> + PipeItem *item;
>
> - item = main_ping_item_new(&mcc->base, size);
> - red_channel_client_pipe_add_push(&mcc->base, &item->base);
> - return TRUE;
> -}
> -
> -int main_channel_push_ping(MainChannel *main_chan, int size)
> -{
> - if (main_chan->base.rcc == NULL) {
> + if (mcc == NULL) {
> return FALSE;
> }
> - return main_channel_client_push_ping(
> - SPICE_CONTAINEROF(main_chan->base.rcc, MainChannelClient, base), size);
> + item = main_ping_item_new(mcc, size);
> + red_channel_client_pipe_add_push(&mcc->base, item);
> + return TRUE;
> }
>
> static void main_channel_marshall_ping(SpiceMarshaller *m, int size, int ping_id)
> @@ -348,26 +389,16 @@ static void main_channel_marshall_ping(SpiceMarshaller *m, int size, int ping_id
> }
> }
>
> -static void main_channel_client_push_mouse_mode(RedChannelClient *rcc, int current_mode,
> - int is_client_mouse_allowed);
> -
> void main_channel_push_mouse_mode(MainChannel *main_chan, int current_mode,
> int is_client_mouse_allowed)
> {
> - if (main_chan && main_chan->base.rcc != NULL) {
> - main_channel_client_push_mouse_mode(main_chan->base.rcc, current_mode,
> - is_client_mouse_allowed);
> - }
> -}
> -
> -static void main_channel_client_push_mouse_mode(RedChannelClient *rcc, int current_mode,
> - int is_client_mouse_allowed)
> -{
> - MouseModePipeItem *item;
> + MainMouseModeItemInfo info = {
> + .current_mode=current_mode,
> + .is_client_mouse_allowed=is_client_mouse_allowed,
> + };
>
> - item = main_mouse_mode_item_new(rcc, current_mode,
> - is_client_mouse_allowed);
> - red_channel_client_pipe_add_push(rcc, &item->base);
> + red_channel_pipes_new_add_push(&main_chan->base,
> + main_mouse_mode_item_new, &info);
> }
>
> static void main_channel_marshall_mouse_mode(SpiceMarshaller *m, int current_mode, int is_client_mouse_allowed)
> @@ -383,23 +414,14 @@ static void main_channel_marshall_mouse_mode(SpiceMarshaller *m, int current_mod
>
> void main_channel_push_agent_connected(MainChannel *main_chan)
> {
> - RedsOutItem *item;
> - RedChannelClient *rcc = main_chan->base.rcc;
> -
> - item = main_pipe_item_new(rcc, SPICE_MSG_MAIN_AGENT_CONNECTED);
> - red_channel_client_pipe_add_push(rcc, &item->base);
> + red_channel_pipes_new_add_push(&main_chan->base,
> + main_pipe_item_new_broadcast, (void*)SPICE_MSG_MAIN_AGENT_CONNECTED);
> }
>
> void main_channel_push_agent_disconnected(MainChannel *main_chan)
> {
> - RedsOutItem *item;
> - RedChannelClient *rcc = main_chan->base.rcc;
> -
> - if (!rcc) {
> - return;
> - }
> - item = main_pipe_item_new(rcc, SPICE_MSG_MAIN_AGENT_DISCONNECTED);
> - red_channel_client_pipe_add_push(rcc, &item->base);
> + red_channel_pipes_new_add_push(&main_chan->base,
> + main_pipe_item_new_broadcast, (void*)SPICE_MSG_MAIN_AGENT_DISCONNECTED);
> }
>
> static void main_channel_marshall_agent_disconnected(SpiceMarshaller *m)
> @@ -410,16 +432,11 @@ static void main_channel_marshall_agent_disconnected(SpiceMarshaller *m)
> spice_marshall_msg_main_agent_disconnected(m, &disconnect);
> }
>
> +// TODO: make this targeted (requires change to agent token accounting)
> void main_channel_push_tokens(MainChannel *main_chan, uint32_t num_tokens)
> {
> - TokensPipeItem *item;
> - RedChannelClient *rcc = main_chan->base.rcc;
> -
> - if (!rcc) {
> - return;
> - }
> - item = main_tokens_item_new(rcc, num_tokens);
> - red_channel_client_pipe_add_push(rcc, &item->base);
> + red_channel_pipes_new_add_push(&main_chan->base,
> + main_tokens_item_new, (void*)(uint64_t)num_tokens);
> }
(void*)(uint64_t)num_tokens
Can we avoid writing 64bits specific code?
> static void main_channel_marshall_tokens(SpiceMarshaller *m, uint32_t num_tokens)
> @@ -433,30 +450,29 @@ static void main_channel_marshall_tokens(SpiceMarshaller *m, uint32_t num_tokens
> void main_channel_push_agent_data(MainChannel *main_chan, uint8_t* data, size_t len,
> spice_marshaller_item_free_func free_data, void *opaque)
> {
> - RedChannelClient *rcc = main_chan->base.rcc;
> - AgentDataPipeItem *item;
> + MainAgentDataItemInfo info = {
> + .data = data,
> + .len = len,
> + .free_data = free_data,
> + .opaque = opaque,
> + .refs = spice_malloc(sizeof(AgentDataPipeItemRefs)),
> + };
>
> - item = main_agent_data_item_new(rcc, data, len, free_data, opaque);
> - red_channel_client_pipe_add_push(rcc, &item->base);
> + info.refs->refs = main_chan->base.clients_num;
> + red_channel_pipes_new_add_push(&main_chan->base,
> + main_agent_data_item_new, &info);
> }
>
> static void main_channel_marshall_agent_data(SpiceMarshaller *m,
> AgentDataPipeItem *item)
> {
> - spice_marshaller_add_ref_full(m,
> - item->data, item->len, item->free_data, item->opaque);
> + spice_marshaller_add_ref(m, item->data, item->len);
> }
>
> static void main_channel_push_migrate_data_item(MainChannel *main_chan)
> {
> - RedsOutItem *item;
> - RedChannelClient *rcc = main_chan->base.rcc;
> -
> - if (!rcc) {
> - return;
> - }
> - item = main_pipe_item_new(rcc, SPICE_MSG_MIGRATE_DATA);
> - red_channel_client_pipe_add_push(rcc, &item->base);
> + red_channel_pipes_new_add_push(&main_chan->base,
> + main_pipe_item_new_broadcast, (void*)(uint64_t)SPICE_MSG_MIGRATE_DATA);
> }
>
> static void main_channel_marshall_migrate_data_item(SpiceMarshaller *m, int serial, int ping_id)
> @@ -468,8 +484,7 @@ static void main_channel_marshall_migrate_data_item(SpiceMarshaller *m, int seri
> data->ping_id = ping_id;
> }
>
> -static uint64_t main_channel_handle_migrate_data_get_serial_proc(
> - RedChannelClient *rcc,
> +static uint64_t main_channel_handle_migrate_data_get_serial(RedChannelClient *base,
> uint32_t size, void *message)
> {
> MainMigrateData *data = message;
> @@ -481,10 +496,10 @@ static uint64_t main_channel_handle_migrate_data_get_serial_proc(
> return data->serial;
> }
>
> -static uint64_t main_channel_handle_migrate_data(RedChannelClient *rcc,
> +static uint64_t main_channel_handle_migrate_data(RedChannelClient *base,
> uint32_t size, void *message)
> {
> - MainChannelClient *mcc = SPICE_CONTAINEROF(rcc, MainChannelClient, base);
> + MainChannelClient *mcc = SPICE_CONTAINEROF(base, MainChannelClient, base);
> MainMigrateData *data = message;
>
> if (size < sizeof(*data)) {
> @@ -501,18 +516,18 @@ void main_channel_push_init(MainChannelClient *mcc, int connection_id,
> int is_client_mouse_allowed, int multi_media_time,
> int ram_hint)
> {
> - InitPipeItem *item;
> + PipeItem *item;
>
> - item = main_init_item_new(&mcc->base,
> + item = main_init_item_new(mcc,
> connection_id, display_channels_hint, current_mouse_mode,
> is_client_mouse_allowed, multi_media_time, ram_hint);
> - red_channel_client_pipe_add_push(&mcc->base, &item->base);
> + red_channel_client_pipe_add_push(&mcc->base, item);
> }
>
> static void main_channel_marshall_init(SpiceMarshaller *m,
> InitPipeItem *item)
> {
> - SpiceMsgMainInit init;
> + SpiceMsgMainInit init; // TODO - remove this copy, make InitPipeItem reuse SpiceMsgMainInit
>
> init.session_id = item->connection_id;
> init.display_channels_hint = item->display_channels_hint;
> @@ -528,13 +543,16 @@ static void main_channel_marshall_init(SpiceMarshaller *m,
> spice_marshall_msg_main_init(m, &init);
> }
>
> +// TODO - some notifications are new client only (like "keyboard is insecure" on startup)
> void main_channel_push_notify(MainChannel *main_chan, uint8_t *mess, const int mess_len)
> {
> - RedChannelClient *rcc = main_chan->base.rcc;
> - NotifyPipeItem *item;
> -
> - item = main_notify_item_new(rcc, mess, mess_len);
> - red_channel_client_pipe_add_push(rcc, &item->base);
> + NotifyPipeInfo info = {
> + .mess = mess,
> + .mess_len = mess_len,
> + };
> +
> + red_channel_pipes_new_add_push(&main_chan->base,
> + main_notify_item_new, &info);
> }
>
> static void main_channel_marshall_notify(SpiceMarshaller *m, NotifyPipeItem *item)
> @@ -554,12 +572,17 @@ void main_channel_push_migrate_begin(MainChannel *main_chan, int port, int sport
> char *host, uint16_t cert_pub_key_type, uint32_t cert_pub_key_len,
> uint8_t *cert_pub_key)
> {
> - RedChannelClient *rcc = main_chan->base.rcc;
> - MigrateBeginPipeItem *item;
> -
> - item = main_migrate_begin_item_new(rcc, port,
> - sport, host, cert_pub_key_type, cert_pub_key_len, cert_pub_key);
> - red_channel_client_pipe_add_push(rcc, &item->base);
> + MigrateBeginItemInfo info = {
> + .port =port,
> + .sport = sport,
> + .host = host,
> + .cert_pub_key_type = cert_pub_key_type,
> + .cert_pub_key_len = cert_pub_key_len,
> + .cert_pub_key = cert_pub_key,
> + };
> +
> + red_channel_pipes_new_add_push(&main_chan->base,
> + main_migrate_begin_item_new, &info);
> }
>
> static void main_channel_marshall_migrate_begin(SpiceMarshaller *m,
> @@ -579,11 +602,8 @@ static void main_channel_marshall_migrate_begin(SpiceMarshaller *m,
>
> void main_channel_push_migrate(MainChannel *main_chan)
> {
> - RedChannelClient *rcc = main_chan->base.rcc;
> - RedsOutItem *item;
> -
> - item = main_pipe_item_new(rcc, SPICE_MSG_MIGRATE);
> - red_channel_client_pipe_add_push(rcc, &item->base);
> + red_channel_pipes_new_add_push(&main_chan->base,
> + main_pipe_item_new_broadcast, (void*)(uint64_t)SPICE_MSG_MIGRATE);
> }
>
> static void main_channel_marshall_migrate(SpiceMarshaller *m)
> @@ -596,37 +616,38 @@ static void main_channel_marshall_migrate(SpiceMarshaller *m)
>
> void main_channel_push_migrate_cancel(MainChannel *main_chan)
> {
> - RedChannelClient *rcc = main_chan->base.rcc;
> - RedsOutItem *item;
> -
> - item = main_pipe_item_new(rcc, SPICE_MSG_MAIN_MIGRATE_CANCEL);
> - red_channel_client_pipe_add_push(rcc, &item->base);
> + red_channel_pipes_new_add_push(&main_chan->base,
> + main_pipe_item_new_broadcast, (void*)SPICE_MSG_MAIN_MIGRATE_CANCEL);
> }
>
> void main_channel_push_multi_media_time(MainChannel *main_chan, int time)
> {
> - RedChannelClient *rcc = main_chan->base.rcc;
> - MultiMediaTimePipeItem *item;
> + MultiMediaTimePipeItem info = {
> + .time = time,
> + };
>
> - item =main_multi_media_time_item_new(rcc, time);
> - red_channel_client_pipe_add_push(rcc, &item->base);
> + red_channel_pipes_new_add_push(&main_chan->base,
> + main_multi_media_time_item_new, &info);
> }
>
> -static PipeItem *main_migrate_switch_item_new(MainChannel *main_chan)
> +static PipeItem *main_migrate_switch_item_new(RedChannelClient *rcc,
> + void *data, int num)
> {
> - PipeItem *item = spice_malloc(sizeof(*item));
> + RefsPipeItem *item = spice_malloc(sizeof(*item));
>
> - red_channel_pipe_item_init(&main_chan->base, item,
> + item->refs = data;
> + red_channel_pipe_item_init(rcc->channel, &item->base,
> SPICE_MSG_MAIN_MIGRATE_SWITCH_HOST);
> - return item;
> + return &item->base;
> }
What is num for?
> void main_channel_push_migrate_switch(MainChannel *main_chan)
> {
> - RedChannelClient *rcc = main_chan->base.rcc;
> + int *refs = spice_malloc0(sizeof(int));
>
> - red_channel_client_pipe_add_push(rcc,
> - main_migrate_switch_item_new(main_chan));
> + *refs = main_chan->base.clients_num;
> + red_channel_pipes_new_add_push(&main_chan->base,
> + main_migrate_switch_item_new, (void*)refs);
> }
>
> static void main_channel_marshall_migrate_switch(SpiceMarshaller *m)
> @@ -637,8 +658,6 @@ static void main_channel_marshall_migrate_switch(SpiceMarshaller *m)
>
> reds_fill_mig_switch(&migrate);
> spice_marshall_msg_main_migrate_switch_host(m, &migrate);
> -
> - reds_mig_release();
> }
>
> static void main_channel_marshall_multi_media_time(SpiceMarshaller *m,
> @@ -717,6 +736,27 @@ static void main_channel_send_item(RedChannelClient *rcc, PipeItem *base)
> static void main_channel_release_pipe_item(RedChannelClient *rcc,
> PipeItem *base, int item_pushed)
> {
> + switch (base->type) {
> + case SPICE_MSG_MAIN_AGENT_DATA: {
> + AgentDataPipeItem *data = (AgentDataPipeItem*)base;
s/data/item
> + if (!--data->refs->refs) {
> + red_printf("SPICE_MSG_MAIN_AGENT_DATA %p %p, %d", data, data->refs, data->refs->refs);
> + free(data->refs);
> + data->data, data->opaque);
> + }
> + break;
> + }
> + case SPICE_MSG_MAIN_MIGRATE_SWITCH_HOST: {
> + RefsPipeItem *data = (RefsPipeItem*)base;
> + if (!--*(data->refs)) {
> + free(data->refs);
s/data/item
but we don't call data->free_data() here?
> + reds_mig_release();
> + }
> + break;
> + }
> + default:
> + break;
> + }
> free(base);
> }
>
> @@ -731,16 +771,16 @@ static int main_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint
> if (!main_chan) {
> return FALSE;
> }
> - reds_on_main_agent_start(main_chan);
> + reds_on_main_agent_start(mcc, main_chan);
> break;
> case SPICE_MSGC_MAIN_AGENT_DATA: {
> - reds_on_main_agent_data(message, size);
> + reds_on_main_agent_data(mcc, message, size);
> break;
> }
> case SPICE_MSGC_MAIN_AGENT_TOKEN:
> break;
> case SPICE_MSGC_MAIN_ATTACH_CHANNELS:
> - main_channel_push_channels(rcc);
> + main_channel_push_channels(mcc);
> break;
> case SPICE_MSGC_MAIN_MIGRATE_CONNECTED:
> red_printf("connected");
> @@ -781,7 +821,8 @@ static int main_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint
> "bandwidth", mcc->latency, roundtrip);
> break;
> }
> - mcc->bitrate_per_sec = (uint64_t)(NET_TEST_BYTES * 8) * 1000000 / (roundtrip - mcc->latency);
> + mcc->bitrate_per_sec = (uint64_t)(NET_TEST_BYTES * 8) * 1000000
> + / (roundtrip - mcc->latency);
> red_printf("net test: latency %f ms, bitrate %lu bps (%f Mbps)%s",
> (double)mcc->latency / 1000,
> mcc->bitrate_per_sec,
> @@ -816,6 +857,7 @@ static int main_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint
>
> static void main_channel_on_error(RedChannelClient *rcc)
> {
> + red_printf("");
> reds_client_disconnect(rcc->client);
> }
>
> @@ -840,7 +882,7 @@ static void main_channel_hold_pipe_item(RedChannelClient *rcc, PipeItem *item)
> {
> }
>
> -static int main_channel_handle_migrate_flush_mark_proc(RedChannelClient *rcc)
> +static int main_channel_handle_migrate_flush_mark(RedChannelClient *rcc)
> {
> main_channel_push_migrate_data_item(SPICE_CONTAINEROF(rcc->channel,
> MainChannel, base));
> @@ -880,6 +922,11 @@ static void ping_timer_cb(void *opaque)
> }
> #endif /* RED_STATISTICS */
>
> +uint32_t main_channel_client_get_link_id(MainChannelClient *mcc)
> +{
> + return mcc->link_id;
> +}
> +
> MainChannelClient *main_channel_client_create(MainChannel *main_chan,
> RedClient *client, RedsStream *stream, uint32_t link_id)
> {
> @@ -919,11 +966,14 @@ MainChannelClient *main_channel_link(Channel *channel, RedClient *client,
> ,main_channel_release_pipe_item
> ,main_channel_on_error
> ,main_channel_on_error
> - ,main_channel_handle_migrate_flush_mark_proc
> + ,main_channel_handle_migrate_flush_mark
> ,main_channel_handle_migrate_data
> - ,main_channel_handle_migrate_data_get_serial_proc);
> + ,main_channel_handle_migrate_data_get_serial);
> ASSERT(channel->data);
> }
> + // TODO - migration - I removed it from channel creation, now put it
> + // into usage somewhere (not an issue until we return migration to it's
> + // former glory)
> red_printf("add main channel client");
> mcc = main_channel_client_create(channel->data, client, stream, link_id);
> return mcc;
> diff --git a/server/main_channel.h b/server/main_channel.h
> index eb1ee04..a294a2f 100644
> --- a/server/main_channel.h
> +++ b/server/main_channel.h
> @@ -21,6 +21,7 @@
> #include <stdint.h>
> #include <spice/vd_agent.h>
> #include "common/marshaller.h"
> +#include "reds.h"
> #include "red_channel.h"
>
> /* This is a temporary measure for reds/main split - should not be in a header,
> @@ -45,23 +46,21 @@ struct MainMigrateData {
> };
>
> typedef struct MainChannel MainChannel;
> -typedef struct MainChannelClient MainChannelClient;
>
> Channel *main_channel_init();
> RedClient *main_channel_get_client_by_link_id(MainChannel *main_chan, uint32_t link_id);
> -/* This is a 'clone' from the reds.h Channel.link callback */
> +/* This is a 'clone' from the reds.h Channel.link callback to allow passing link_id */
> MainChannelClient *main_channel_link(struct Channel *, RedClient *client,
> RedsStream *stream, uint32_t link_id, int migration, int num_common_caps,
> uint32_t *common_caps, int num_caps, uint32_t *caps);
> void main_channel_close(MainChannel *main_chan); // not destroy, just socket close
> -int main_channel_push_ping(MainChannel *main_chan, int size);
> void main_channel_push_mouse_mode(MainChannel *main_chan, int current_mode, int is_client_mouse_allowed);
> void main_channel_push_agent_connected(MainChannel *main_chan);
> void main_channel_push_agent_disconnected(MainChannel *main_chan);
> void main_channel_push_tokens(MainChannel *main_chan, uint32_t num_tokens);
> void main_channel_push_agent_data(MainChannel *main_chan, uint8_t* data, size_t len,
> spice_marshaller_item_free_func free_data, void *opaque);
> -void main_channel_start_net_test(MainChannelClient *mcc);
> +void main_channel_client_start_net_test(MainChannelClient *mcc);
> // TODO: huge. Consider making a reds_* interface for these functions
> // and calling from main.
> void main_channel_push_init(MainChannelClient *mcc, int connection_id, int display_channels_hint,
> @@ -77,6 +76,7 @@ void main_channel_push_migrate_cancel(MainChannel *main_chan);
> void main_channel_push_multi_media_time(MainChannel *main_chan, int time);
> int main_channel_getsockname(MainChannel *main_chan, struct sockaddr *sa, socklen_t *salen);
> int main_channel_getpeername(MainChannel *main_chan, struct sockaddr *sa, socklen_t *salen);
> +uint32_t main_channel_client_get_link_id(MainChannelClient *mcc);
>
> int main_channel_client_is_low_bandwidth(MainChannelClient *mcc);
> uint64_t main_channel_client_get_bitrate_per_sec(MainChannelClient *mcc);
> diff --git a/server/reds.c b/server/reds.c
> index 413b348..0ce6f1c 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -982,8 +982,10 @@ void reds_on_main_agent_start()
> reds->agent_state.write_filter.discard_all = FALSE;
> }
>
> -void reds_on_main_agent_data(void *message, size_t size)
> +void reds_on_main_agent_data(MainChannelClient *mcc, void *message, size_t size)
> {
> + // TODO - use mcc (and start tracking agent data per channel. probably just move the whole
> + // tokens accounting to mainchannel.
> RingItem *ring_item;
> VDAgentExtBuf *buf;
> int res;
> @@ -1560,7 +1562,7 @@ static void reds_handle_main_link(RedLinkInfo *link)
> reds_get_mm_time() - MM_TIME_DELTA,
> red_dispatcher_qxl_ram_size());
>
> - main_channel_start_net_test(mcc);
> + main_channel_client_start_net_test(mcc);
> /* Now that we have a client, forward any pending agent data */
> while (read_from_vdi_port());
> }
> @@ -3099,6 +3101,7 @@ void reds_mig_switch(void)
> void reds_fill_mig_switch(SpiceMsgMainMigrationSwitchHost *migrate)
> {
> RedsMigSpice *s = reds->mig_spice;
> +
> migrate->port = s->port;
> migrate->sport = s->sport;
> migrate->host_size = strlen(s->host) + 1;
> diff --git a/server/reds.h b/server/reds.h
> index 8eea164..5698c22 100644
> --- a/server/reds.h
> +++ b/server/reds.h
> @@ -152,7 +152,7 @@ void reds_update_stat_value(uint32_t value);
>
> // callbacks from main channel messages
> void reds_on_main_agent_start();
> -void reds_on_main_agent_data(void *message, size_t size);
> +void reds_on_main_agent_data(MainChannelClient *mcc, void *message, size_t size);
> void reds_on_main_migrate_connected();
> void reds_on_main_migrate_connect_error();
> void reds_on_main_receive_migrate_data(MainMigrateData *data, uint8_t *end);
> --
> 1.7.4.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
--
Marc-André Lureau
More information about the Spice-devel
mailing list