[Spice-devel] [PATCH spice-server 07/10] Propagate running property from RedWorker to DisplayChannel

[Frediano Ziglio]

> 
> Somehow this information doesn't really seem to belong to
> DisplayChannel, but I can't put my finger on exactly why it feels out
> of place...
> 
> 
> 
> On Wed, 2019-03-20 at 09:59 +0000, Frediano Ziglio wrote:
> > This is a preparatory patch to allows DisplayChannel to check
> > if device is running.
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/display-channel-private.h | 1 +
> >  server/display-channel.c         | 8 ++++++++
> >  server/display-channel.h         | 2 ++
> >  server/red-worker.c              | 2 ++
> >  4 files changed, 13 insertions(+)
> > 
> > diff --git a/server/display-channel-private.h b/server/display-
> > channel-private.h
> > index 58179531..067c6418 100644
> > --- a/server/display-channel-private.h
> > +++ b/server/display-channel-private.h
> > @@ -89,6 +89,7 @@ struct DisplayChannelPrivate
> >      uint32_t renderer;
> >      int enable_jpeg;
> >      int enable_zlib_glz_wrap;
> > +    bool running;
> 
> At minimum, this seems like way too generic of a name. I will probably
> look at this later and wonder what it means for a DisplayChannel to be
> 'running'. But in reality this variable indicates that the QXL device
> is started? Actually, after looking briefly at this handle_dev_start()
> function, I can't even figure out what causes this function to be
> called...
> 

Yes, all these dispatching can be confusing.
Basically the VM is running or not (paused for different reasons).
When the VM is paused you don't want to send events to it or change
its status.
Maybe "qxl_active" ?

> >  
> >      /* A ring of pending drawables for this DisplayChannel,
> > regardless of which
> >       * surface they're associated with. This list is mainly used to
> > flush older
> > diff --git a/server/display-channel.c b/server/display-channel.c
> > index cb052bfc..49a2bd48 100644
> > --- a/server/display-channel.c
> > +++ b/server/display-channel.c
> > @@ -2540,3 +2540,11 @@ void display_channel_debug_oom(DisplayChannel
> > *display, const char *msg)
> >                  ring_get_length(&display->priv->current_list),
> >                  red_channel_sum_pipes_size(channel));
> >  }
> > +
> > +void display_channel_set_running(DisplayChannel *display, bool
> > running)
> > +{
> > +    if (running == display->priv->running) {
> > +        return;
> > +    }
> > +    display->priv->running = running;
> > +}
> > diff --git a/server/display-channel.h b/server/display-channel.h
> > index f64da0bd..bdf2f059 100644
> > --- a/server/display-channel.h
> > +++ b/server/display-channel.h
> > @@ -158,6 +158,8 @@ void
> > display_channel_reset_image_cache(DisplayChannel *self);
> >  
> >  void display_channel_debug_oom(DisplayChannel *display, const char
> > *msg);
> >  
> > +void display_channel_set_running(DisplayChannel *display, bool
> > running);
> > +
> >  G_END_DECLS
> >  
> >  #endif /* DISPLAY_CHANNEL_H_ */
> > diff --git a/server/red-worker.c b/server/red-worker.c
> > index 07329b17..c9c47133 100644
> > --- a/server/red-worker.c
> > +++ b/server/red-worker.c
> > @@ -588,6 +588,7 @@ static void handle_dev_stop(void *opaque, void
> > *payload)
> >      spice_assert(worker->running);
> >  
> >      worker->running = FALSE;
> > +    display_channel_set_running(worker->display_channel, false);
> >  
> >      display_channel_free_glz_drawables(worker->display_channel);
> >      display_channel_flush_all_surfaces(worker->display_channel);
> > @@ -616,6 +617,7 @@ static void handle_dev_start(void *opaque, void
> > *payload)
> >          display_channel_wait_for_migrate_data(worker-
> > >display_channel);
> >      }
> >      worker->running = TRUE;
> > +    display_channel_set_running(worker->display_channel, true);
> >      worker->event_timeout = 0;
> >      guest_set_client_capabilities(worker);
> >  } 

Frediano