qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] char-udp: Fix initial backend open status
@ 2025-12-08 22:58 Eric K
  2025-12-09  8:02 ` Marc-André Lureau
  0 siblings, 1 reply; 3+ messages in thread
From: Eric K @ 2025-12-08 22:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Marc-André Lureau, Eric K

This patch removes the `*be_opened = false` override for the UDP chardev
backend. Since UDP is connectionless it never sends a `CHR_EVENT_OPENED`
so it is never marked open. This causes some frontends (e.g. virtio-serial)
to never perform any operations on the socket.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2993
Signed-off-by: Eric K <erickra@cs.utexas.edu>
---
 chardev/char-udp.c     | 2 --
 tests/unit/test-char.c | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/chardev/char-udp.c b/chardev/char-udp.c
index 572fab0ad1..1025f577a0 100644
--- a/chardev/char-udp.c
+++ b/chardev/char-udp.c
@@ -215,8 +215,6 @@ static void qmp_chardev_open_udp(Chardev *chr,
     g_free(name);
 
     s->ioc = QIO_CHANNEL(sioc);
-    /* be isn't opened until we get a connection */
-    *be_opened = false;
 }
 
 static void char_udp_class_init(ObjectClass *oc, const void *data)
diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
index 8a98e42cad..2869c4e09d 100644
--- a/tests/unit/test-char.c
+++ b/tests/unit/test-char.c
@@ -1012,6 +1012,8 @@ static void char_udp_test_internal(Chardev *reuse_chr, int sock)
         qemu_chr_fe_init(fe, chr, &error_abort);
     }
 
+    g_assert(chr->be_open);
+
     d.chr = chr;
     qemu_chr_fe_set_handlers(fe, socket_can_read_hello, socket_read_hello,
                              NULL, NULL, &d, NULL, true);
-- 
2.52.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] char-udp: Fix initial backend open status
  2025-12-08 22:58 [PATCH] char-udp: Fix initial backend open status Eric K
@ 2025-12-09  8:02 ` Marc-André Lureau
  2025-12-09  8:33   ` Daniel P. Berrangé
  0 siblings, 1 reply; 3+ messages in thread
From: Marc-André Lureau @ 2025-12-09  8:02 UTC (permalink / raw)
  To: Eric K; +Cc: qemu-devel, Paolo Bonzini

Hi Eric

On Tue, Dec 9, 2025 at 5:18 AM Eric K <erickra@cs.utexas.edu> wrote:
>
> This patch removes the `*be_opened = false` override for the UDP chardev
> backend. Since UDP is connectionless it never sends a `CHR_EVENT_OPENED`
> so it is never marked open. This causes some frontends (e.g. virtio-serial)
> to never perform any operations on the socket.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2993
> Signed-off-by: Eric K <erickra@cs.utexas.edu>

UDP is connection-less, so it will not be notified when a client is
ready to receive. If we make the chardev always open, the device may
send data too early though.

At the same time, a chardev that only reads isn't very useful.

I don't think we need to introduce a property for the change of behaviour.

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

> ---
>  chardev/char-udp.c     | 2 --
>  tests/unit/test-char.c | 2 ++
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/chardev/char-udp.c b/chardev/char-udp.c
> index 572fab0ad1..1025f577a0 100644
> --- a/chardev/char-udp.c
> +++ b/chardev/char-udp.c
> @@ -215,8 +215,6 @@ static void qmp_chardev_open_udp(Chardev *chr,
>      g_free(name);
>
>      s->ioc = QIO_CHANNEL(sioc);
> -    /* be isn't opened until we get a connection */
> -    *be_opened = false;
>  }
>
>  static void char_udp_class_init(ObjectClass *oc, const void *data)
> diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
> index 8a98e42cad..2869c4e09d 100644
> --- a/tests/unit/test-char.c
> +++ b/tests/unit/test-char.c
> @@ -1012,6 +1012,8 @@ static void char_udp_test_internal(Chardev *reuse_chr, int sock)
>          qemu_chr_fe_init(fe, chr, &error_abort);
>      }
>
> +    g_assert(chr->be_open);
> +
>      d.chr = chr;
>      qemu_chr_fe_set_handlers(fe, socket_can_read_hello, socket_read_hello,
>                               NULL, NULL, &d, NULL, true);
> --
> 2.52.0
>
>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] char-udp: Fix initial backend open status
  2025-12-09  8:02 ` Marc-André Lureau
@ 2025-12-09  8:33   ` Daniel P. Berrangé
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel P. Berrangé @ 2025-12-09  8:33 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: Eric K, qemu-devel, Paolo Bonzini

On Tue, Dec 09, 2025 at 12:02:10PM +0400, Marc-André Lureau wrote:
> Hi Eric
> 
> On Tue, Dec 9, 2025 at 5:18 AM Eric K <erickra@cs.utexas.edu> wrote:
> >
> > This patch removes the `*be_opened = false` override for the UDP chardev
> > backend. Since UDP is connectionless it never sends a `CHR_EVENT_OPENED`
> > so it is never marked open. This causes some frontends (e.g. virtio-serial)
> > to never perform any operations on the socket.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2993
> > Signed-off-by: Eric K <erickra@cs.utexas.edu>
> 
> UDP is connection-less, so it will not be notified when a client is
> ready to receive. If we make the chardev always open, the device may
> send data too early though.
> 
> At the same time, a chardev that only reads isn't very useful.

Loosing data due to no client in the receive end is the price users
willing accept when they choose to use the UDP backend.

If they need reliable data transfer we have better chardev backends
than UDP available to use.

> I don't think we need to introduce a property for the change of behaviour.
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> > ---
> >  chardev/char-udp.c     | 2 --
> >  tests/unit/test-char.c | 2 ++
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/chardev/char-udp.c b/chardev/char-udp.c
> > index 572fab0ad1..1025f577a0 100644
> > --- a/chardev/char-udp.c
> > +++ b/chardev/char-udp.c
> > @@ -215,8 +215,6 @@ static void qmp_chardev_open_udp(Chardev *chr,
> >      g_free(name);
> >
> >      s->ioc = QIO_CHANNEL(sioc);
> > -    /* be isn't opened until we get a connection */
> > -    *be_opened = false;
> >  }
> >
> >  static void char_udp_class_init(ObjectClass *oc, const void *data)
> > diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
> > index 8a98e42cad..2869c4e09d 100644
> > --- a/tests/unit/test-char.c
> > +++ b/tests/unit/test-char.c
> > @@ -1012,6 +1012,8 @@ static void char_udp_test_internal(Chardev *reuse_chr, int sock)
> >          qemu_chr_fe_init(fe, chr, &error_abort);
> >      }
> >
> > +    g_assert(chr->be_open);
> > +
> >      d.chr = chr;
> >      qemu_chr_fe_set_handlers(fe, socket_can_read_hello, socket_read_hello,
> >                               NULL, NULL, &d, NULL, true);
> > --
> > 2.52.0
> >
> >
> 
> 
> -- 
> Marc-André Lureau
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-12-09  8:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-08 22:58 [PATCH] char-udp: Fix initial backend open status Eric K
2025-12-09  8:02 ` Marc-André Lureau
2025-12-09  8:33   ` Daniel P. Berrangé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).