[Spice-devel] [RFC v4 20/62] server/main_channel: support multiple clients
Alon Levy
alevy at redhat.com
Tue May 3 00:21:28 PDT 2011
On Tue, May 03, 2011 at 01:52:31AM +0200, Marc-André Lureau wrote:
> 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?
But the _ref part here is done in one go - I set the refs to the number
of clients (which is known at pipe item creation time).
The RefsPipeItem is badly named - I assumed I'd use that in more places,
but it ends up just being a SwitchHostPipeItem. The purpose of the refs
is to have a "item sent to all clients" event, as you can see in the free
function (--ref, and only if 0 then do something). I should probably just
make this a SwitchHostPipeItem and comment on the refs.
I think using glib / gobject is a good idea, just don't see how it helps
this particular case (because of the assymetry in reference set once and
reference remove multiple times).
>
> 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.
Ok, I thought the structs were easier, but I can remove those.
Regarding the RCC / RC argument, I think it's there as a "just in case", I
don't remember if it's actually used.
>
> 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)
>
It used to be used through red_channel_pipes_new_add_push but I see now
that it isn't any more, so I'll drop those num's where I can.
> Could we have just one fonction with a "bool broadcast" instead?
Or I could do this.
>
> > +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?
Yes, I'll make that a struct.
>
> > 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