[PATCH 1/1] chardev/char: fix qemu_chr_is_busy() check

[PATCH 1/1] chardev/char: fix qemu_chr_is_busy() check

[Roman Penyaev]

`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(-)

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

Re: [PATCH 1/1] chardev/char: fix qemu_chr_is_busy() check

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]

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
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2281 bytes --]

Re: [PATCH 1/1] chardev/char: fix qemu_chr_is_busy() check

[Roman Penyaev]

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 2099 bytes --]

Re: [PATCH 1/1] chardev/char: fix qemu_chr_is_busy() check

[Roman Penyaev]

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