[Spice-devel] [PATCH spice-server 1/2] red-channel-client: Introduce some helpers to update watch event mask

[Christophe Fergeau]

On Mon, Sep 11, 2017 at 11:41:17AM -0400, Frediano Ziglio wrote:
> 
> > 
> > Why ?
> > 
> > Christophe
> 
> for 2/2.
> 
> Maybe I should add 
> 
> "These helpers will be reused by following patch."

Yup, something like that would be useful, thanks.

> > 
> > On Mon, Sep 11, 2017 at 01:29:05PM +0100, Frediano Ziglio wrote:
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > >  server/red-channel-client.c | 27 ++++++++++++++++-----------
> > >  1 file changed, 16 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/server/red-channel-client.c b/server/red-channel-client.c
> > > index d9333ba6f..eab1d593d 100644
> > > --- a/server/red-channel-client.c
> > > +++ b/server/red-channel-client.c
> > > @@ -45,6 +45,8 @@
> > >  #define IOV_MAX 1024
> > >  #endif
> > >  
> > > +#define SPICE_WATCH_EVENTS_READ_WRITE
> > > (SPICE_WATCH_EVENT_READ|SPICE_WATCH_EVENT_WRITE)

I would not add this here, this is not directly related to that patch.

> > > +
> > >  typedef struct SpiceDataHeaderOpaque SpiceDataHeaderOpaque;
> > >  
> > >  typedef uint16_t (*get_msg_type_proc)(SpiceDataHeaderOpaque *header);
> > > @@ -972,6 +974,16 @@ cleanup:
> > >      return local_error == NULL;
> > >  }
> > >  
> > > +static void
> > > +red_channel_client_watch_update_mask(RedChannelClient *rcc, int
> > > event_mask)
> > > +{
> > > +    if (rcc->priv->stream->watch) {
> > > +        SpiceCoreInterfaceInternal *core;
> > > +        core = red_channel_get_core_interface(rcc->priv->channel);
> > > +        core->watch_update_mask(core, rcc->priv->stream->watch,
> > > event_mask);
> > > +    }

In general, I find code formatted like below easier to read:

+    SpiceCoreInterfaceInternal *core;
+    if (!rcc->priv->stream->watch) {
+        return;
+    }

+    core = red_channel_get_core_interface(rcc->priv->channel);
+    core->watch_update_mask(core, rcc->priv->stream->watch, event_mask);
}


Acked-by: Christophe Fergeau <cfergeau at redhat.com>


> > > +}
> > > +
> > >  static void red_channel_client_seamless_migration_done(RedChannelClient
> > >  *rcc)
> > >  {
> > >      rcc->priv->wait_migrate_data = FALSE;
> > > @@ -1315,12 +1327,8 @@ void red_channel_client_push(RedChannelClient *rcc)
> > >      while ((pipe_item = red_channel_client_pipe_item_get(rcc))) {
> > >          red_channel_client_send_item(rcc, pipe_item);
> > >      }
> > > -    if (red_channel_client_no_item_being_sent(rcc) &&
> > > g_queue_is_empty(&rcc->priv->pipe)
> > > -        && rcc->priv->stream->watch) {
> > > -        SpiceCoreInterfaceInternal *core;
> > > -        core = red_channel_get_core_interface(rcc->priv->channel);
> > > -        core->watch_update_mask(core, rcc->priv->stream->watch,
> > > -                                SPICE_WATCH_EVENT_READ);
> > > +    if (red_channel_client_no_item_being_sent(rcc) &&
> > > g_queue_is_empty(&rcc->priv->pipe)) {
> > > +        red_channel_client_watch_update_mask(rcc, SPICE_WATCH_EVENT_READ);
> > >      }
> > >      rcc->priv->during_send = FALSE;
> > >      g_object_unref(rcc);
> > > @@ -1533,11 +1541,8 @@ static inline gboolean
> > > prepare_pipe_add(RedChannelClient *rcc, RedPipeItem *item
> > >          red_pipe_item_unref(item);
> > >          return FALSE;
> > >      }
> > > -    if (g_queue_is_empty(&rcc->priv->pipe) && rcc->priv->stream->watch) {
> > > -        SpiceCoreInterfaceInternal *core;
> > > -        core = red_channel_get_core_interface(rcc->priv->channel);
> > > -        core->watch_update_mask(core, rcc->priv->stream->watch,
> > > -                                SPICE_WATCH_EVENT_READ |
> > > SPICE_WATCH_EVENT_WRITE);
> > > +    if (g_queue_is_empty(&rcc->priv->pipe)) {
> > > +        red_channel_client_watch_update_mask(rcc,
> > > SPICE_WATCH_EVENTS_READ_WRITE);
> > >      }
> > >      return TRUE;
> > >  }