[Qemu-devel] [PATCH v2 0/2] mux: fix ctrl-a b again

[Qemu-devel] [PATCH v2 0/2] mux: fix ctrl-a b again

[Philippe Mathieu-Daudé]

Hi, this series contains the rebased patches for the ctrl-a+b fix.

Please consider to include in qemu-stable.

Regards,

Phil.

v2:
- rebase on Peter serial cleanup:
  "Drop compile time limit on number of serial ports"
  http://lists.nongnu.org/archive/html/qemu-devel/2018-04/msg03376.html
- added ctrl-ab test (Marc-André)

v1 was http://lists.nongnu.org/archive/html/qemu-devel/2018-04/msg03168.html

Marc-André Lureau (1):
  mux: fix ctrl-a b again

Philippe Mathieu-Daudé (1):
  superio: Fix inconsistent use of Chardev->be

 chardev/char-mux.c   | 1 +
 hw/isa/isa-superio.c | 4 ++--
 tests/test-char.c    | 8 ++++++++
 3 files changed, 11 insertions(+), 2 deletions(-)

-- 
2.17.0

[Qemu-devel] [PATCH v2 1/2] hw/isa/superio: Fix inconsistent use of Chardev->be

[Philippe Mathieu-Daudé]

4c3119a6e3e and cd9526ab7c0 introduced an incorrect and inconsistent
use of Chardev->be. Also, this CharBackend member is private and is
not supposed to be accessible.

Fix it by removing the inconsistent check.

