Re: [PATCH 1/2] ui/vdagent: call vdagent_disconnect() when agent connection is lost

[Guoyi Tu]

On 2023/8/17 21:36, 【外部账号】Marc-André Lureau wrote:
> Hi
>
> On Thu, Aug 17, 2023 at 5:10 PM Guoyi Tu <tugy@chinatelecom.cn> wrote:
> > On 2023/8/17 20:49, 【外部账号】Marc-André Lureau wrote:
> > > Hi
> > >
> > > On Thu, Aug 17, 2023 at 3:32 PM <tugy@chinatelecom.cn> wrote:
> > > > From: Guoyi Tu <tugy@chinatelecom.cn>
> > > >
> > > > when the agent connection is lost, the input handler of the mouse
> > > > doesn't deactivate, which results in unresponsive mouse events in
> > > > VNC windows.
> > > >
> > > > To fix this issue, call vdagent_disconnect() to reset the state
> > > > each time the frontend disconncect
> > > >
> > > > Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn>
> > > > Signed-off-by: dengpengcheng <dengpc12@chinatelecom.cn>
> > > > ---
> > > >    ui/vdagent.c | 4 +++-
> > > >    1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/ui/vdagent.c b/ui/vdagent.c
> > > > index 8a651492f0..386dc5abe0 100644
> > > > --- a/ui/vdagent.c
> > > > +++ b/ui/vdagent.c
> > > > @@ -870,8 +870,11 @@ static void vdagent_disconnect(VDAgentChardev *vd)
> > > >
> > > >    static void vdagent_chr_set_fe_open(struct Chardev *chr, int fe_open)
> > > >    {
> > > > +    VDAgentChardev *vd = QEMU_VDAGENT_CHARDEV(chr);
> > > > +
> > > >        if (!fe_open) {
> > > >            trace_vdagent_close();
> > > > +        vdagent_disconnect(vd);
> > > >            /* To reset_serial, we CLOSED our side. Make sure the other end 
> > > > knows we
> > > >             * are ready again. */
> > > >            qemu_chr_be_event(chr, CHR_EVENT_OPENED);
> > > > @@ -922,7 +925,6 @@ static void vdagent_chr_fini(Object *obj)
> > > >        VDAgentChardev *vd = QEMU_VDAGENT_CHARDEV(obj);
> > > >
> > > >        migrate_del_blocker(vd->migration_blocker);
> > > > -    vdagent_disconnect(vd);
> > >
> > > why remove this cleanup ? (the function seems safe to call multiple
> > > times, if it is the case during finalize)
> > Before the instance finalize, the connection will be closed and the
> > vdagent_chr_set_fe_open() will be called with be_open set to false,
> > which in turn call vdagent_disconnect to clean up resources.
>
> Let's leave it, as it's more straightforward and safe, even if it's
> called two times in some code paths.
Okay, it sounds reasonable, i will send another patches