| Message ID | 20241010092619.323489-1-r.peniaev@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] chardev/char: fix qemu_chr_is_busy() check | expand |
Hi Roman On Thu, Oct 10, 2024 at 1:28 PM Roman Penyaev <r.peniaev@gmail.com> wrote: > `mux_cnt` struct member never goes negative or decrements, > so mux chardev can be !busy only when there are no > frontends attached. This patch fixes the always-true > check. > > Fixes: a4afa548fc6d ("char: move front end handlers in CharBackend") > Signed-off-by: Roman Penyaev <r.peniaev@gmail.com> > Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com> > Cc: qemu-devel@nongnu.org Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> That would be worth some new tests for chardev removal. It seems to be lacking. And mux probably need extra fixing. I can take a look if you don't. thanks > --- > chardev/char.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/chardev/char.c b/chardev/char.c > index c0cc52824b48..f54dc3a86286 100644 > --- a/chardev/char.c > +++ b/chardev/char.c > @@ -333,7 +333,7 @@ static bool qemu_chr_is_busy(Chardev *s) > { > if (CHARDEV_IS_MUX(s)) { > MuxChardev *d = MUX_CHARDEV(s); > - return d->mux_cnt >= 0; > + return d->mux_cnt > 0; > } else { > return s->be != NULL; > } > -- > 2.34.1 > > >
On Thu, Oct 10, 2024, 12:20 Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > Hi Roman > > On Thu, Oct 10, 2024 at 1:28 PM Roman Penyaev <r.peniaev@gmail.com> wrote: > >> `mux_cnt` struct member never goes negative or decrements, >> so mux chardev can be !busy only when there are no >> frontends attached. This patch fixes the always-true >> check. >> >> Fixes: a4afa548fc6d ("char: move front end handlers in CharBackend") >> Signed-off-by: Roman Penyaev <r.peniaev@gmail.com> >> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com> >> Cc: qemu-devel@nongnu.org > > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > That would be worth some new tests for chardev removal. It seems to be > lacking. And mux probably need extra fixing. I can take a look if you don't. > I assume no one removes mux device, so no one observes the error. Please, go ahead, I'm not sure I do fully understand what exactly should be fixed. -- Roman
Hi Marc-André, On Thu, Oct 10, 2024 at 12:20 PM Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > > Hi Roman > > On Thu, Oct 10, 2024 at 1:28 PM Roman Penyaev <r.peniaev@gmail.com> wrote: >> >> `mux_cnt` struct member never goes negative or decrements, >> so mux chardev can be !busy only when there are no >> frontends attached. This patch fixes the always-true >> check. >> >> Fixes: a4afa548fc6d ("char: move front end handlers in CharBackend") >> Signed-off-by: Roman Penyaev <r.peniaev@gmail.com> >> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com> >> Cc: qemu-devel@nongnu.org > > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > That would be worth some new tests for chardev removal. It seems to be lacking. And mux probably need extra fixing. I can take a look if you don't. I've just sent an attempt to fix the removal of frontends, plus some test cases. Please take a look. Thanks. -- Roman
diff --git a/chardev/char.c b/chardev/char.c index c0cc52824b48..f54dc3a86286 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -333,7 +333,7 @@ static bool qemu_chr_is_busy(Chardev *s) { if (CHARDEV_IS_MUX(s)) { MuxChardev *d = MUX_CHARDEV(s); - return d->mux_cnt >= 0; + return d->mux_cnt > 0; } else { return s->be != NULL; }
`mux_cnt` struct member never goes negative or decrements, so mux chardev can be !busy only when there are no frontends attached. This patch fixes the always-true check. Fixes: a4afa548fc6d ("char: move front end handlers in CharBackend") Signed-off-by: Roman Penyaev <r.peniaev@gmail.com> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com> Cc: qemu-devel@nongnu.org --- chardev/char.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)