[Spice-devel] [PATCH 03/17] Store a reference to RedsState in Channel base class

Frediano Ziglio fziglio at redhat.com
Mon Feb 15 15:55:29 UTC 2016


> 
> On Thu, 2016-02-11 at 14:30 -0500, Frediano Ziglio wrote:
> > > 
> > > From: Jonathon Jongsma <jjongsma at redhat.com>
> > > 
> > > ---
> > >  server/inputs-channel.c |  1 +
> > >  server/main-channel.c   |  3 ++-
> > >  server/red-channel.c    | 26 +++++++++++++++-----------
> > >  server/red-channel.h    | 19 +++++++++++--------
> > >  server/red-worker.c     |  2 +-
> > >  server/smartcard.c      |  1 +
> > >  server/sound.c          |  4 ++--
> > >  server/spicevmc.c       |  2 +-
> > >  8 files changed, 34 insertions(+), 24 deletions(-)
> > > 
> > > diff --git a/server/inputs-channel.c b/server/inputs-channel.c
> > > index f47a586..71716d5 100644
> > > --- a/server/inputs-channel.c
> > > +++ b/server/inputs-channel.c
> > > @@ -636,6 +636,7 @@ InputsChannel* inputs_channel_new(void)
> > >  
> > >      inputs = (InputsChannel *)red_channel_create_parser(
> > >                                      sizeof(InputsChannel),
> > > +                                    reds,
> > >                                      reds_get_core_interface(reds),
> > 
> > Now that core is bound to reds we should remove the core argument
> > and just keep the reds one.
> > 
> > >                                      SPICE_CHANNEL_INPUTS, 0,
> > >                                      FALSE, /* handle_acks */
> > > diff --git a/server/main-channel.c b/server/main-channel.c
> > > index 8b1d7d4..14f74ce 100644
> > > --- a/server/main-channel.c
> > > +++ b/server/main-channel.c
> > > @@ -1179,7 +1179,8 @@ MainChannel* main_channel_new(void)
> > >      channel_cbs.handle_migrate_data = main_channel_handle_migrate_data;
> > >  
> > >      // TODO: set the migration flag of the channel
> > > -    channel = red_channel_create_parser(sizeof(MainChannel),
> > > reds_get_core_interface(reds),
> > > +    channel = red_channel_create_parser(sizeof(MainChannel), reds,
> > > +                                        reds_get_core_interface(reds),
> > >                                          SPICE_CHANNEL_MAIN, 0,
> > >                                          FALSE, /* handle_acks */
> > >                                         
> > >  spice_get_client_channel_parser(SPICE_CHANNEL_MAIN,
> > >                                          NULL),
> > > diff --git a/server/red-channel.c b/server/red-channel.c
> > > index a6069a9..490f08a 100644
> > > --- a/server/red-channel.c
> > > +++ b/server/red-channel.c
> > > @@ -1016,6 +1016,7 @@ void
> > > red_channel_client_default_migrate(RedChannelClient *rcc)
> > >  }
> > >  
> > >  RedChannel *red_channel_create(int size,
> > > +                               RedsState *reds,
> > >                                 const SpiceCoreInterfaceInternal *core,
> > >                                 uint32_t type, uint32_t id,
> > >                                 int handle_acks,
> > > @@ -1039,6 +1040,7 @@ RedChannel *red_channel_create(int size,
> > >      channel->migration_flags = migration_flags;
> > >      memcpy(&channel->channel_cbs, channel_cbs, sizeof(ChannelCbs));
> > >  
> > > +    channel->reds = reds;
> > >      channel->core = core;
> > >      ring_init(&channel->clients);
> > >  
> > > @@ -1095,7 +1097,7 @@ SpiceCoreInterfaceInternal dummy_core = {
> > >      .watch_remove = dummy_watch_remove,
> > >  };
> > >  
> > > -RedChannel *red_channel_create_dummy(int size, uint32_t type, uint32_t
> > > id)
> > > +RedChannel *red_channel_create_dummy(int size, RedsState *reds, uint32_t
> > > type, uint32_t id)
> > >  {
> > >      RedChannel *channel;
> > >      ClientCbs client_cbs = { NULL, };
> > > @@ -1105,6 +1107,7 @@ RedChannel *red_channel_create_dummy(int size,
> > > uint32_t
> > > type, uint32_t id)
> > >      channel->type = type;
> > >      channel->id = id;
> > >      channel->refs = 1;
> > > +    channel->reds = reds;
> > >      channel->core = &dummy_core;
> > >      ring_init(&channel->clients);
> > >      client_cbs.connect = red_channel_client_default_connect;
> > > @@ -1132,15 +1135,16 @@ static int
> > > do_nothing_handle_message(RedChannelClient
> > > *rcc,
> > >  }
> > >  
> > >  RedChannel *red_channel_create_parser(int size,
> > > -                               const SpiceCoreInterfaceInternal *core,
> > > -                               uint32_t type, uint32_t id,
> > > -                               int handle_acks,
> > > -                               spice_parse_channel_func_t parser,
> > > -                               channel_handle_parsed_proc handle_parsed,
> > > -                               const ChannelCbs *channel_cbs,
> > > -                               uint32_t migration_flags)
> > > -{
> > > -    RedChannel *channel = red_channel_create(size, core, type, id,
> > > +                                      RedsState *reds,
> > > +                                      const SpiceCoreInterfaceInternal
> > > *core,
> > > +                                      uint32_t type, uint32_t id,
> > > +                                      int handle_acks,
> > > +                                      spice_parse_channel_func_t parser,
> > > +                                      channel_handle_parsed_proc
> > > handle_parsed,
> > > +                                      const ChannelCbs *channel_cbs,
> > > +                                      uint32_t migration_flags)
> > > +{
> > > +    RedChannel *channel = red_channel_create(size, reds, core, type, id,
> > >                                               handle_acks,
> > >                                               do_nothing_handle_message,
> > >                                               channel_cbs,
> > > @@ -1162,7 +1166,7 @@ void red_channel_set_stat_node(RedChannel *channel,
> > > StatNodeRef stat)
> > >  
> > >  #ifdef RED_STATISTICS
> > >      channel->stat = stat;
> > > -    channel->out_bytes_counter = reds_stat_add_counter(reds, stat,
> > > "out_bytes", TRUE);
> > > +    channel->out_bytes_counter = reds_stat_add_counter(channel->reds,
> > > stat,
> > > "out_bytes", TRUE);
> > >  #endif
> > >  }
> > >  
> > > diff --git a/server/red-channel.h b/server/red-channel.h
> > > index b5ab7ac..3a12ae0 100644
> > > --- a/server/red-channel.h
> > > +++ b/server/red-channel.h
> > > @@ -337,6 +337,7 @@ struct RedChannel {
> > >  
> > >      // TODO: when different channel_clients are in different threads
> > >      from
> > >      Channel -> need to protect!
> > >      pthread_t thread_id;
> > > +    RedsState *reds;
> > >  #ifdef RED_STATISTICS
> > >      StatNodeRef stat;
> > >      uint64_t *out_bytes_counter;
> > > @@ -359,6 +360,7 @@ struct RedChannel {
> > >  /* if one of the callbacks should cause disconnect, use
> > > red_channel_shutdown
> > >  and don't
> > >   * explicitly destroy the channel */
> > >  RedChannel *red_channel_create(int size,
> > > +                               RedsState *reds,
> > >                                 const SpiceCoreInterfaceInternal *core,
> > >                                 uint32_t type, uint32_t id,
> > >                                 int handle_acks,
> > > @@ -369,13 +371,14 @@ RedChannel *red_channel_create(int size,
> > >  /* alternative constructor, meant for marshaller based (inputs,main)
> > >  channels,
> > >   * will become default eventually */
> > >  RedChannel *red_channel_create_parser(int size,
> > > -                               const SpiceCoreInterfaceInternal *core,
> > > -                               uint32_t type, uint32_t id,
> > > -                               int handle_acks,
> > > -                               spice_parse_channel_func_t parser,
> > > -                               channel_handle_parsed_proc handle_parsed,
> > > -                               const ChannelCbs *channel_cbs,
> > > -                               uint32_t migration_flags);
> > > +                                      RedsState *reds,
> > > +                                      const SpiceCoreInterfaceInternal
> > > *core,
> > > +                                      uint32_t type, uint32_t id,
> > > +                                      int handle_acks,
> > > +                                      spice_parse_channel_func_t parser,
> > > +                                      channel_handle_parsed_proc
> > > handle_parsed,
> > > +                                      const ChannelCbs *channel_cbs,
> > > +                                      uint32_t migration_flags);
> > >  void red_channel_set_stat_node(RedChannel *channel, StatNodeRef stat);
> > >  
> > >  void red_channel_register_client_cbs(RedChannel *channel, const
> > >  ClientCbs
> > >  *client_cbs);
> > > @@ -392,7 +395,7 @@ RedChannelClient *red_channel_client_create(int size,
> > > RedChannel *channel, RedCl
> > >  // TODO: tmp, for channels that don't use RedChannel yet (e.g., snd
> > >  channel), but
> > >  // do use the client callbacks. So the channel clients are not connected
> > >  (the channel doesn't
> > >  // have list of them, but they do have a link to the channel, and the
> > > client
> > >  has a list of them)
> > > -RedChannel *red_channel_create_dummy(int size, uint32_t type, uint32_t
> > > id);
> > > +RedChannel *red_channel_create_dummy(int size, RedsState *reds, uint32_t
> > > type, uint32_t id);
> > >  RedChannelClient *red_channel_client_create_dummy(int size,
> > >                                                    RedChannel *channel,
> > >                                                    RedClient  *client,
> > > diff --git a/server/red-worker.c b/server/red-worker.c
> > > index 7e5742f..d4e10d7 100644
> > > --- a/server/red-worker.c
> > > +++ b/server/red-worker.c
> > > @@ -513,7 +513,7 @@ CommonChannel *red_worker_new_channel(RedWorker
> > > *worker,
> > > int size,
> > >      channel_cbs->alloc_recv_buf = common_alloc_recv_buf;
> > >      channel_cbs->release_recv_buf = common_release_recv_buf;
> > >  
> > > -    channel = red_channel_create_parser(size, &worker->core,
> > > +    channel = red_channel_create_parser(size, reds, &worker->core,
> > >                                          channel_type, worker->qxl->id,
> > >                                          TRUE /* handle_acks */,
> > >                                         
> > >  spice_get_client_channel_parser(channel_type,
> > >                                          NULL),
> > > diff --git a/server/smartcard.c b/server/smartcard.c
> > > index e9e58a8..c7b1f30 100644
> > > --- a/server/smartcard.c
> > > +++ b/server/smartcard.c
> > > @@ -849,6 +849,7 @@ static void smartcard_init(void)
> > >      channel_cbs.handle_migrate_data =
> > >      smartcard_channel_client_handle_migrate_data;
> > >  
> > >      g_smartcard_channel =
> > >      (SmartCardChannel*)red_channel_create(sizeof(SmartCardChannel),
> > > +                                             reds,
> > >                                               reds_get_core_interface(reds),
> > >                                               SPICE_CHANNEL_SMARTCARD, 0,
> > >                                               FALSE /* handle_acks */,
> > > diff --git a/server/sound.c b/server/sound.c
> > > index 3c77d77..fd0b416 100644
> > > --- a/server/sound.c
> > > +++ b/server/sound.c
> > > @@ -1519,7 +1519,7 @@ void snd_attach_playback(SpicePlaybackInstance
> > > *sin)
> > >      sin->st->frequency = SND_CODEC_CELT_PLAYBACK_FREQ; /* Default to the
> > >      legacy rate */
> > >  
> > >      // TODO: Make RedChannel base of worker? instead of assigning it to
> > >      channel->data
> > > -    channel = red_channel_create_dummy(sizeof(RedChannel),
> > > SPICE_CHANNEL_PLAYBACK, 0);
> > > +    channel = red_channel_create_dummy(sizeof(RedChannel), reds,
> > > SPICE_CHANNEL_PLAYBACK, 0);
> > >  
> > >      client_cbs.connect = snd_set_playback_peer;
> > >      client_cbs.disconnect = snd_disconnect_channel_client;
> > > @@ -1549,7 +1549,7 @@ void snd_attach_record(SpiceRecordInstance *sin)
> > >      sin->st->frequency = SND_CODEC_CELT_PLAYBACK_FREQ; /* Default to the
> > >      legacy rate */
> > >  
> > >      // TODO: Make RedChannel base of worker? instead of assigning it to
> > >      channel->data
> > > -    channel = red_channel_create_dummy(sizeof(RedChannel),
> > > SPICE_CHANNEL_RECORD, 0);
> > > +    channel = red_channel_create_dummy(sizeof(RedChannel), reds,
> > > SPICE_CHANNEL_RECORD, 0);
> > >  
> > >      client_cbs.connect = snd_set_record_peer;
> > >      client_cbs.disconnect = snd_disconnect_channel_client;
> > > diff --git a/server/spicevmc.c b/server/spicevmc.c
> > > index 4fc95e3..35d1393 100644
> > > --- a/server/spicevmc.c
> > > +++ b/server/spicevmc.c
> > > @@ -524,7 +524,7 @@ SpiceCharDeviceState
> > > *spicevmc_device_connect(RedsState
> > > *reds,
> > >      channel_cbs.handle_migrate_flush_mark =
> > >      spicevmc_channel_client_handle_migrate_flush_mark;
> > >      channel_cbs.handle_migrate_data =
> > >      spicevmc_channel_client_handle_migrate_data;
> > >  
> > > -    state = (SpiceVmcState*)red_channel_create(sizeof(SpiceVmcState),
> > > +    state = (SpiceVmcState*)red_channel_create(sizeof(SpiceVmcState),
> > > reds,
> > >                                     reds_get_core_interface(reds),
> > >                                     channel_type, id[channel_type]++,
> > >                                     FALSE /* handle_acks */,
> > >                                    
> > >  spicevmc_red_channel_client_handle_message,
> > 
> > I'll post an updated patch
> 
> 
> So, it turns out that if we remove the RedsState typedef from stat.h (see
> patch
> 1/17), this patch fails to build due to the use of RedsState in the header.
> 
> 

Yes, too many changed are going on.

I'll post an update!

Frediano


More information about the Spice-devel mailing list