[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] chardev: fix mess in OPENED/CLOSED events when
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH] chardev: fix mess in OPENED/CLOSED events when muxed |
Date: |
Mon, 5 Nov 2018 11:29:10 +0400 |
Hi
On Mon, Nov 5, 2018 at 11:22 AM Artem Pisarenko
<address@hidden> wrote:
>
> Sorry, I forgot to check unit tests. Although, it's very strange that this
> specific test failed while things work functionally...
>
> > I am a bit reluctant to take patches that don't actually "fix" things.
> >
> > Could you add some tests to demonstrate the problems?
>
> Ok
>
> >> @@ -257,6 +257,7 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
> >> {
> >> Chardev *s;
> >> int fe_open;
> >> + static __thread bool mux_reentered;
> >
> > Not very elegant. Maybe mux_chr_set_handlers() could call a refactored
> > internal chr_fe_set_handlers() with an extra arg "no_open_event" ?
>
> Agree. It would make this hack more elegant. Although, it may become
> irrelevant as soon as I'll find soultion for failed docker test...
>
> >> @@ -272,21 +272,24 @@ static void char_mux_finalize(Object *obj)
> >> for (i = 0; i < d->mux_cnt; i++) {
> >> CharBackend *be = d->backends[i];
> >> if (be) {
> >> + if (be->chr && be->chr->be_open) {
> >> + qemu_chr_be_event(be->chr, CHR_EVENT_CLOSED);
> >> + }
> >
> > It looks like this could be a seperate patch, with a seperate test.
>
> Why this should be separate ? Do you mean that overall "opened/closed"
> pairing fix should be separated to "opened" fix+test and "closed" fix+test ?
>
> Since in next version it cannot be single patch anymore, how should I proceed
> with it ? Should I publish it as patch series with same subject marked as V2
> or start new patch series?
A patch series would be great. You can keep the subject and make it v2.
thanks