[Spice-devel] [PATCH 03/24] server/red_channel: add red_channel_pipe_add_push
Alon Levy
alevy at redhat.com
Wed Feb 2 13:03:48 PST 2011
On Wed, Feb 02, 2011 at 07:50:01PM +0100, Marc-André Lureau wrote:
> What is the context? Why do you need to call red_channel_push()
> immediately here?
>
> Why not calling red_channel_pipe_add() from
> red_channel_pipe_add_push() to avoid logic duplication?
>
First, I am trying to not change any of the red_worker logic. It
already has some places where it adds but doesn't push, and red_channel
didn't have this distinction, so I introduce it in this patch.
Second, this could possibly be used later - if you add multiple messages
in a single batch. That said I have a separate patch that implements
this generally for purpose of aggregation (don't send unless 1000 bytes
ready or 10 ms have passed since last request to send), which won't need
this.
So basically, my first reason.
> On Wed, Jan 19, 2011 at 7:07 PM, Alon Levy <alevy at redhat.com> wrote:
> > ---
> > server/inputs_channel.c | 8 ++++----
> > server/main_channel.c | 28 ++++++++++++++--------------
> > server/red_channel.c | 7 +++++++
> > server/red_channel.h | 1 +
> > server/smartcard.c | 10 +++++-----
> > 5 files changed, 31 insertions(+), 23 deletions(-)
> >
> > diff --git a/server/inputs_channel.c b/server/inputs_channel.c
> > index f9777ed..2f3909e 100644
> > --- a/server/inputs_channel.c
> > +++ b/server/inputs_channel.c
> > @@ -243,7 +243,7 @@ static void inputs_pipe_add_type(InputsChannel *channel, int type)
> > {
> > InputsPipeItem* pipe_item = inputs_pipe_item_new(channel, type);
> >
> > - red_channel_pipe_add(&channel->base, &pipe_item->base);
> > + red_channel_pipe_add_push(&channel->base, &pipe_item->base);
> > }
> >
> > static void inputs_channel_release_pipe_item(RedChannel *channel,
> > @@ -476,7 +476,7 @@ static void inputs_migrate(Channel *channel)
> >
> > ASSERT(g_inputs_channel == (InputsChannel *)channel->data);
> > item = inputs_pipe_item_new(inputs_channel, PIPE_ITEM_MIGRATE);
> > - red_channel_pipe_add(&inputs_channel->base, &item->base);
> > + red_channel_pipe_add_push(&inputs_channel->base, &item->base);
> > }
> >
> > static void inputs_pipe_add_init(InputsChannel *inputs_channel)
> > @@ -486,7 +486,7 @@ static void inputs_pipe_add_init(InputsChannel *inputs_channel)
> > red_channel_pipe_item_init(&inputs_channel->base, &item->base,
> > PIPE_ITEM_INIT);
> > item->modifiers = kbd_get_leds(keyboard);
> > - red_channel_pipe_add(&inputs_channel->base, &item->base);
> > + red_channel_pipe_add_push(&inputs_channel->base, &item->base);
> > }
> >
> > static int inputs_channel_config_socket(RedChannel *channel)
> > @@ -545,7 +545,7 @@ static void inputs_push_keyboard_modifiers(uint8_t modifiers)
> > return;
> > }
> > item = inputs_key_modifiers_item_new(g_inputs_channel, modifiers);
> > - red_channel_pipe_add(&g_inputs_channel->base, &item->base);
> > + red_channel_pipe_add_push(&g_inputs_channel->base, &item->base);
> > }
> >
> > void inputs_on_keyboard_leds_change(void *opaque, uint8_t leds)
> > diff --git a/server/main_channel.c b/server/main_channel.c
> > index 72ba460..bb0e019 100644
> > --- a/server/main_channel.c
> > +++ b/server/main_channel.c
> > @@ -277,7 +277,7 @@ static void main_channel_push_channels(MainChannel *main_chan)
> > RedsOutItem *item;
> >
> > item = main_pipe_item_new(main_chan, SPICE_MSG_MAIN_CHANNELS_LIST);
> > - red_channel_pipe_add(&main_chan->base, &item->base);
> > + red_channel_pipe_add_push(&main_chan->base, &item->base);
> > }
> >
> > static void main_channel_marshall_channels(MainChannel *main_chan)
> > @@ -301,7 +301,7 @@ int main_channel_push_ping(Channel *channel, int size)
> > return FALSE;
> > }
> > item = main_ping_item_new(main_chan, size);
> > - red_channel_pipe_add(&main_chan->base, &item->base);
> > + red_channel_pipe_add_push(&main_chan->base, &item->base);
> > return TRUE;
> > }
> >
> > @@ -331,7 +331,7 @@ void main_channel_push_mouse_mode(Channel *channel, int current_mode,
> >
> > item = main_mouse_mode_item_new(main_chan, current_mode,
> > is_client_mouse_allowed);
> > - red_channel_pipe_add(&main_chan->base, &item->base);
> > + red_channel_pipe_add_push(&main_chan->base, &item->base);
> > }
> >
> > static void main_channel_marshall_mouse_mode(MainChannel *main_chan, int current_mode, int is_client_mouse_allowed)
> > @@ -352,7 +352,7 @@ void main_channel_push_agent_connected(Channel *channel)
> > MainChannel *main_chan = channel->data;
> >
> > item = main_pipe_item_new(main_chan, SPICE_MSG_MAIN_AGENT_CONNECTED);
> > - red_channel_pipe_add(&main_chan->base, &item->base);
> > + red_channel_pipe_add_push(&main_chan->base, &item->base);
> > }
> >
> > void main_channel_push_agent_disconnected(Channel *channel)
> > @@ -361,7 +361,7 @@ void main_channel_push_agent_disconnected(Channel *channel)
> > MainChannel *main_chan = channel->data;
> >
> > item = main_pipe_item_new(main_chan, SPICE_MSG_MAIN_AGENT_DISCONNECTED);
> > - red_channel_pipe_add(&main_chan->base, &item->base);
> > + red_channel_pipe_add_push(&main_chan->base, &item->base);
> > }
> >
> > static void main_channel_marshall_agent_disconnected(MainChannel *main_chan)
> > @@ -378,7 +378,7 @@ void main_channel_push_tokens(Channel *channel, uint32_t num_tokens)
> > MainChannel *main_chan = channel->data;
> > TokensPipeItem *item = main_tokens_item_new(main_chan, num_tokens);
> >
> > - red_channel_pipe_add(&main_chan->base, &item->base);
> > + red_channel_pipe_add_push(&main_chan->base, &item->base);
> > }
> >
> > static void main_channel_marshall_tokens(MainChannel *main_chan, uint32_t num_tokens)
> > @@ -397,7 +397,7 @@ void main_channel_push_agent_data(Channel *channel, uint8_t* data, size_t len,
> > AgentDataPipeItem *item;
> >
> > item = main_agent_data_item_new(main_chan, data, len, free_data, opaque);
> > - red_channel_pipe_add(&main_chan->base, &item->base);
> > + red_channel_pipe_add_push(&main_chan->base, &item->base);
> > }
> >
> > static void main_channel_marshall_agent_data(MainChannel *main_chan,
> > @@ -411,7 +411,7 @@ static void main_channel_push_migrate_data_item(MainChannel *main_chan)
> > {
> > RedsOutItem *item = main_pipe_item_new(main_chan, SPICE_MSG_MIGRATE_DATA);
> >
> > - red_channel_pipe_add(&main_chan->base, &item->base);
> > + red_channel_pipe_add_push(&main_chan->base, &item->base);
> > }
> >
> > static void main_channel_marshall_migrate_data_item(MainChannel *main_chan)
> > @@ -442,7 +442,7 @@ void main_channel_push_init(Channel *channel, int connection_id,
> > item = main_init_item_new(main_chan,
> > connection_id, display_channels_hint, current_mouse_mode,
> > is_client_mouse_allowed, multi_media_time, ram_hint);
> > - red_channel_pipe_add(&main_chan->base, &item->base);
> > + red_channel_pipe_add_push(&main_chan->base, &item->base);
> > }
> >
> > static void main_channel_marshall_init(MainChannel *main_chan,
> > @@ -469,7 +469,7 @@ void main_channel_push_notify(Channel *channel, uint8_t *mess, const int mess_le
> > MainChannel *main_chan = channel->data;
> > NotifyPipeItem *item = main_notify_item_new(main_chan, mess, mess_len);
> >
> > - red_channel_pipe_add(&main_chan->base, &item->base);
> > + red_channel_pipe_add_push(&main_chan->base, &item->base);
> > }
> >
> > static void main_channel_marshall_notify(MainChannel *main_chan, NotifyPipeItem *item)
> > @@ -494,7 +494,7 @@ void main_channel_push_migrate_begin(Channel *channel, int port, int sport,
> > MigrateBeginPipeItem *item = main_migrate_begin_item_new(main_chan, port,
> > sport, host, cert_pub_key_type, cert_pub_key_len, cert_pub_key);
> >
> > - red_channel_pipe_add(&main_chan->base, &item->base);
> > + red_channel_pipe_add_push(&main_chan->base, &item->base);
> > }
> >
> > static void main_channel_marshall_migrate_begin(MainChannel *main_chan,
> > @@ -518,7 +518,7 @@ void main_channel_push_migrate(Channel *channel)
> > MainChannel *main_chan = channel->data;
> > RedsOutItem *item = main_pipe_item_new(main_chan, SPICE_MSG_MIGRATE);
> >
> > - red_channel_pipe_add(&main_chan->base, &item->base);
> > + red_channel_pipe_add_push(&main_chan->base, &item->base);
> > }
> >
> > static void main_channel_marshall_migrate(MainChannel *main_chan)
> > @@ -535,7 +535,7 @@ void main_channel_push_migrate_cancel(Channel *channel)
> > RedsOutItem *item = main_pipe_item_new(main_chan,
> > SPICE_MSG_MAIN_MIGRATE_CANCEL);
> >
> > - red_channel_pipe_add(&main_chan->base, &item->base);
> > + red_channel_pipe_add_push(&main_chan->base, &item->base);
> > }
> >
> > void main_channel_push_multi_media_time(Channel *channel, int time)
> > @@ -544,7 +544,7 @@ void main_channel_push_multi_media_time(Channel *channel, int time)
> >
> > MultiMediaTimePipeItem *item =
> > main_multi_media_time_item_new(main_chan, time);
> > - red_channel_pipe_add(&main_chan->base, &item->base);
> > + red_channel_pipe_add_push(&main_chan->base, &item->base);
> > }
> >
> > static PipeItem *main_migrate_switch_item_new(MainChannel *main_chan)
> > diff --git a/server/red_channel.c b/server/red_channel.c
> > index c208ec3..5220341 100644
> > --- a/server/red_channel.c
> > +++ b/server/red_channel.c
> > @@ -507,7 +507,14 @@ void red_channel_pipe_add(RedChannel *channel, PipeItem *item)
> >
> > channel->pipe_size++;
> > ring_add(&channel->pipe, &item->link);
> > +}
> > +
> > +void red_channel_pipe_add_push(RedChannel *channel, PipeItem *item)
> > +{
> > + ASSERT(channel);
> >
> > + channel->pipe_size++;
> > + ring_add(&channel->pipe, &item->link);
> > red_channel_push(channel);
> > }
> >
> > diff --git a/server/red_channel.h b/server/red_channel.h
> > index db0ae9a..ec178f5 100644
> > --- a/server/red_channel.h
> > +++ b/server/red_channel.h
> > @@ -211,6 +211,7 @@ void red_channel_set_message_serial(RedChannel *channel, uint64_t);
> > void red_channel_begin_send_message(RedChannel *channel);
> >
> > void red_channel_pipe_item_init(RedChannel *channel, PipeItem *item, int type);
> > +void red_channel_pipe_add_push(RedChannel *channel, PipeItem *item);
> > void red_channel_pipe_add(RedChannel *channel, PipeItem *item);
> > int red_channel_pipe_item_is_linked(RedChannel *channel, PipeItem *item);
> > void red_channel_pipe_item_remove(RedChannel *channel, PipeItem *item);
> > diff --git a/server/smartcard.c b/server/smartcard.c
> > index 318cb27..5f70391 100644
> > --- a/server/smartcard.c
> > +++ b/server/smartcard.c
> > @@ -353,9 +353,9 @@ static void smartcard_channel_disconnect(RedChannel *channel)
> > /* this is called from both device input and client input. since the device is
> > * a usb device, the context is still the main thread (kvm_main_loop, timers)
> > * so no mutex is required. */
> > -static void smartcard_channel_pipe_add(SmartCardChannel *channel, PipeItem *item)
> > +static void smartcard_channel_pipe_add_push(SmartCardChannel *channel, PipeItem *item)
> > {
> > - red_channel_pipe_add(&channel->base, item);
> > + red_channel_pipe_add_push(&channel->base, item);
> > }
> >
> > static void smartcard_push_error(SmartCardChannel* channel, reader_id_t reader_id, VSCErrorCode error)
> > @@ -365,7 +365,7 @@ static void smartcard_push_error(SmartCardChannel* channel, reader_id_t reader_i
> > error_item->base.type = PIPE_ITEM_TYPE_ERROR;
> > error_item->reader_id = reader_id;
> > error_item->error = error;
> > - smartcard_channel_pipe_add(channel, &error_item->base);
> > + smartcard_channel_pipe_add_push(channel, &error_item->base);
> > }
> >
> > static void smartcard_push_reader_add_response(SmartCardChannel *channel, uint32_t reader_id)
> > @@ -374,7 +374,7 @@ static void smartcard_push_reader_add_response(SmartCardChannel *channel, uint32
> >
> > rar_item->base.type = PIPE_ITEM_TYPE_READER_ADD_RESPONSE;
> > rar_item->reader_id = reader_id;
> > - smartcard_channel_pipe_add(channel, &rar_item->base);
> > + smartcard_channel_pipe_add_push(channel, &rar_item->base);
> > }
> >
> > static void smartcard_push_vscmsg(SmartCardChannel *channel, VSCMsgHeader *vheader)
> > @@ -383,7 +383,7 @@ static void smartcard_push_vscmsg(SmartCardChannel *channel, VSCMsgHeader *vhead
> >
> > msg_item->base.type = PIPE_ITEM_TYPE_MSG;
> > msg_item->vheader = vheader;
> > - smartcard_channel_pipe_add(channel, &msg_item->base);
> > + smartcard_channel_pipe_add_push(channel, &msg_item->base);
> > }
> >
> > void smartcard_on_message_from_device(SmartCardChannel *smartcard_channel,
> > --
> > 1.7.3.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