Re: [Qemu-devel] [PATCH v2 2/5] spice: notify on vm state change only via spice_server_vm_start/stop

[Yonit Halperin]

Hi,
On 08/21/2012 09:58 AM, Gerd Hoffmann wrote:
>    Hi,
>
> > +#if SPICE_SERVER_VERSION>= 0x000b02 /* 0.11.2 */
> > +static void display_start(void)
> > +{
> > +    DisplayListItem *item;
> > +
> > +    QLIST_FOREACH(item,&display_list, link) {
> > +        item->display->running = true;
> > +    }
> > +}
>
> I think we should simply use a global variable for 'running' here.  It's
> global state anyway.  Having this in per-display state struct buys us
> nothing and adds alot of code for updating and display list maintainance.
ok, good point.
> >   void qemu_spice_vm_change_state_handler(void *opaque, int running,
> >                                           RunState state)
> >   {
> > +#if SPICE_SERVER_VERSION<  0x000b02 /* before 0.11.2 */
> >       SimpleSpiceDisplay *ssd = opaque;
>
> > +#if SPICE_SERVER_VERSION<  0x000b02 /* before 0.11.2 */
> >       
> > qemu_add_vm_change_state_handler(qemu_spice_vm_change_state_handler,&sdpy);
> > +#endif
>
> If you don't register qemu_spice_vm_change_state_handler on new
> spice-server you should #ifdef the whole function, not the body only.
qemu_spice_vm_change_state_handler is also called from qxl.c, as part of 
its own vm_change_state handler. I didn't want to add another ifdef there.

> cheers,
>    Gerd