[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [qemu-s390x] [Qemu-devel] [PATCH v2 3/6] terminal3270: do not use ba
Re: [qemu-s390x] [Qemu-devel] [PATCH v2 3/6] terminal3270: do not use backend timer sources
Mon, 11 Feb 2019 10:57:01 +0100
On Mon, Feb 11, 2019 at 8:13 AM Peter Xu <address@hidden> wrote:
> On Thu, Feb 07, 2019 at 06:14:15PM +0100, Cornelia Huck wrote:
> > On Wed, 6 Feb 2019 18:43:25 +0100
> > Marc-André Lureau <address@hidden> wrote:
> > > terminal3270 uses the front-end side of the chardev. It shouldn't
> > > create sources from backend side context (with backend
> > > functions).
> > >
> > > send_timing_mark_cb calls qemu_chr_fe_write_all() which should be
> > > thread safe.
> > >
> > > This partially reverts changes from commit
> > > 2c716ba1506769c9be2caa02f0f6d6e7c00f4304.
> > >
> > > CC: Peter Xu <address@hidden>
> > > Signed-off-by: Marc-André Lureau <address@hidden>
> > > ---
> > > hw/char/terminal3270.c | 15 ++++++---------
> > > 1 file changed, 6 insertions(+), 9 deletions(-)
> > I've verified that 3270 continues to work as before.
> > Acked-by: Cornelia Huck <address@hidden>
> A pure question: is it broken before this patch?
> Asked since I don't understand this patch and what it tries to fix...
A front-end shouldn't use backend functions.
> E.g., in terminal_init(), qemu_chr_fe_set_handlers() is always passing
> NULL as chardev context so AFAICT this patch changes nothing from
> functional-wise for now. Meanwhile, if we pass in some non-NULL into
> it in the future, IMHO this patch would break it instead of fixing
> anything... anything I've missed?
If the frontend switches the context and creates timers with this
context, it should do it without using the backend functions.