* [PATCH 1/8] chardev/char: fix qemu_chr_is_busy() check
2024-10-14 11:41 [PATCH 0/8] chardev/mux: implement frontend detach Roman Penyaev
@ 2024-10-14 11:41 ` Roman Penyaev
2024-10-14 11:41 ` [PATCH 2/8] chardev/chardev-internal: remove unused `max_size` struct member Roman Penyaev
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Roman Penyaev @ 2024-10-14 11:41 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
`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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/8] chardev/chardev-internal: remove unused `max_size` struct member
2024-10-14 11:41 [PATCH 0/8] chardev/mux: implement frontend detach Roman Penyaev
2024-10-14 11:41 ` [PATCH 1/8] chardev/char: fix qemu_chr_is_busy() check Roman Penyaev
@ 2024-10-14 11:41 ` Roman Penyaev
2024-10-14 11:41 ` [PATCH 3/8] chardev/mux: use bool type for `linestart` and `term_got_escape` Roman Penyaev
` (6 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Roman Penyaev @ 2024-10-14 11:41 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
Clean up forgotten leftovers.
Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org
---
chardev/chardev-internal.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index 4e03af31476c..c3024b51fdda 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -40,7 +40,6 @@ struct MuxChardev {
int focus;
int mux_cnt;
int term_got_escape;
- int max_size;
/* Intermediate input buffer catches escape sequences even if the
currently active device is not accepting any input - but only until it
is full as well. */
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/8] chardev/mux: use bool type for `linestart` and `term_got_escape`
2024-10-14 11:41 [PATCH 0/8] chardev/mux: implement frontend detach Roman Penyaev
2024-10-14 11:41 ` [PATCH 1/8] chardev/char: fix qemu_chr_is_busy() check Roman Penyaev
2024-10-14 11:41 ` [PATCH 2/8] chardev/chardev-internal: remove unused `max_size` struct member Roman Penyaev
@ 2024-10-14 11:41 ` Roman Penyaev
2024-10-14 11:41 ` [PATCH 4/8] chardev/mux: convert size members to unsigned int Roman Penyaev
` (5 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Roman Penyaev @ 2024-10-14 11:41 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
Those are boolean variables, not signed integers.
Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org
---
chardev/char-mux.c | 10 +++++-----
chardev/chardev-internal.h | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index ee2d47b20d9b..728596c6f346 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -73,11 +73,11 @@ static int mux_chr_write(Chardev *chr, const uint8_t *buf, int len)
* qemu_chr_fe_write and background I/O callbacks */
qemu_chr_fe_write_all(&d->chr,
(uint8_t *)buf1, strlen(buf1));
- d->linestart = 0;
+ d->linestart = false;
}
ret += qemu_chr_fe_write(&d->chr, buf + i, 1);
if (buf[i] == '\n') {
- d->linestart = 1;
+ d->linestart = true;
}
}
}
@@ -145,7 +145,7 @@ static void mux_chr_be_event(Chardev *chr, QEMUChrEvent event)
static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)
{
if (d->term_got_escape) {
- d->term_got_escape = 0;
+ d->term_got_escape = false;
if (ch == term_escape_char) {
goto send_char;
}
@@ -175,11 +175,11 @@ static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)
case 't':
d->timestamps = !d->timestamps;
d->timestamps_start = -1;
- d->linestart = 0;
+ d->linestart = false;
break;
}
} else if (ch == term_escape_char) {
- d->term_got_escape = 1;
+ d->term_got_escape = true;
} else {
send_char:
return 1;
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index c3024b51fdda..975c16de803e 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -39,7 +39,7 @@ struct MuxChardev {
CharBackend chr;
int focus;
int mux_cnt;
- int term_got_escape;
+ bool term_got_escape;
/* Intermediate input buffer catches escape sequences even if the
currently active device is not accepting any input - but only until it
is full as well. */
@@ -49,7 +49,7 @@ struct MuxChardev {
int timestamps;
/* Protected by the Chardev chr_write_lock. */
- int linestart;
+ bool linestart;
int64_t timestamps_start;
};
typedef struct MuxChardev MuxChardev;
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/8] chardev/mux: convert size members to unsigned int
2024-10-14 11:41 [PATCH 0/8] chardev/mux: implement frontend detach Roman Penyaev
` (2 preceding siblings ...)
2024-10-14 11:41 ` [PATCH 3/8] chardev/mux: use bool type for `linestart` and `term_got_escape` Roman Penyaev
@ 2024-10-14 11:41 ` Roman Penyaev
2024-10-14 11:41 ` [PATCH 5/8] chardev/mux: introduce `mux_chr_attach_frontend() call Roman Penyaev
` (4 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Roman Penyaev @ 2024-10-14 11:41 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
There is no sense to keep `focus`, `mux_cnt`, `prod`, `cons`
and `tag` variables as signed, those represent either size,
either position in array, which both are unsigned.
`focus` member of `MuxChardev` is kept signed, because initially
set to -1.
Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org
---
chardev/char-fe.c | 2 +-
chardev/char-mux.c | 10 +++++-----
chardev/chardev-internal.h | 8 ++++----
include/chardev/char-fe.h | 2 +-
4 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index b214ba3802b1..69b47d16bdfa 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -191,7 +191,7 @@ bool qemu_chr_fe_backend_open(CharBackend *be)
bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
{
- int tag = 0;
+ unsigned int tag = 0;
if (s) {
if (CHARDEV_IS_MUX(s)) {
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 728596c6f346..b2d7abf2fc01 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -124,7 +124,8 @@ static void mux_print_help(Chardev *chr)
}
}
-static void mux_chr_send_event(MuxChardev *d, int mux_nr, QEMUChrEvent event)
+static void mux_chr_send_event(MuxChardev *d, unsigned int mux_nr,
+ QEMUChrEvent event)
{
CharBackend *be = d->backends[mux_nr];
@@ -242,7 +243,7 @@ static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event)
{
MuxChardev *d = MUX_CHARDEV(chr);
- int i;
+ unsigned int i;
if (!muxes_opened) {
return;
@@ -275,7 +276,7 @@ static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
static void char_mux_finalize(Object *obj)
{
MuxChardev *d = MUX_CHARDEV(obj);
- int i;
+ unsigned int i;
for (i = 0; i < d->mux_cnt; i++) {
CharBackend *be = d->backends[i];
@@ -300,11 +301,10 @@ static void mux_chr_update_read_handlers(Chardev *chr)
chr->gcontext, true, false);
}
-void mux_set_focus(Chardev *chr, int focus)
+void mux_set_focus(Chardev *chr, unsigned int focus)
{
MuxChardev *d = MUX_CHARDEV(chr);
- assert(focus >= 0);
assert(focus < d->mux_cnt);
if (d->focus != -1) {
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index 975c16de803e..ab93f6ea1720 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -38,14 +38,14 @@ struct MuxChardev {
CharBackend *backends[MAX_MUX];
CharBackend chr;
int focus;
- int mux_cnt;
+ unsigned int mux_cnt;
bool term_got_escape;
/* Intermediate input buffer catches escape sequences even if the
currently active device is not accepting any input - but only until it
is full as well. */
unsigned char buffer[MAX_MUX][MUX_BUFFER_SIZE];
- int prod[MAX_MUX];
- int cons[MAX_MUX];
+ unsigned int prod[MAX_MUX];
+ unsigned int cons[MAX_MUX];
int timestamps;
/* Protected by the Chardev chr_write_lock. */
@@ -59,7 +59,7 @@ DECLARE_INSTANCE_CHECKER(MuxChardev, MUX_CHARDEV,
#define CHARDEV_IS_MUX(chr) \
object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX)
-void mux_set_focus(Chardev *chr, int focus);
+void mux_set_focus(Chardev *chr, unsigned int focus);
void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
Object *get_chardevs_root(void);
diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index 3310449eaf03..8ef05b3dd095 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -20,7 +20,7 @@ struct CharBackend {
IOReadHandler *chr_read;
BackendChangeHandler *chr_be_change;
void *opaque;
- int tag;
+ unsigned int tag;
bool fe_is_open;
};
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 5/8] chardev/mux: introduce `mux_chr_attach_frontend() call
2024-10-14 11:41 [PATCH 0/8] chardev/mux: implement frontend detach Roman Penyaev
` (3 preceding siblings ...)
2024-10-14 11:41 ` [PATCH 4/8] chardev/mux: convert size members to unsigned int Roman Penyaev
@ 2024-10-14 11:41 ` Roman Penyaev
2024-10-14 11:41 ` [PATCH 6/8] chardev/mux: switch mux frontends management to bitset Roman Penyaev
` (3 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Roman Penyaev @ 2024-10-14 11:41 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
Move away logic which attaches frontend device to a mux
from `char-fe.c` to actual `char-mux.c` implementation
and make it a separate function.
No logic changes are made.
Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org
---
chardev/char-fe.c | 9 +--------
chardev/char-mux.c | 17 +++++++++++++++++
chardev/chardev-internal.h | 2 ++
3 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 69b47d16bdfa..3b8771ca2ac4 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -197,16 +197,9 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
if (CHARDEV_IS_MUX(s)) {
MuxChardev *d = MUX_CHARDEV(s);
- if (d->mux_cnt >= MAX_MUX) {
- error_setg(errp,
- "too many uses of multiplexed chardev '%s'"
- " (maximum is " stringify(MAX_MUX) ")",
- s->label);
+ if (!mux_chr_attach_frontend(d, b, &tag, errp)) {
return false;
}
-
- d->backends[d->mux_cnt] = b;
- tag = d->mux_cnt++;
} else if (s->be) {
error_setg(errp, "chardev '%s' is already in use", s->label);
return false;
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index b2d7abf2fc01..9294f955462e 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -301,6 +301,23 @@ static void mux_chr_update_read_handlers(Chardev *chr)
chr->gcontext, true, false);
}
+bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
+ unsigned int *tag, Error **errp)
+{
+ if (d->mux_cnt >= MAX_MUX) {
+ error_setg(errp,
+ "too many uses of multiplexed chardev '%s'"
+ " (maximum is " stringify(MAX_MUX) ")",
+ d->parent.label);
+ return false;
+ }
+
+ d->backends[d->mux_cnt] = b;
+ *tag = d->mux_cnt++;
+
+ return true;
+}
+
void mux_set_focus(Chardev *chr, unsigned int focus)
{
MuxChardev *d = MUX_CHARDEV(chr);
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index ab93f6ea1720..8126ce180690 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -59,6 +59,8 @@ DECLARE_INSTANCE_CHECKER(MuxChardev, MUX_CHARDEV,
#define CHARDEV_IS_MUX(chr) \
object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX)
+bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
+ unsigned int *tag, Error **errp);
void mux_set_focus(Chardev *chr, unsigned int focus);
void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 6/8] chardev/mux: switch mux frontends management to bitset
2024-10-14 11:41 [PATCH 0/8] chardev/mux: implement frontend detach Roman Penyaev
` (4 preceding siblings ...)
2024-10-14 11:41 ` [PATCH 5/8] chardev/mux: introduce `mux_chr_attach_frontend() call Roman Penyaev
@ 2024-10-14 11:41 ` Roman Penyaev
2024-10-14 13:20 ` Marc-André Lureau
2024-10-14 11:41 ` [PATCH 7/8] chardev/mux: implement detach of frontends from mux Roman Penyaev
` (2 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Roman Penyaev @ 2024-10-14 11:41 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
Frontends can be attached and detached during run-time (although detach
is not implemented, but will follow). Counter variable of muxes is not
enough for proper attach/detach management, so this patch implements
bitset: if bit is set for the `mux_bitset` variable, then frontend
device can be found in the `backend` array (yes, huge confusion with
backend and frontends names).
Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org
---
chardev/char-mux.c | 41 +++++++++++++++++++++++++-------------
chardev/char.c | 2 +-
chardev/chardev-internal.h | 2 +-
3 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 9294f955462e..9c3cacb2fecd 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -26,6 +26,7 @@
#include "qapi/error.h"
#include "qemu/module.h"
#include "qemu/option.h"
+#include "qemu/bitops.h"
#include "chardev/char.h"
#include "sysemu/block-backend.h"
#include "qapi/qapi-commands-control.h"
@@ -168,12 +169,19 @@ static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)
case 'b':
qemu_chr_be_event(chr, CHR_EVENT_BREAK);
break;
- case 'c':
- assert(d->mux_cnt > 0); /* handler registered with first fe */
+ case 'c': {
+ unsigned int bit;
+
+ /* Handler registered with first fe */
+ assert(d->mux_bitset != 0);
/* Switch to the next registered device */
- mux_set_focus(chr, (d->focus + 1) % d->mux_cnt);
+ bit = find_next_bit(&d->mux_bitset, MAX_MUX, d->focus + 1);
+ if (bit >= MAX_MUX) {
+ bit = find_next_bit(&d->mux_bitset, MAX_MUX, 0);
+ }
+ mux_set_focus(chr, bit);
break;
- case 't':
+ } case 't':
d->timestamps = !d->timestamps;
d->timestamps_start = -1;
d->linestart = false;
@@ -243,15 +250,16 @@ static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event)
{
MuxChardev *d = MUX_CHARDEV(chr);
- unsigned int i;
+ int bit;
if (!muxes_opened) {
return;
}
/* Send the event to all registered listeners */
- for (i = 0; i < d->mux_cnt; i++) {
- mux_chr_send_event(d, i, event);
+ bit = -1;
+ while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) < MAX_MUX) {
+ mux_chr_send_event(d, bit, event);
}
}
@@ -276,10 +284,11 @@ static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
static void char_mux_finalize(Object *obj)
{
MuxChardev *d = MUX_CHARDEV(obj);
- unsigned int i;
+ int bit;
- for (i = 0; i < d->mux_cnt; i++) {
- CharBackend *be = d->backends[i];
+ bit = -1;
+ while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) < MAX_MUX) {
+ CharBackend *be = d->backends[bit];
if (be) {
be->chr = NULL;
}
@@ -304,7 +313,10 @@ static void mux_chr_update_read_handlers(Chardev *chr)
bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
unsigned int *tag, Error **errp)
{
- if (d->mux_cnt >= MAX_MUX) {
+ unsigned int bit;
+
+ bit = find_next_zero_bit(&d->mux_bitset, MAX_MUX, 0);
+ if (bit >= MAX_MUX) {
error_setg(errp,
"too many uses of multiplexed chardev '%s'"
" (maximum is " stringify(MAX_MUX) ")",
@@ -312,8 +324,9 @@ bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
return false;
}
- d->backends[d->mux_cnt] = b;
- *tag = d->mux_cnt++;
+ d->mux_bitset |= (1 << bit);
+ d->backends[bit] = b;
+ *tag = bit;
return true;
}
@@ -322,7 +335,7 @@ void mux_set_focus(Chardev *chr, unsigned int focus)
{
MuxChardev *d = MUX_CHARDEV(chr);
- assert(focus < d->mux_cnt);
+ assert(find_next_bit(&d->mux_bitset, MAX_MUX, focus) < MAX_MUX);
if (d->focus != -1) {
mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_OUT);
diff --git a/chardev/char.c b/chardev/char.c
index f54dc3a86286..a1722aa076d9 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_bitset != 0;
} else {
return s->be != NULL;
}
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index 8126ce180690..b89aada5413b 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -37,8 +37,8 @@ struct MuxChardev {
Chardev parent;
CharBackend *backends[MAX_MUX];
CharBackend chr;
+ unsigned long mux_bitset;
int focus;
- unsigned int mux_cnt;
bool term_got_escape;
/* Intermediate input buffer catches escape sequences even if the
currently active device is not accepting any input - but only until it
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 6/8] chardev/mux: switch mux frontends management to bitset
2024-10-14 11:41 ` [PATCH 6/8] chardev/mux: switch mux frontends management to bitset Roman Penyaev
@ 2024-10-14 13:20 ` Marc-André Lureau
2024-10-14 13:56 ` Roman Penyaev
0 siblings, 1 reply; 15+ messages in thread
From: Marc-André Lureau @ 2024-10-14 13:20 UTC (permalink / raw)
To: Roman Penyaev; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5784 bytes --]
On Mon, Oct 14, 2024 at 3:45 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
> Frontends can be attached and detached during run-time (although detach
> is not implemented, but will follow). Counter variable of muxes is not
> enough for proper attach/detach management, so this patch implements
> bitset: if bit is set for the `mux_bitset` variable, then frontend
> device can be found in the `backend` array (yes, huge confusion with
> backend and frontends names).
>
> Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> Cc: qemu-devel@nongnu.org
> ---
> chardev/char-mux.c | 41 +++++++++++++++++++++++++-------------
> chardev/char.c | 2 +-
> chardev/chardev-internal.h | 2 +-
> 3 files changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> index 9294f955462e..9c3cacb2fecd 100644
> --- a/chardev/char-mux.c
> +++ b/chardev/char-mux.c
> @@ -26,6 +26,7 @@
> #include "qapi/error.h"
> #include "qemu/module.h"
> #include "qemu/option.h"
> +#include "qemu/bitops.h"
> #include "chardev/char.h"
> #include "sysemu/block-backend.h"
> #include "qapi/qapi-commands-control.h"
> @@ -168,12 +169,19 @@ static int mux_proc_byte(Chardev *chr, MuxChardev
> *d, int ch)
> case 'b':
> qemu_chr_be_event(chr, CHR_EVENT_BREAK);
> break;
> - case 'c':
> - assert(d->mux_cnt > 0); /* handler registered with first fe */
> + case 'c': {
> + unsigned int bit;
> +
> + /* Handler registered with first fe */
> + assert(d->mux_bitset != 0);
> /* Switch to the next registered device */
> - mux_set_focus(chr, (d->focus + 1) % d->mux_cnt);
> + bit = find_next_bit(&d->mux_bitset, MAX_MUX, d->focus + 1);
> + if (bit >= MAX_MUX) {
> + bit = find_next_bit(&d->mux_bitset, MAX_MUX, 0);
> + }
> + mux_set_focus(chr, bit);
> break;
> - case 't':
> + } case 't':
> d->timestamps = !d->timestamps;
> d->timestamps_start = -1;
> d->linestart = false;
> @@ -243,15 +250,16 @@ static void mux_chr_read(void *opaque, const uint8_t
> *buf, int size)
> void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event)
> {
> MuxChardev *d = MUX_CHARDEV(chr);
> - unsigned int i;
> + int bit;
>
> if (!muxes_opened) {
> return;
> }
>
> /* Send the event to all registered listeners */
> - for (i = 0; i < d->mux_cnt; i++) {
> - mux_chr_send_event(d, i, event);
> + bit = -1;
> + while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) <
> MAX_MUX) {
> + mux_chr_send_event(d, bit, event);
> }
> }
>
> @@ -276,10 +284,11 @@ static GSource *mux_chr_add_watch(Chardev *s,
> GIOCondition cond)
> static void char_mux_finalize(Object *obj)
> {
> MuxChardev *d = MUX_CHARDEV(obj);
> - unsigned int i;
> + int bit;
>
> - for (i = 0; i < d->mux_cnt; i++) {
> - CharBackend *be = d->backends[i];
> + bit = -1;
> + while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) <
> MAX_MUX) {
> + CharBackend *be = d->backends[bit];
> if (be) {
> be->chr = NULL;
> }
> @@ -304,7 +313,10 @@ static void mux_chr_update_read_handlers(Chardev *chr)
> bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
> unsigned int *tag, Error **errp)
> {
> - if (d->mux_cnt >= MAX_MUX) {
> + unsigned int bit;
> +
> + bit = find_next_zero_bit(&d->mux_bitset, MAX_MUX, 0);
> + if (bit >= MAX_MUX) {
> error_setg(errp,
> "too many uses of multiplexed chardev '%s'"
> " (maximum is " stringify(MAX_MUX) ")",
> @@ -312,8 +324,9 @@ bool mux_chr_attach_frontend(MuxChardev *d,
> CharBackend *b,
> return false;
> }
>
> - d->backends[d->mux_cnt] = b;
> - *tag = d->mux_cnt++;
> + d->mux_bitset |= (1 << bit);
> + d->backends[bit] = b;
> + *tag = bit;
>
> return true;
> }
> @@ -322,7 +335,7 @@ void mux_set_focus(Chardev *chr, unsigned int focus)
> {
> MuxChardev *d = MUX_CHARDEV(chr);
>
> - assert(focus < d->mux_cnt);
> + assert(find_next_bit(&d->mux_bitset, MAX_MUX, focus) < MAX_MUX);
>
Wouldn't this be more correct?
+ assert(find_next_bit(&d->mux_bitset, MAX_MUX, focus) == focus);
>
> if (d->focus != -1) {
> mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_OUT);
> diff --git a/chardev/char.c b/chardev/char.c
> index f54dc3a86286..a1722aa076d9 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_bitset != 0;
> } else {
> return s->be != NULL;
> }
> diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
> index 8126ce180690..b89aada5413b 100644
> --- a/chardev/chardev-internal.h
> +++ b/chardev/chardev-internal.h
> @@ -37,8 +37,8 @@ struct MuxChardev {
> Chardev parent;
> CharBackend *backends[MAX_MUX];
> CharBackend chr;
> + unsigned long mux_bitset;
> int focus;
> - unsigned int mux_cnt;
> bool term_got_escape;
> /* Intermediate input buffer catches escape sequences even if the
> currently active device is not accepting any input - but only
> until it
> --
> 2.34.1
>
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 7573 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 6/8] chardev/mux: switch mux frontends management to bitset
2024-10-14 13:20 ` Marc-André Lureau
@ 2024-10-14 13:56 ` Roman Penyaev
2024-10-14 14:02 ` Marc-André Lureau
0 siblings, 1 reply; 15+ messages in thread
From: Roman Penyaev @ 2024-10-14 13:56 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel
On Mon, Oct 14, 2024 at 3:20 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
>
>
> On Mon, Oct 14, 2024 at 3:45 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
>>
>> Frontends can be attached and detached during run-time (although detach
>> is not implemented, but will follow). Counter variable of muxes is not
>> enough for proper attach/detach management, so this patch implements
>> bitset: if bit is set for the `mux_bitset` variable, then frontend
>> device can be found in the `backend` array (yes, huge confusion with
>> backend and frontends names).
>>
>> Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
>> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
>> Cc: qemu-devel@nongnu.org
>> ---
>> chardev/char-mux.c | 41 +++++++++++++++++++++++++-------------
>> chardev/char.c | 2 +-
>> chardev/chardev-internal.h | 2 +-
>> 3 files changed, 29 insertions(+), 16 deletions(-)
>>
>> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
>> index 9294f955462e..9c3cacb2fecd 100644
>> --- a/chardev/char-mux.c
>> +++ b/chardev/char-mux.c
>> @@ -26,6 +26,7 @@
>> #include "qapi/error.h"
>> #include "qemu/module.h"
>> #include "qemu/option.h"
>> +#include "qemu/bitops.h"
>> #include "chardev/char.h"
>> #include "sysemu/block-backend.h"
>> #include "qapi/qapi-commands-control.h"
>> @@ -168,12 +169,19 @@ static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)
>> case 'b':
>> qemu_chr_be_event(chr, CHR_EVENT_BREAK);
>> break;
>> - case 'c':
>> - assert(d->mux_cnt > 0); /* handler registered with first fe */
>> + case 'c': {
>> + unsigned int bit;
>> +
>> + /* Handler registered with first fe */
>> + assert(d->mux_bitset != 0);
>> /* Switch to the next registered device */
>> - mux_set_focus(chr, (d->focus + 1) % d->mux_cnt);
>> + bit = find_next_bit(&d->mux_bitset, MAX_MUX, d->focus + 1);
>> + if (bit >= MAX_MUX) {
>> + bit = find_next_bit(&d->mux_bitset, MAX_MUX, 0);
>> + }
>> + mux_set_focus(chr, bit);
>> break;
>> - case 't':
>> + } case 't':
>> d->timestamps = !d->timestamps;
>> d->timestamps_start = -1;
>> d->linestart = false;
>> @@ -243,15 +250,16 @@ static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
>> void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event)
>> {
>> MuxChardev *d = MUX_CHARDEV(chr);
>> - unsigned int i;
>> + int bit;
>>
>> if (!muxes_opened) {
>> return;
>> }
>>
>> /* Send the event to all registered listeners */
>> - for (i = 0; i < d->mux_cnt; i++) {
>> - mux_chr_send_event(d, i, event);
>> + bit = -1;
>> + while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) < MAX_MUX) {
>> + mux_chr_send_event(d, bit, event);
>> }
>> }
>>
>> @@ -276,10 +284,11 @@ static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
>> static void char_mux_finalize(Object *obj)
>> {
>> MuxChardev *d = MUX_CHARDEV(obj);
>> - unsigned int i;
>> + int bit;
>>
>> - for (i = 0; i < d->mux_cnt; i++) {
>> - CharBackend *be = d->backends[i];
>> + bit = -1;
>> + while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) < MAX_MUX) {
>> + CharBackend *be = d->backends[bit];
>> if (be) {
>> be->chr = NULL;
>> }
>> @@ -304,7 +313,10 @@ static void mux_chr_update_read_handlers(Chardev *chr)
>> bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
>> unsigned int *tag, Error **errp)
>> {
>> - if (d->mux_cnt >= MAX_MUX) {
>> + unsigned int bit;
>> +
>> + bit = find_next_zero_bit(&d->mux_bitset, MAX_MUX, 0);
>> + if (bit >= MAX_MUX) {
>> error_setg(errp,
>> "too many uses of multiplexed chardev '%s'"
>> " (maximum is " stringify(MAX_MUX) ")",
>> @@ -312,8 +324,9 @@ bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
>> return false;
>> }
>>
>> - d->backends[d->mux_cnt] = b;
>> - *tag = d->mux_cnt++;
>> + d->mux_bitset |= (1 << bit);
>> + d->backends[bit] = b;
>> + *tag = bit;
>>
>> return true;
>> }
>> @@ -322,7 +335,7 @@ void mux_set_focus(Chardev *chr, unsigned int focus)
>> {
>> MuxChardev *d = MUX_CHARDEV(chr);
>>
>> - assert(focus < d->mux_cnt);
>> + assert(find_next_bit(&d->mux_bitset, MAX_MUX, focus) < MAX_MUX);
>
>
> Wouldn't this be more correct?
>
> + assert(find_next_bit(&d->mux_bitset, MAX_MUX, focus) == focus);
>
Yes, makes sense. Thanks.
Do you want the whole patchset to be resend,
or only changed patches with "v2" prefix in subject?
--
Roman
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 6/8] chardev/mux: switch mux frontends management to bitset
2024-10-14 13:56 ` Roman Penyaev
@ 2024-10-14 14:02 ` Marc-André Lureau
0 siblings, 0 replies; 15+ messages in thread
From: Marc-André Lureau @ 2024-10-14 14:02 UTC (permalink / raw)
To: Roman Penyaev; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5554 bytes --]
On Mon, Oct 14, 2024 at 5:58 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
> On Mon, Oct 14, 2024 at 3:20 PM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> >
> >
> > On Mon, Oct 14, 2024 at 3:45 PM Roman Penyaev <r.peniaev@gmail.com>
> wrote:
> >>
> >> Frontends can be attached and detached during run-time (although detach
> >> is not implemented, but will follow). Counter variable of muxes is not
> >> enough for proper attach/detach management, so this patch implements
> >> bitset: if bit is set for the `mux_bitset` variable, then frontend
> >> device can be found in the `backend` array (yes, huge confusion with
> >> backend and frontends names).
> >>
> >> Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
> >> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> >> Cc: qemu-devel@nongnu.org
> >> ---
> >> chardev/char-mux.c | 41 +++++++++++++++++++++++++-------------
> >> chardev/char.c | 2 +-
> >> chardev/chardev-internal.h | 2 +-
> >> 3 files changed, 29 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> >> index 9294f955462e..9c3cacb2fecd 100644
> >> --- a/chardev/char-mux.c
> >> +++ b/chardev/char-mux.c
> >> @@ -26,6 +26,7 @@
> >> #include "qapi/error.h"
> >> #include "qemu/module.h"
> >> #include "qemu/option.h"
> >> +#include "qemu/bitops.h"
> >> #include "chardev/char.h"
> >> #include "sysemu/block-backend.h"
> >> #include "qapi/qapi-commands-control.h"
> >> @@ -168,12 +169,19 @@ static int mux_proc_byte(Chardev *chr, MuxChardev
> *d, int ch)
> >> case 'b':
> >> qemu_chr_be_event(chr, CHR_EVENT_BREAK);
> >> break;
> >> - case 'c':
> >> - assert(d->mux_cnt > 0); /* handler registered with first
> fe */
> >> + case 'c': {
> >> + unsigned int bit;
> >> +
> >> + /* Handler registered with first fe */
> >> + assert(d->mux_bitset != 0);
> >> /* Switch to the next registered device */
> >> - mux_set_focus(chr, (d->focus + 1) % d->mux_cnt);
> >> + bit = find_next_bit(&d->mux_bitset, MAX_MUX, d->focus + 1);
> >> + if (bit >= MAX_MUX) {
> >> + bit = find_next_bit(&d->mux_bitset, MAX_MUX, 0);
> >> + }
> >> + mux_set_focus(chr, bit);
> >> break;
> >> - case 't':
> >> + } case 't':
> >> d->timestamps = !d->timestamps;
> >> d->timestamps_start = -1;
> >> d->linestart = false;
> >> @@ -243,15 +250,16 @@ static void mux_chr_read(void *opaque, const
> uint8_t *buf, int size)
> >> void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event)
> >> {
> >> MuxChardev *d = MUX_CHARDEV(chr);
> >> - unsigned int i;
> >> + int bit;
> >>
> >> if (!muxes_opened) {
> >> return;
> >> }
> >>
> >> /* Send the event to all registered listeners */
> >> - for (i = 0; i < d->mux_cnt; i++) {
> >> - mux_chr_send_event(d, i, event);
> >> + bit = -1;
> >> + while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) <
> MAX_MUX) {
> >> + mux_chr_send_event(d, bit, event);
> >> }
> >> }
> >>
> >> @@ -276,10 +284,11 @@ static GSource *mux_chr_add_watch(Chardev *s,
> GIOCondition cond)
> >> static void char_mux_finalize(Object *obj)
> >> {
> >> MuxChardev *d = MUX_CHARDEV(obj);
> >> - unsigned int i;
> >> + int bit;
> >>
> >> - for (i = 0; i < d->mux_cnt; i++) {
> >> - CharBackend *be = d->backends[i];
> >> + bit = -1;
> >> + while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) <
> MAX_MUX) {
> >> + CharBackend *be = d->backends[bit];
> >> if (be) {
> >> be->chr = NULL;
> >> }
> >> @@ -304,7 +313,10 @@ static void mux_chr_update_read_handlers(Chardev
> *chr)
> >> bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
> >> unsigned int *tag, Error **errp)
> >> {
> >> - if (d->mux_cnt >= MAX_MUX) {
> >> + unsigned int bit;
> >> +
> >> + bit = find_next_zero_bit(&d->mux_bitset, MAX_MUX, 0);
> >> + if (bit >= MAX_MUX) {
> >> error_setg(errp,
> >> "too many uses of multiplexed chardev '%s'"
> >> " (maximum is " stringify(MAX_MUX) ")",
> >> @@ -312,8 +324,9 @@ bool mux_chr_attach_frontend(MuxChardev *d,
> CharBackend *b,
> >> return false;
> >> }
> >>
> >> - d->backends[d->mux_cnt] = b;
> >> - *tag = d->mux_cnt++;
> >> + d->mux_bitset |= (1 << bit);
> >> + d->backends[bit] = b;
> >> + *tag = bit;
> >>
> >> return true;
> >> }
> >> @@ -322,7 +335,7 @@ void mux_set_focus(Chardev *chr, unsigned int focus)
> >> {
> >> MuxChardev *d = MUX_CHARDEV(chr);
> >>
> >> - assert(focus < d->mux_cnt);
> >> + assert(find_next_bit(&d->mux_bitset, MAX_MUX, focus) < MAX_MUX);
> >
> >
> > Wouldn't this be more correct?
> >
> > + assert(find_next_bit(&d->mux_bitset, MAX_MUX, focus) == focus);
> >
>
> Yes, makes sense. Thanks.
>
> Do you want the whole patchset to be resend,
> or only changed patches with "v2" prefix in subject?
>
Yes, please send a v2 with the change and r-b. For the other series, use
Based-on tag as necessary (
https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01288.html)
thanks
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 8078 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 7/8] chardev/mux: implement detach of frontends from mux
2024-10-14 11:41 [PATCH 0/8] chardev/mux: implement frontend detach Roman Penyaev
` (5 preceding siblings ...)
2024-10-14 11:41 ` [PATCH 6/8] chardev/mux: switch mux frontends management to bitset Roman Penyaev
@ 2024-10-14 11:41 ` Roman Penyaev
2024-10-14 13:22 ` Marc-André Lureau
2024-10-14 11:41 ` [PATCH 8/8] tests/unit/test-char: implement a few mux remove test cases Roman Penyaev
2024-10-14 13:22 ` [PATCH 0/8] chardev/mux: implement frontend detach Marc-André Lureau
8 siblings, 1 reply; 15+ messages in thread
From: Roman Penyaev @ 2024-10-14 11:41 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
With bitset management now it becomes feasible to implement
the logic of detaching frontends from multiplexer.
Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org
---
chardev/char-fe.c | 2 +-
chardev/char-mux.c | 20 +++++++++++++++++---
chardev/chardev-internal.h | 1 +
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 3b8771ca2ac4..8ac6bebb6f74 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -225,7 +225,7 @@ void qemu_chr_fe_deinit(CharBackend *b, bool del)
}
if (CHARDEV_IS_MUX(b->chr)) {
MuxChardev *d = MUX_CHARDEV(b->chr);
- d->backends[b->tag] = NULL;
+ mux_chr_detach_frontend(d, b->tag);
}
if (del) {
Object *obj = OBJECT(b->chr);
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 9c3cacb2fecd..649f8ff6ccbf 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -289,10 +289,10 @@ static void char_mux_finalize(Object *obj)
bit = -1;
while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) < MAX_MUX) {
CharBackend *be = d->backends[bit];
- if (be) {
- be->chr = NULL;
- }
+ be->chr = NULL;
+ d->backends[bit] = NULL;
}
+ d->mux_bitset = 0;
qemu_chr_fe_deinit(&d->chr, false);
}
@@ -331,6 +331,21 @@ bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
return true;
}
+bool mux_chr_detach_frontend(MuxChardev *d, unsigned int tag)
+{
+ unsigned int bit;
+
+ bit = find_next_bit(&d->mux_bitset, MAX_MUX, tag);
+ if (bit >= MAX_MUX) {
+ return false;
+ }
+
+ d->mux_bitset &= ~(1 << bit);
+ d->backends[bit] = NULL;
+
+ return true;
+}
+
void mux_set_focus(Chardev *chr, unsigned int focus)
{
MuxChardev *d = MUX_CHARDEV(chr);
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index b89aada5413b..853807f3cb88 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -61,6 +61,7 @@ DECLARE_INSTANCE_CHECKER(MuxChardev, MUX_CHARDEV,
bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
unsigned int *tag, Error **errp);
+bool mux_chr_detach_frontend(MuxChardev *d, unsigned int tag);
void mux_set_focus(Chardev *chr, unsigned int focus);
void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 7/8] chardev/mux: implement detach of frontends from mux
2024-10-14 11:41 ` [PATCH 7/8] chardev/mux: implement detach of frontends from mux Roman Penyaev
@ 2024-10-14 13:22 ` Marc-André Lureau
2024-10-14 13:57 ` Roman Penyaev
0 siblings, 1 reply; 15+ messages in thread
From: Marc-André Lureau @ 2024-10-14 13:22 UTC (permalink / raw)
To: Roman Penyaev; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2894 bytes --]
Hi
On Mon, Oct 14, 2024 at 3:46 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
> With bitset management now it becomes feasible to implement
> the logic of detaching frontends from multiplexer.
>
> Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> Cc: qemu-devel@nongnu.org
> ---
> chardev/char-fe.c | 2 +-
> chardev/char-mux.c | 20 +++++++++++++++++---
> chardev/chardev-internal.h | 1 +
> 3 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> index 3b8771ca2ac4..8ac6bebb6f74 100644
> --- a/chardev/char-fe.c
> +++ b/chardev/char-fe.c
> @@ -225,7 +225,7 @@ void qemu_chr_fe_deinit(CharBackend *b, bool del)
> }
> if (CHARDEV_IS_MUX(b->chr)) {
> MuxChardev *d = MUX_CHARDEV(b->chr);
> - d->backends[b->tag] = NULL;
> + mux_chr_detach_frontend(d, b->tag);
>
}
> if (del) {
> Object *obj = OBJECT(b->chr);
> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> index 9c3cacb2fecd..649f8ff6ccbf 100644
> --- a/chardev/char-mux.c
> +++ b/chardev/char-mux.c
> @@ -289,10 +289,10 @@ static void char_mux_finalize(Object *obj)
> bit = -1;
> while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) <
> MAX_MUX) {
> CharBackend *be = d->backends[bit];
> - if (be) {
> - be->chr = NULL;
> - }
> + be->chr = NULL;
> + d->backends[bit] = NULL;
> }
> + d->mux_bitset = 0;
> qemu_chr_fe_deinit(&d->chr, false);
> }
>
> @@ -331,6 +331,21 @@ bool mux_chr_attach_frontend(MuxChardev *d,
> CharBackend *b,
> return true;
> }
>
> +bool mux_chr_detach_frontend(MuxChardev *d, unsigned int tag)
> +{
> + unsigned int bit;
> +
> + bit = find_next_bit(&d->mux_bitset, MAX_MUX, tag);
> + if (bit >= MAX_MUX) {
>
if (bit != tag) instead?
> + return false;
> + }
> +
> + d->mux_bitset &= ~(1 << bit);
> + d->backends[bit] = NULL;
> +
> + return true;
> +}
> +
> void mux_set_focus(Chardev *chr, unsigned int focus)
> {
> MuxChardev *d = MUX_CHARDEV(chr);
> diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
> index b89aada5413b..853807f3cb88 100644
> --- a/chardev/chardev-internal.h
> +++ b/chardev/chardev-internal.h
> @@ -61,6 +61,7 @@ DECLARE_INSTANCE_CHECKER(MuxChardev, MUX_CHARDEV,
>
> bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
> unsigned int *tag, Error **errp);
> +bool mux_chr_detach_frontend(MuxChardev *d, unsigned int tag);
> void mux_set_focus(Chardev *chr, unsigned int focus);
> void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
>
> --
> 2.34.1
>
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 4199 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 7/8] chardev/mux: implement detach of frontends from mux
2024-10-14 13:22 ` Marc-André Lureau
@ 2024-10-14 13:57 ` Roman Penyaev
0 siblings, 0 replies; 15+ messages in thread
From: Roman Penyaev @ 2024-10-14 13:57 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel
On Mon, Oct 14, 2024 at 3:22 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
[cut]
>>
>> +bool mux_chr_detach_frontend(MuxChardev *d, unsigned int tag)
>> +{
>> + unsigned int bit;
>> +
>> + bit = find_next_bit(&d->mux_bitset, MAX_MUX, tag);
>> + if (bit >= MAX_MUX) {
>
>
> if (bit != tag) instead?
Right, thanks.
--
Roman
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 8/8] tests/unit/test-char: implement a few mux remove test cases
2024-10-14 11:41 [PATCH 0/8] chardev/mux: implement frontend detach Roman Penyaev
` (6 preceding siblings ...)
2024-10-14 11:41 ` [PATCH 7/8] chardev/mux: implement detach of frontends from mux Roman Penyaev
@ 2024-10-14 11:41 ` Roman Penyaev
2024-10-14 13:22 ` [PATCH 0/8] chardev/mux: implement frontend detach Marc-André Lureau
8 siblings, 0 replies; 15+ messages in thread
From: Roman Penyaev @ 2024-10-14 11:41 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
This patch tests:
1. feasibility of removing mux which does not have frontends attached
or frontends were prior detached.
2. inability to remove mux which has frontends attached (mux is "busy")
Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org
---
tests/unit/test-char.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
index f273ce522612..2837dbb863a8 100644
--- a/tests/unit/test-char.c
+++ b/tests/unit/test-char.c
@@ -1,6 +1,7 @@
#include "qemu/osdep.h"
#include <glib/gstdio.h>
+#include "qapi/error.h"
#include "qemu/config-file.h"
#include "qemu/module.h"
#include "qemu/option.h"
@@ -184,6 +185,21 @@ static void char_mux_test(void)
char *data;
FeHandler h1 = { 0, false, 0, false, }, h2 = { 0, false, 0, false, };
CharBackend chr_be1, chr_be2;
+ Error *error = NULL;
+
+ /* Create mux and chardev to be immediately removed */
+ opts = qemu_opts_create(qemu_find_opts("chardev"), "mux-label",
+ 1, &error_abort);
+ qemu_opt_set(opts, "backend", "ringbuf", &error_abort);
+ qemu_opt_set(opts, "size", "128", &error_abort);
+ qemu_opt_set(opts, "mux", "on", &error_abort);
+ chr = qemu_chr_new_from_opts(opts, NULL, &error_abort);
+ g_assert_nonnull(chr);
+ qemu_opts_del(opts);
+
+ /* Remove just created mux and chardev */
+ qmp_chardev_remove("mux-label", &error_abort);
+ qmp_chardev_remove("mux-label-base", &error_abort);
opts = qemu_opts_create(qemu_find_opts("chardev"), "mux-label",
1, &error_abort);
@@ -334,7 +350,13 @@ static void char_mux_test(void)
g_free(data);
qemu_chr_fe_deinit(&chr_be1, false);
- qemu_chr_fe_deinit(&chr_be2, true);
+
+ error = NULL;
+ qmp_chardev_remove("mux-label", &error);
+ g_assert_cmpstr(error_get_pretty(error), ==, "Chardev 'mux-label' is busy");
+
+ qemu_chr_fe_deinit(&chr_be2, false);
+ qmp_chardev_remove("mux-label", &error_abort);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 0/8] chardev/mux: implement frontend detach
2024-10-14 11:41 [PATCH 0/8] chardev/mux: implement frontend detach Roman Penyaev
` (7 preceding siblings ...)
2024-10-14 11:41 ` [PATCH 8/8] tests/unit/test-char: implement a few mux remove test cases Roman Penyaev
@ 2024-10-14 13:22 ` Marc-André Lureau
8 siblings, 0 replies; 15+ messages in thread
From: Marc-André Lureau @ 2024-10-14 13:22 UTC (permalink / raw)
To: Roman Penyaev; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1740 bytes --]
Hi
On Mon, Oct 14, 2024 at 3:47 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
> Frontend device can be detached in run-time, which can lead to a
> "Chardev 'MUX' is busy" error (see the last patch with the test case
> implementation). This series implements frontend detach for the
> multiplexer based on bitset, which provides the ability to attach or
> detach frontend devices in any order.
>
> Also first patches do some refactoring the purpose of which is to make
> integer unsigned where possible (such as sizes or lengths).
>
> Roman Penyaev (8):
> chardev/char: fix qemu_chr_is_busy() check
> chardev/chardev-internal: remove unused `max_size` struct member
> chardev/mux: use bool type for `linestart` and `term_got_escape`
> chardev/mux: convert size members to unsigned int
> chardev/mux: introduce `mux_chr_attach_frontend() call
> chardev/mux: switch mux frontends management to bitset
> chardev/mux: implement detach of frontends from mux
> tests/unit/test-char: implement a few mux remove test cases
>
> chardev/char-fe.c | 13 ++----
> chardev/char-mux.c | 88 ++++++++++++++++++++++++++++----------
> chardev/char.c | 2 +-
> chardev/chardev-internal.h | 16 ++++---
> include/chardev/char-fe.h | 2 +-
> tests/unit/test-char.c | 24 ++++++++++-
> 6 files changed, 103 insertions(+), 42 deletions(-)
>
> Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> Cc: qemu-devel@nongnu.org
lgtm, with some pre-conditions that could be improved I belive
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
>
> --
> 2.34.1
>
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 2720 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread