| Message ID | d285e8729552a6206ffa1cd4520fc8f9c6be5957.1692268800.git.tugy@chinatelecom.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ui/vdagent: Fix two bugs about disconnect event handling | expand |
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) > buffer_free(&vd->outbuf); > error_free(vd->migration_blocker); > } > -- > 2.27.0 > >
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. >> buffer_free(&vd->outbuf); >> error_free(vd->migration_blocker); >> } >> -- >> 2.27.0 >> >> > >
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.
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
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); buffer_free(&vd->outbuf); error_free(vd->migration_blocker); }