Cc: qemu-stable@nongnu.org
Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/isa/isa-superio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/isa/isa-superio.c b/hw/isa/isa-superio.c
index 76286c81a1..bb88251f95 100644
--- a/hw/isa/isa-superio.c
+++ b/hw/isa/isa-superio.c
@@ -43,7 +43,7 @@ static void isa_superio_realize(DeviceState *dev, Error **errp)
         if (!k->parallel.is_enabled || k->parallel.is_enabled(sio, i)) {
             /* FIXME use a qdev chardev prop instead of parallel_hds[] */
             chr = parallel_hds[i];
-            if (chr == NULL || chr->be) {
+            if (chr == NULL) {
                 name = g_strdup_printf("discarding-parallel%d", i);
                 chr = qemu_chr_new(name, "null");
             } else {
@@ -83,7 +83,7 @@ static void isa_superio_realize(DeviceState *dev, Error **errp)
         if (!k->serial.is_enabled || k->serial.is_enabled(sio, i)) {
             /* FIXME use a qdev chardev prop instead of serial_hd() */
             chr = serial_hd(i);
-            if (chr == NULL || chr->be) {
+            if (chr == NULL) {
                 name = g_strdup_printf("discarding-serial%d", i);
                 chr = qemu_chr_new(name, "null");
             } else {
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 1/2] hw/isa/superio: Fix inconsistent use of Chardev->be

[Marc-André Lureau]

On Tue, May 15, 2018 at 5:24 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> 4c3119a6e3e and cd9526ab7c0 introduced an incorrect and inconsistent
> use of Chardev->be. Also, this CharBackend member is private and is
> not supposed to be accessible.
>
> Fix it by removing the inconsistent check.
>
> Cc: qemu-stable@nongnu.org
> Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  hw/isa/isa-superio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/isa/isa-superio.c b/hw/isa/isa-superio.c
> index 76286c81a1..bb88251f95 100644
> --- a/hw/isa/isa-superio.c
> +++ b/hw/isa/isa-superio.c
> @@ -43,7 +43,7 @@ static void isa_superio_realize(DeviceState *dev, Error **errp)
>          if (!k->parallel.is_enabled || k->parallel.is_enabled(sio, i)) {
>              /* FIXME use a qdev chardev prop instead of parallel_hds[] */
>              chr = parallel_hds[i];
> -            if (chr == NULL || chr->be) {
> +            if (chr == NULL) {
>                  name = g_strdup_printf("discarding-parallel%d", i);
>                  chr = qemu_chr_new(name, "null");
>              } else {
> @@ -83,7 +83,7 @@ static void isa_superio_realize(DeviceState *dev, Error **errp)
>          if (!k->serial.is_enabled || k->serial.is_enabled(sio, i)) {
>              /* FIXME use a qdev chardev prop instead of serial_hd() */
>              chr = serial_hd(i);
> -            if (chr == NULL || chr->be) {
> +            if (chr == NULL) {
>                  name = g_strdup_printf("discarding-serial%d", i);
>                  chr = qemu_chr_new(name, "null");
>              } else {
> --
> 2.17.0
>

Re: [Qemu-devel] [PATCH v2 1/2] hw/isa/superio: Fix inconsistent use of Chardev->be

[Paolo Bonzini]

On 15/05/2018 17:24, Philippe Mathieu-Daudé wrote:
> 4c3119a6e3e and cd9526ab7c0 introduced an incorrect and inconsistent
> use of Chardev->be. Also, this CharBackend member is private and is
> not supposed to be accessible.
> 
> Fix it by removing the inconsistent check.
> 
> Cc: qemu-stable@nongnu.org
> Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/isa/isa-superio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/isa/isa-superio.c b/hw/isa/isa-superio.c
> index 76286c81a1..bb88251f95 100644
> --- a/hw/isa/isa-superio.c
> +++ b/hw/isa/isa-superio.c
> @@ -43,7 +43,7 @@ static void isa_superio_realize(DeviceState *dev, Error **errp)
>          if (!k->parallel.is_enabled || k->parallel.is_enabled(sio, i)) {
>              /* FIXME use a qdev chardev prop instead of parallel_hds[] */
>              chr = parallel_hds[i];
> -            if (chr == NULL || chr->be) {
> +            if (chr == NULL) {
>                  name = g_strdup_printf("discarding-parallel%d", i);
>                  chr = qemu_chr_new(name, "null");
>              } else {
> @@ -83,7 +83,7 @@ static void isa_superio_realize(DeviceState *dev, Error **errp)
>          if (!k->serial.is_enabled || k->serial.is_enabled(sio, i)) {
>              /* FIXME use a qdev chardev prop instead of serial_hd() */
>              chr = serial_hd(i);
> -            if (chr == NULL || chr->be) {
> +            if (chr == NULL) {
>                  name = g_strdup_printf("discarding-serial%d", i);
>                  chr = qemu_chr_new(name, "null");
>              } else {
> 

Queued, thanks.

Paolo

[Qemu-devel] [PATCH v2 2/2] mux: fix ctrl-a b again

[Philippe Mathieu-Daudé]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Commit fb5e19d2e1472e96d72d5e4d89c20033f8ab345c originally fixed the
regression, but was inadvertently broken again in merge commit
2d6752d38d8acda.

Fixes:
https://bugs.launchpad.net/qemu/+bug/1654137

Cc: qemu-stable@nongnu.org
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 chardev/char-mux.c | 1 +
 tests/test-char.c  | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 1b925c8dec..6055e76293 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -304,6 +304,7 @@ void mux_set_focus(Chardev *chr, int focus)
     }
 
     d->focus = focus;
+    chr->be = d->backends[focus];
     mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_IN);
 }
 
diff --git a/tests/test-char.c b/tests/test-char.c
index 1880d36783..5905d31441 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -214,6 +214,10 @@ static void char_mux_test(void)
     g_assert_cmpint(h2.last_event, ==, -1);
 
     /* switch focus */
+    qemu_chr_be_write(base, (void *)"\1b", 2);
+    g_assert_cmpint(h1.last_event, ==, 42);
+    g_assert_cmpint(h2.last_event, ==, CHR_EVENT_BREAK);
+
     qemu_chr_be_write(base, (void *)"\1c", 2);
     g_assert_cmpint(h1.last_event, ==, CHR_EVENT_MUX_IN);
     g_assert_cmpint(h2.last_event, ==, CHR_EVENT_MUX_OUT);
@@ -227,6 +231,10 @@ static void char_mux_test(void)
     g_assert_cmpstr(h1.read_buf, ==, "hello");
     h1.read_count = 0;
 
+    qemu_chr_be_write(base, (void *)"\1b", 2);
+    g_assert_cmpint(h1.last_event, ==, CHR_EVENT_BREAK);
+    g_assert_cmpint(h2.last_event, ==, CHR_EVENT_MUX_OUT);
+
     /* remove first handler */
     qemu_chr_fe_set_handlers(&chr_be1, NULL, NULL, NULL, NULL,
                              NULL, NULL, true);
-- 
2.17.0