On Thu, 2016-09-01 at 17:45 +0200, Christophe Fergeau wrote:
> On Wed, Aug 31, 2016 at 11:54:38AM -0500, Jonathon Jongsma wrote:
> >
> > Reduce direct access to RedChannelClient, and get ready to convert
> > to
> > GObject.
> > ---
[snip]
> > diff --git a/server/red-channel-client.c b/server/red-channel-
> > client.c
> > new file mode 100644
> > index 0000000..253e30e
> > --- /dev/null
> > +++ b/server/red-channel-client.c
> > @@ -0,0 +1,1626 @@
> > +
> > +static void red_channel_handle_migrate_flush_mark(RedChannelClient
> > *rcc)
> > +{
> > + RedChannel *channel = red_channel_client_get_channel(rcc);
> > + if (channel->channel_cbs.handle_migrate_flush_mark) {
> > + channel->channel_cbs.handle_migrate_flush_mark(rcc);
> > + }
> > +}
> > +
> > +// TODO: the whole migration is broken with multiple clients. What
> > do we want to do?
> > +// basically just
> > +// 1) source send mark to all
> > +// 2) source gets at various times the data (waits for all)
> > +// 3) source migrates to target
> > +// 4) target sends data to all
> > +// So need to make all the handlers work with per channel/client
> > data (what data exactly?)
> > +static void red_channel_handle_migrate_data(RedChannelClient *rcc,
> > uint32_t size, void *message)
>
>
> Just to be sure, these 2 functions were intended to be moved? They
> are
> not in the red_channel_client_ namespace but in the red_channel_ one.
>
Hmm, yeah. I should have renamed these to
red_channel_client_handle_migrate...
[snip]
> > diff --git a/server/red-channel.c b/server/red-channel.c
> > index bf290b1..03338aa 100644
> > --- a/server/red-channel.c
> > +++ b/server/red-channel.c
> > @@ -2179,11 +707,7 @@ void
> > red_client_semi_seamless_migrate_complete(RedClient *client)
> > link = client->channels;
> > while (link) {
> > next = link->next;
> > - RedChannelClient *rcc = link->data;
> > -
> > - if (rcc->latency_monitor.timer) {
> > - red_channel_client_start_ping_timer(rcc,
> > PING_TEST_IDLE_NET_TIMEOUT_MS);
> > - }
> > + red_channel_client_semi_seamless_migration_complete(link-
> > >data);
>
> Non-obvious name given the code it replaces, but I guess you
> double-checked that's a good name?
>
I see your point. Basically I needed to avoid modifying the
RedChannelClient object from within the RedClient implementation.
Before, there was a single function that accessed internal data of both
the RedClient and RedChannelClient objects:
red_client_semi_seamless_migrate_complete(). So I simply split the
function into two parts, each responsible for accessing the internals
of its own object:
- red_client_semi_seamless_migrate_complete() - handles the RedClient
part
- red_channel_client_semi_seamless_migration_complete() - handles the
RedChannelClient part
That was my justification for the name anyway. I'm open to alternative
suggestions. I believe I considered something like
_on_semi_seamless_migration_complete(), but didn't use that name. The
_on_ terminology might make it more obvious that this function is sort
of an 'event handler' to be called when semi-seamless migration is
complete?
> > -// TODO: again - what is the context exactly? this happens in
> > channel disconnect. but our
> > -// current red_channel_shutdown also closes the socket - is there
> > a socket to close?
> > -// are we reading from an fd here? arghh
> > -void red_channel_client_pipe_clear(RedChannelClient *rcc);
>
> Note that this comment is attached to
> red_channel_client_pipe_clear().
> In the new header, _pipe_clear() is static and the header now has
>
> >
> > +// TODO: again - what is the context exactly? this happens in
> > channel disconnect. but our
> > +// current red_channel_shutdown also closes the socket - is there
> > a socket to close?
> > +// are we reading from an fd here? arghh
> > +void red_channel_client_receive(RedChannelClient *rcc);
Oops, I'll move that comment down to the static _pipe_clear() function.
Jonathon
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel