[Spice-devel] [PATCH vdagent-linux] vdagentd: send monitors config to currently active agent only

[Hans de Goede]

Hi,

On 11/22/2012 12:27 AM, Marc-André Lureau wrote:
> Sending monitors config to all agents will results in unnecessary work
> from some of them, or racy behaviour. Currently, several session agent
> sharing the same X session may want to create or modify the same mode
> resulting in X errors:
>
> spice-vdagent[2985]: received monitor 0 config 1440x823+0+0
> spice-vdagent[2985]: after zeroing: 1, 1
> spice-vdagent[2985]: received monitor 0 config 1440x823+0+0
> X Error of failed request:  BadName (named color or font does not exist)
>    Major opcode of failed request:  139 (RANDR)
>    Minor opcode of failed request:  16 (RRCreateMode)
>    Serial number of failed request:  52
>    Current serial number in output stream:  52
>
> (gdb) bt
>      width=1440, height=823) at src/vdagent-x11-randr.c:310
>      x=0, y=0, width=1440, height=823) at src/vdagent-x11-randr.c:336
>      mon_config=0x61a020) at src/vdagent-x11-randr.c:662
>
> On server side, the error is triggered when the second RRModeCreateUser is received:
>
> Breakpoint 1, RRModeCreateUser (pScreen=0xb36700, modeInfo=0xbfd7d8, name=0xbfd7f8 "vdagent-0", error=0x7ffff4dcb794) at rrmode.c:146
> 146            mode = RRModeFindByName(name, modeInfo->nameLength);
> (gdb) n
> 147       if (mode) {
> (gdb)
> 148           *error = BadName;
> (gdb)
> 149           return NULL;
>
> https://bugzilla.redhat.com/show_bug.cgi?id=872633
> ---
>   src/vdagentd.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/vdagentd.c b/src/vdagentd.c
> index a4db935..02e73ca 100644
> --- a/src/vdagentd.c
> +++ b/src/vdagentd.c
> @@ -130,9 +130,9 @@ static void do_client_monitors(struct vdagent_virtio_port *vport, int port_nr,
>       }
>       memcpy(mon_config, new_monitors, size);
>
> -    /* Send monitor config to currently connected agents */
> -    udscs_server_write_all(server, VDAGENTD_MONITORS_CONFIG, 0, 0,
> -                           (uint8_t *)mon_config, size);
> +    /* Send monitor config to currently active agent */
> +    udscs_write(active_session_conn, VDAGENTD_MONITORS_CONFIG, 0, 0,
> +                (uint8_t *)mon_config, size);
>
>       /* Acknowledge reception of monitors config to spice server / client */
>       reply.type  = VD_AGENT_MONITORS_CONFIG;
>

Good catch! This may very well explain much if the issues we've been seeing. But this needs some
more work, we need to also send an (potentially updated) monitor-config to the
new active-agent when the active session changes, ie, we've 2 agents running for 2 sessions
not sharing the same X-server, ie using user switching, then when we switch to the other user,
the agent should be notified so that the resolution of the other X-server gets adjusted too.

Regards,

Hans