Update the terminal size upon SIGWINCH delivery.

Signed-off-by: Maximilian Immanuel Brandtner <maxbr@linux.ibm.com>
---

To be committed with the patch-set: [PATCH v4 00/10] virtio-console: notify about the terminal size

v1 -> v2: Move comment regarding patch dependencies to note section
---
 chardev/char-pty.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index 674e9b3f14..802bae9037 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -28,6 +28,7 @@
 #include "io/channel-file.h"
 #include "qemu/sockets.h"
 #include "qemu/error-report.h"
+#include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "qemu/option.h"
 #include "qemu/qemu-print.h"
@@ -35,6 +36,8 @@
 #include "chardev/char-io.h"
 #include "qom/object.h"
 
+#include <sys/ioctl.h>
+
 struct PtyChardev {
     Chardev parent;
     QIOChannel *ioc;
@@ -43,6 +46,8 @@ struct PtyChardev {
     int connected;
     GSource *timer_src;
     char *path;
+
+    Notifier resize_notifier;
 };
 typedef struct PtyChardev PtyChardev;
 
@@ -85,6 +90,15 @@ static void pty_chr_rearm_timer(Chardev *chr, int ms)
     g_free(name);
 }
 
+static void pty_chr_resize(PtyChardev *s)
+{
+    struct winsize ws;
+
+    if (ioctl(QIO_CHANNEL_FILE(s->ioc)->fd, TIOCGWINSZ, &ws) != -1) {
+        qemu_chr_resize(CHARDEV(s), ws.ws_col, ws.ws_row);
+    }
+}
+
 static void pty_chr_update_read_handler(Chardev *chr)
 {
     PtyChardev *s = PTY_CHARDEV(chr);
@@ -331,6 +345,12 @@ static int qemu_openpty_raw(int *aslave, char *pty_name)
     return amaster;
 }
 
+static void term_resize_notify(Notifier *n, void *data)
+{
+    PtyChardev *s = container_of(n, PtyChardev, resize_notifier);
+    pty_chr_resize(s);
+}
+
 static void char_pty_open(Chardev *chr,
                           ChardevBackend *backend,
                           bool *be_opened,
@@ -376,6 +396,10 @@ static void char_pty_open(Chardev *chr,
             s->path = g_strdup(path);
         }
     }
+
+    pty_chr_resize(s);
+    s->resize_notifier.notify = term_resize_notify;
+    sigwinch_add_notifier(&s->resize_notifier);
 }
 
 static void char_pty_parse(QemuOpts *opts, ChardevBackend *backend,
-- 
2.50.1

On Mon, 2025-09-15 at 18:34 +0200, Maximilian Immanuel Brandtner wrote:
> Update the terminal size upon SIGWINCH delivery.
> 
> Signed-off-by: Maximilian Immanuel Brandtner <maxbr@linux.ibm.com>

I don't think this will work, because SIGWINCH is only delivered for
the process' controling terminal. Unfortunately I don't think there is
any way to get size notifications for arbitrary terminal.

On Tue, 2025-09-16 at 00:02 +0200, Filip Hejsek wrote:
> On Mon, 2025-09-15 at 18:34 +0200, Maximilian Immanuel Brandtner
> wrote:
> > Update the terminal size upon SIGWINCH delivery.
> > 
> > Signed-off-by: Maximilian Immanuel Brandtner <maxbr@linux.ibm.com>
> 
> I don't think this will work, because SIGWINCH is only delivered for
> the process' controling terminal. Unfortunately I don't think there
> is
> any way to get size notifications for arbitrary terminal.

In that case there are two solutions:
1. make qemu the controlling process of the pty (this has the
disadvantage of QEMU being quit when the pty is closed)
2. create a timer polling every eg 100ms to check if the winsize has
changed

I would go with the second approach then and implement the timer as a
g_source. Or are there other timer mechanisms I should use instead?

On September 17, 2025 11:39:55 AM GMT+02:00, Maximilian Immanuel Brandtner <maxbr@linux.ibm.com> wrote:
> On Tue, 2025-09-16 at 00:02 +0200, Filip Hejsek wrote:
> > On Mon, 2025-09-15 at 18:34 +0200, Maximilian Immanuel Brandtner
> > wrote:
> > > Update the terminal size upon SIGWINCH delivery.
> > > 
> > > Signed-off-by: Maximilian Immanuel Brandtner <maxbr@linux.ibm.com>
> > 
> > I don't think this will work, because SIGWINCH is only delivered for
> > the process' controling terminal. Unfortunately I don't think there
> > is
> > any way to get size notifications for arbitrary terminal.
> 
> In that case there are two solutions:
> 1. make qemu the controlling process of the pty (this has the
> disadvantage of QEMU being quit when the pty is closed)

A bigger disadvantage is that a process can only have one controlling terminal, and a terminal can only be the controlling terminal for a single session (and only sends signals to the foreground process group of that session). It would require forking a process for each pty, and I don't even know if the master end can have its own session.

> 2. create a timer polling every eg 100ms to check if the winsize has
> changed
> 
> I would go with the second approach then

Me too, the timer is a bit unfortunate, but it's probably the less bad option.

On Wed, Sep 17, 2025 at 03:09:50PM +0200, Filip Hejsek wrote:
> 
> 
> On September 17, 2025 11:39:55 AM GMT+02:00, Maximilian Immanuel Brandtner <maxbr@linux.ibm.com> wrote:
> > On Tue, 2025-09-16 at 00:02 +0200, Filip Hejsek wrote:
> > > On Mon, 2025-09-15 at 18:34 +0200, Maximilian Immanuel Brandtner
> > > wrote:
> > > > Update the terminal size upon SIGWINCH delivery.
> > > > 
> > > > Signed-off-by: Maximilian Immanuel Brandtner <maxbr@linux.ibm.com>
> > > 
> > > I don't think this will work, because SIGWINCH is only delivered for
> > > the process' controling terminal. Unfortunately I don't think there
> > > is
> > > any way to get size notifications for arbitrary terminal.
> > 
> > In that case there are two solutions:
> > 1. make qemu the controlling process of the pty (this has the
> > disadvantage of QEMU being quit when the pty is closed)
> 
> A bigger disadvantage is that a process can only have one controlling terminal, and a terminal can only be the controlling terminal for a single session (and only sends signals to the foreground process group of that session). It would require forking a process for each pty, and I don't even know if the master end can have its own session.
> 
> > 2. create a timer polling every eg 100ms to check if the winsize has
> > changed
> > 
> > I would go with the second approach then
> 
> Me too, the timer is a bit unfortunate, but it's probably the less bad option.

I don't think we want a timer polling for an situation that will very
rarely arise.  We already add the 'chardev_resize' QMP command, which is
a good enough way to kick QEMU to re-read the size.

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

On Wed, 2025-09-17 at 14:31 +0100, Daniel P. Berrangé wrote:
> On Wed, Sep 17, 2025 at 03:09:50PM +0200, Filip Hejsek wrote:
> > 
> > 
> > On September 17, 2025 11:39:55 AM GMT+02:00, Maximilian Immanuel
> > Brandtner <maxbr@linux.ibm.com> wrote:
> > > On Tue, 2025-09-16 at 00:02 +0200, Filip Hejsek wrote:
> > > > On Mon, 2025-09-15 at 18:34 +0200, Maximilian Immanuel
> > > > Brandtner
> > > > wrote:
> > > > > Update the terminal size upon SIGWINCH delivery.
> > > > > 
> > > > > Signed-off-by: Maximilian Immanuel Brandtner
> > > > > <maxbr@linux.ibm.com>
> > > > 
> > > > I don't think this will work, because SIGWINCH is only
> > > > delivered for
> > > > the process' controling terminal. Unfortunately I don't think
> > > > there
> > > > is
> > > > any way to get size notifications for arbitrary terminal.
> > > 
> > > In that case there are two solutions:
> > > 1. make qemu the controlling process of the pty (this has the
> > > disadvantage of QEMU being quit when the pty is closed)
> > 
> > A bigger disadvantage is that a process can only have one
> > controlling terminal, and a terminal can only be the controlling
> > terminal for a single session (and only sends signals to the
> > foreground process group of that session). It would require forking
> > a process for each pty, and I don't even know if the master end can
> > have its own session.
> > 
> > > 2. create a timer polling every eg 100ms to check if the winsize
> > > has
> > > changed
> > > 
> > > I would go with the second approach then
> > 
> > Me too, the timer is a bit unfortunate, but it's probably the less
> > bad option.
> 
> I don't think we want a timer polling for an situation that will very
> rarely arise.  We already add the 'chardev_resize' QMP command, which
> is
> a good enough way to kick QEMU to re-read the size.
> 
> With regards,
> Daniel

This approach would only work with libvirt and not generic pty
applications though. Perhaps a bool poll_resize could be added to the
struct Chardev which is disabled, as soon as the chardev_resize QMP
command is used to avoid race conditions.

On September 18, 2025 9:53:27 AM GMT+02:00, Maximilian Immanuel Brandtner <maxbr@linux.ibm.com> wrote:
> On Wed, 2025-09-17 at 14:31 +0100, Daniel P. Berrangé wrote:
> > On Wed, Sep 17, 2025 at 03:09:50PM +0200, Filip Hejsek wrote:
> > > 
> > > 
> > > On September 17, 2025 11:39:55 AM GMT+02:00, Maximilian Immanuel
> > > Brandtner <maxbr@linux.ibm.com> wrote:
> > > > On Tue, 2025-09-16 at 00:02 +0200, Filip Hejsek wrote:
> > > > > On Mon, 2025-09-15 at 18:34 +0200, Maximilian Immanuel
> > > > > Brandtner
> > > > > wrote:
> > > > > > Update the terminal size upon SIGWINCH delivery.
> > > > > > 
> > > > > > Signed-off-by: Maximilian Immanuel Brandtner
> > > > > > <maxbr@linux.ibm.com>
> > > > > 
> > > > > I don't think this will work, because SIGWINCH is only
> > > > > delivered for
> > > > > the process' controling terminal. Unfortunately I don't think
> > > > > there
> > > > > is
> > > > > any way to get size notifications for arbitrary terminal.
> > > > 
> > > > In that case there are two solutions:
> > > > 1. make qemu the controlling process of the pty (this has the
> > > > disadvantage of QEMU being quit when the pty is closed)
> > > 
> > > A bigger disadvantage is that a process can only have one
> > > controlling terminal, and a terminal can only be the controlling
> > > terminal for a single session (and only sends signals to the
> > > foreground process group of that session). It would require forking
> > > a process for each pty, and I don't even know if the master end can
> > > have its own session.
> > > 
> > > > 2. create a timer polling every eg 100ms to check if the winsize
> > > > has
> > > > changed
> > > > 
> > > > I would go with the second approach then
> > > 
> > > Me too, the timer is a bit unfortunate, but it's probably the less
> > > bad option.
> > 
> > I don't think we want a timer polling for an situation that will very
> > rarely arise.  We already add the 'chardev_resize' QMP command, which
> > is
> > a good enough way to kick QEMU to re-read the size.
> > 
> > With regards,
> > Daniel
> 
> This approach would only work with libvirt and not generic pty
> applications though. Perhaps a bool poll_resize could be added to the
> struct Chardev which is disabled, as soon as the chardev_resize QMP
> command is used to avoid race conditions.

Well, terminal emulators that set the pty size typically create their own pty
and control it from the master side. I don't know what pty applications you
have in mind—changing the window size from the slave side is somewhat
atypical. Applications designed for a real serial port probably don't set the size at all.

Regards,
Filip

On September 17, 2025 3:31:57 PM GMT+02:00, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
> [...]
> > > 2. create a timer polling every eg 100ms to check if the winsize has
> > > changed
> [...]
> 
> I don't think we want a timer polling for an situation that will very
> rarely arise.  We already add the 'chardev_resize' QMP command, which is
> a good enough way to kick QEMU to re-read the size.

So the size provided in the command would be ignored, and QEMU would instead query the pty fd?

Note that this would mean there is no size info if the command is not used, because the size will be 0x0 when the pty is created by QEMU (though we could add device parameters for the initial size).

Best regards,
Filip

On Wed, Sep 17, 2025 at 04:08:01PM +0200, Filip Hejsek wrote:
> 
> 
> On September 17, 2025 3:31:57 PM GMT+02:00, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
> > [...]
> > > > 2. create a timer polling every eg 100ms to check if the winsize has
> > > > changed
> > [...]
> > 
> > I don't think we want a timer polling for an situation that will very
> > rarely arise.  We already add the 'chardev_resize' QMP command, which is
> > a good enough way to kick QEMU to re-read the size.
> 
> So the size provided in the command would be ignored, and QEMU would
> instead query the pty fd?

Actually I was just thinking we ignore the pty and only support the QMP
command. There is little point in trying for access size via the pty
if clients need the QMP command regardless to notify QEMU.

> Note that this would mean there is no size info if the command is
> not used, because the size will be 0x0 when the pty is created by
> QEMU (though we could add device parameters for the initial size).

We shouldn't send any size info to the guest if the hsot backend
does not have it available.

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

On Wed, 2025-09-17 at 17:17 +0100, Daniel P. Berrangé wrote:

> We shouldn't send any size info to the guest if the hsot backend
> does not have it available.

Does that mean sending 0x0, or not sending anything at all? The later
is tricky, because for non-multiport devices it's only really possible
by not offering the feature bit, but we don't know upfront whether the
size command will be used.

On Wed, Sep 17, 2025 at 07:11:03PM +0200, Filip Hejsek wrote:
> On Wed, 2025-09-17 at 17:17 +0100, Daniel P. Berrangé wrote:
> 
> > We shouldn't send any size info to the guest if the hsot backend
> > does not have it available.
> 
> Does that mean sending 0x0, or not sending anything at all? The later
> is tricky, because for non-multiport devices it's only really possible
> by not offering the feature bit, but we don't know upfront whether the
> size command will be used.

Nothing at all - is in no difference from current QEMU behaviour.

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

On Wed, 2025-09-17 at 18:53 +0100, Daniel P. Berrangé wrote:
> On Wed, Sep 17, 2025 at 07:11:03PM +0200, Filip Hejsek wrote:
> > On Wed, 2025-09-17 at 17:17 +0100, Daniel P. Berrangé wrote:
> > 
> > > We shouldn't send any size info to the guest if the hsot backend
> > > does not have it available.
> > 
> > Does that mean sending 0x0, or not sending anything at all? The later
> > is tricky, because for non-multiport devices it's only really possible
> > by not offering the feature bit, but we don't know upfront whether the
> > size command will be used.
> 
> Nothing at all - is in no difference from current QEMU behaviour.

As I said, that's not possible with the current semantics of the resize
command, as we would need to know upfront whether it is going to be
used.

To get the exact same behavior as current QEMU, we would need to add
some way to inform QEMU whether we want to use the resize command (e.g.
device property).

Even then, depending on how you interpret the virtio spec, there would
be a problem with multiport devices if port 0 didn't support size, but
another port did. Not providing port 0 size can only be done by not
offering the size feature bit, and then the question is, can you still
send resize events for the other ports? The spec does not say either
way.

Note that getting the exact same behavior as current QEMU is still
possible by disabling the console-size property on the virtio-serial
device (but it applies to all ports).

With regards,
Filip

On Wed, Sep 17, 2025 at 08:29:39PM +0200, Filip Hejsek wrote:
> On Wed, 2025-09-17 at 18:53 +0100, Daniel P. Berrangé wrote:
> > On Wed, Sep 17, 2025 at 07:11:03PM +0200, Filip Hejsek wrote:
> > > On Wed, 2025-09-17 at 17:17 +0100, Daniel P. Berrangé wrote:
> > > 
> > > > We shouldn't send any size info to the guest if the hsot backend
> > > > does not have it available.
> > > 
> > > Does that mean sending 0x0, or not sending anything at all? The later
> > > is tricky, because for non-multiport devices it's only really possible
> > > by not offering the feature bit, but we don't know upfront whether the
> > > size command will be used.

What are the semantics in the guest if we sent 0x0 as the size ?
AFAICT the virtio spec is silent on what '0x0' means.

It seems like it could conceivably have any behaviour, whether
a zero-size console, or a console clamped to 1x1 as a min size,
or a console reset to an arbitrary guest default like 80x24.


> > Nothing at all - is in no difference from current QEMU behaviour.
> 
> As I said, that's not possible with the current semantics of the resize
> command, as we would need to know upfront whether it is going to be
> used.
> 
> To get the exact same behavior as current QEMU, we would need to add
> some way to inform QEMU whether we want to use the resize command (e.g.
> device property).

That is usually unknowable at the time we spawn QEMU.

I'd say that the common case is for guests to get given a console
connected to a UNIX socket. Most of the time the console will not
be used. If it is, then we've no idea whether the client will be
something virtualization-unaware like a plain 'socat', or something
virtualization-aware like libvirt's  'virsh console'.

> Even then, depending on how you interpret the virtio spec, there would
> be a problem with multiport devices if port 0 didn't support size, but
> another port did. Not providing port 0 size can only be done by not
> offering the size feature bit, and then the question is, can you still
> send resize events for the other ports? The spec does not say either
> way.
>
> Note that getting the exact same behavior as current QEMU is still
> possible by disabling the console-size property on the virtio-serial
> device (but it applies to all ports).

Yes, it seems like the spec ties our hands here wrt multiple ports.

I didn't apprepreciate in my previous review that integrating this support
into QEMU was going to imply us /always/ informing the guest about the
requested console size for all ports, regardless of the backend.

I had been under the belief that we were only going to pass size info to
the guest, if the chardev was 'stdio', and for all other chardev backends
no size info would be passed unless we had issued the QMP resize command.

That we will always pass size info the guest regardless of the backend,
across all ports, changes my view about whether it is  reasonable to
enable resize by default given the known Linux guest bug.

The impact of the guest bug is just about tolerable if we were only going
to enable passing size information when the user had chosen 'stdio' backend
as that is relatively rarely used and mostly by ad-hoc dev usage where it
is perhaps easier for users to get a fixed guest kernel.

If we enable this for all ports though, regardless of backend, then I think
we're going to cause too much pain for users with the inverted rows/cols,
as its going to apply in every single deployment of QEMU using virtioconsole.

So IMHO we cannot enable this without explict user opt-in.

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

On Thu, 2025-09-18 at 09:35 +0100, Daniel P. Berrangé wrote:
> On Wed, Sep 17, 2025 at 08:29:39PM +0200, Filip Hejsek wrote:
> > On Wed, 2025-09-17 at 18:53 +0100, Daniel P. Berrangé wrote:
> > > On Wed, Sep 17, 2025 at 07:11:03PM +0200, Filip Hejsek wrote:
> > > > On Wed, 2025-09-17 at 17:17 +0100, Daniel P. Berrangé wrote:
> > > > 
> > > > > We shouldn't send any size info to the guest if the hsot
> > > > > backend
> > > > > does not have it available.
> > > > 
> > > > Does that mean sending 0x0, or not sending anything at all? The
> > > > later
> > > > is tricky, because for non-multiport devices it's only really
> > > > possible
> > > > by not offering the feature bit, but we don't know upfront
> > > > whether the
> > > > size command will be used.
> 
> What are the semantics in the guest if we sent 0x0 as the size ?
> AFAICT the virtio spec is silent on what '0x0' means.
> 
> It seems like it could conceivably have any behaviour, whether
> a zero-size console, or a console clamped to 1x1 as a min size,
> or a console reset to an arbitrary guest default like 80x24.

During testing the kernel resized the tty to 0x0 if VirtIO instructed
the kernel to resize the tty to 0x0.
> 
> 
> > > Nothing at all - is in no difference from current QEMU behaviour.
> > 
> > As I said, that's not possible with the current semantics of the
> > resize
> > command, as we would need to know upfront whether it is going to be
> > used.
> > 
> > To get the exact same behavior as current QEMU, we would need to
> > add
> > some way to inform QEMU whether we want to use the resize command
> > (e.g.
> > device property).
> 
> That is usually unknowable at the time we spawn QEMU.
> 
> I'd say that the common case is for guests to get given a console
> connected to a UNIX socket. Most of the time the console will not
> be used. If it is, then we've no idea whether the client will be
> something virtualization-unaware like a plain 'socat', or something
> virtualization-aware like libvirt's  'virsh console'.
> 
> > Even then, depending on how you interpret the virtio spec, there
> > would
> > be a problem with multiport devices if port 0 didn't support size,
> > but
> > another port did. Not providing port 0 size can only be done by not
> > offering the size feature bit, and then the question is, can you
> > still
> > send resize events for the other ports? The spec does not say
> > either
> > way.
> > 
> > Note that getting the exact same behavior as current QEMU is still
> > possible by disabling the console-size property on the virtio-
> > serial
> > device (but it applies to all ports).
> 
> Yes, it seems like the spec ties our hands here wrt multiple ports.
> 
> I didn't apprepreciate in my previous review that integrating this
> support
> into QEMU was going to imply us /always/ informing the guest about
> the
> requested console size for all ports, regardless of the backend.
> 
> I had been under the belief that we were only going to pass size info
> to
> the guest, if the chardev was 'stdio', and for all other chardev
> backends
> no size info would be passed unless we had issued the QMP resize
> command.
> 
> That we will always pass size info the guest regardless of the
> backend,
> across all ports, changes my view about whether it is  reasonable to
> enable resize by default given the known Linux guest bug.
> 
> The impact of the guest bug is just about tolerable if we were only
> going
> to enable passing size information when the user had chosen 'stdio'
> backend
> as that is relatively rarely used and mostly by ad-hoc dev usage
> where it
> is perhaps easier for users to get a fixed guest kernel.
> 
> If we enable this for all ports though, regardless of backend, then I
> think
> we're going to cause too much pain for users with the inverted
> rows/cols,
> as its going to apply in every single deployment of QEMU using
> virtioconsole.
> 
> So IMHO we cannot enable this without explict user opt-in.
> 
> With regards,
> Daniel

On Thu, Sep 18, 2025 at 10:39:28AM +0200, Maximilian Immanuel Brandtner wrote:
> On Thu, 2025-09-18 at 09:35 +0100, Daniel P. Berrangé wrote:
> > On Wed, Sep 17, 2025 at 08:29:39PM +0200, Filip Hejsek wrote:
> > > On Wed, 2025-09-17 at 18:53 +0100, Daniel P. Berrangé wrote:
> > > > On Wed, Sep 17, 2025 at 07:11:03PM +0200, Filip Hejsek wrote:
> > > > > On Wed, 2025-09-17 at 17:17 +0100, Daniel P. Berrangé wrote:
> > > > > 
> > > > > > We shouldn't send any size info to the guest if the hsot
> > > > > > backend
> > > > > > does not have it available.
> > > > > 
> > > > > Does that mean sending 0x0, or not sending anything at all? The
> > > > > later
> > > > > is tricky, because for non-multiport devices it's only really
> > > > > possible
> > > > > by not offering the feature bit, but we don't know upfront
> > > > > whether the
> > > > > size command will be used.
> > 
> > What are the semantics in the guest if we sent 0x0 as the size ?
> > AFAICT the virtio spec is silent on what '0x0' means.
> > 
> > It seems like it could conceivably have any behaviour, whether
> > a zero-size console, or a console clamped to 1x1 as a min size,
> > or a console reset to an arbitrary guest default like 80x24.
> 
> During testing the kernel resized the tty to 0x0 if VirtIO instructed
> the kernel to resize the tty to 0x0.

If the chardev backends are defaulting to 0x0 for everything except
the 'stdio' backend, then this series is surely going to break all
existing usage of virtio-console for non-stdio backends ?

What am I missing here ?

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

On Thu, 2025-09-18 at 09:48 +0100, Daniel P. Berrangé wrote:
> On Thu, Sep 18, 2025 at 10:39:28AM +0200, Maximilian Immanuel
> Brandtner wrote:
> > On Thu, 2025-09-18 at 09:35 +0100, Daniel P. Berrangé wrote:
> > > On Wed, Sep 17, 2025 at 08:29:39PM +0200, Filip Hejsek wrote:
> > > > On Wed, 2025-09-17 at 18:53 +0100, Daniel P. Berrangé wrote:
> > > > > On Wed, Sep 17, 2025 at 07:11:03PM +0200, Filip Hejsek wrote:
> > > > > > On Wed, 2025-09-17 at 17:17 +0100, Daniel P. Berrangé
> > > > > > wrote:
> > > > > > 
> > > > > > > We shouldn't send any size info to the guest if the hsot
> > > > > > > backend
> > > > > > > does not have it available.
> > > > > > 
> > > > > > Does that mean sending 0x0, or not sending anything at all?
> > > > > > The
> > > > > > later
> > > > > > is tricky, because for non-multiport devices it's only
> > > > > > really
> > > > > > possible
> > > > > > by not offering the feature bit, but we don't know upfront
> > > > > > whether the
> > > > > > size command will be used.
> > > 
> > > What are the semantics in the guest if we sent 0x0 as the size ?
> > > AFAICT the virtio spec is silent on what '0x0' means.
> > > 
> > > It seems like it could conceivably have any behaviour, whether
> > > a zero-size console, or a console clamped to 1x1 as a min size,
> > > or a console reset to an arbitrary guest default like 80x24.
> > 
> > During testing the kernel resized the tty to 0x0 if VirtIO
> > instructed
> > the kernel to resize the tty to 0x0.
> 
> If the chardev backends are defaulting to 0x0 for everything except
> the 'stdio' backend, then this series is surely going to break all
> existing usage of virtio-console for non-stdio backends ?
> 
> What am I missing here ?
> 
> With regards,
> Daniel

Most applications fall back to 80x24 if the terminal size is 0x0 so
it's not as big of a dealbreaker as you might think.

However, I think it would be even better if the patch-set could be
changed to account for that. After initializing the VirtIO console a
resize event could be sent to set the initial size (80x24), which might
later be changed or be left as is.
If the VIRTIO_CONSOLE_F_SIZE is negotiated(and this feature flag is
necessary for multiport resize messages not to be ignored) QEMU is
responsible for setting the initial terminal size.

Kind regards,
Max Brandtner

On Thu, Sep 18, 2025 at 10:54:45AM +0200, Maximilian Immanuel Brandtner wrote:
> On Thu, 2025-09-18 at 09:48 +0100, Daniel P. Berrangé wrote:
> > On Thu, Sep 18, 2025 at 10:39:28AM +0200, Maximilian Immanuel
> > Brandtner wrote:
> > > On Thu, 2025-09-18 at 09:35 +0100, Daniel P. Berrangé wrote:
> > > > On Wed, Sep 17, 2025 at 08:29:39PM +0200, Filip Hejsek wrote:
> > > > > On Wed, 2025-09-17 at 18:53 +0100, Daniel P. Berrangé wrote:
> > > > > > On Wed, Sep 17, 2025 at 07:11:03PM +0200, Filip Hejsek wrote:
> > > > > > > On Wed, 2025-09-17 at 17:17 +0100, Daniel P. Berrangé
> > > > > > > wrote:
> > > > > > > 
> > > > > > > > We shouldn't send any size info to the guest if the hsot
> > > > > > > > backend
> > > > > > > > does not have it available.
> > > > > > > 
> > > > > > > Does that mean sending 0x0, or not sending anything at all?
> > > > > > > The
> > > > > > > later
> > > > > > > is tricky, because for non-multiport devices it's only
> > > > > > > really
> > > > > > > possible
> > > > > > > by not offering the feature bit, but we don't know upfront
> > > > > > > whether the
> > > > > > > size command will be used.
> > > > 
> > > > What are the semantics in the guest if we sent 0x0 as the size ?
> > > > AFAICT the virtio spec is silent on what '0x0' means.
> > > > 
> > > > It seems like it could conceivably have any behaviour, whether
> > > > a zero-size console, or a console clamped to 1x1 as a min size,
> > > > or a console reset to an arbitrary guest default like 80x24.
> > > 
> > > During testing the kernel resized the tty to 0x0 if VirtIO
> > > instructed
> > > the kernel to resize the tty to 0x0.
> > 
> > If the chardev backends are defaulting to 0x0 for everything except
> > the 'stdio' backend, then this series is surely going to break all
> > existing usage of virtio-console for non-stdio backends ?
> > 
> > What am I missing here ?
> 
> Most applications fall back to 80x24 if the terminal size is 0x0 so
> it's not as big of a dealbreaker as you might think.

I'm not convinced that its a good idea for QEMU to be relying on every
application to be doing that. I can forsee the bug reports from situations
where this doesn't happen and something ends up dividing by zero when
doing an aspect ratio calculation.  Yes, we could point to the app code
and call it buggy, but I think there's a strong case to be made that we
shouldn't have been sending 0x0 to begin with.

> However, I think it would be even better if the patch-set could be
> changed to account for that. After initializing the VirtIO console a
> resize event could be sent to set the initial size (80x24), which might
> later be changed or be left as is.

The problem with QEMU sending 80x24 instead of 0x0 is that the majority
of Linux guests will then treat that as 24x80 due to the historical bug
in Linux drivers. This will probably be even worse than the bugs we get
from sending 0x0.

> If the VIRTIO_CONSOLE_F_SIZE is negotiated(and this feature flag is
> necessary for multiport resize messages not to be ignored) QEMU is
> responsible for setting the initial terminal size.

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

On Thu, 2025-09-18 at 09:59 +0100, Daniel P. Berrangé wrote:
> On Thu, Sep 18, 2025 at 10:54:45AM +0200, Maximilian Immanuel
> Brandtner wrote:
> > On Thu, 2025-09-18 at 09:48 +0100, Daniel P. Berrangé wrote:
> > > On Thu, Sep 18, 2025 at 10:39:28AM +0200, Maximilian Immanuel
> > > Brandtner wrote:
> > > > On Thu, 2025-09-18 at 09:35 +0100, Daniel P. Berrangé wrote:
> > > > > On Wed, Sep 17, 2025 at 08:29:39PM +0200, Filip Hejsek wrote:
> > > > > > On Wed, 2025-09-17 at 18:53 +0100, Daniel P. Berrangé
> > > > > > wrote:
> > > > > > > On Wed, Sep 17, 2025 at 07:11:03PM +0200, Filip Hejsek
> > > > > > > wrote:
> > > > > > > > On Wed, 2025-09-17 at 17:17 +0100, Daniel P. Berrangé
> > > > > > > > wrote:
> > > > > > > > 
> > > > > > > > > We shouldn't send any size info to the guest if the
> > > > > > > > > hsot
> > > > > > > > > backend
> > > > > > > > > does not have it available.
> > > > > > > > 
> > > > > > > > Does that mean sending 0x0, or not sending anything at
> > > > > > > > all?
> > > > > > > > The
> > > > > > > > later
> > > > > > > > is tricky, because for non-multiport devices it's only
> > > > > > > > really
> > > > > > > > possible
> > > > > > > > by not offering the feature bit, but we don't know
> > > > > > > > upfront
> > > > > > > > whether the
> > > > > > > > size command will be used.
> > > > > 
> > > > > What are the semantics in the guest if we sent 0x0 as the
> > > > > size ?
> > > > > AFAICT the virtio spec is silent on what '0x0' means.
> > > > > 
> > > > > It seems like it could conceivably have any behaviour,
> > > > > whether
> > > > > a zero-size console, or a console clamped to 1x1 as a min
> > > > > size,
> > > > > or a console reset to an arbitrary guest default like 80x24.
> > > > 
> > > > During testing the kernel resized the tty to 0x0 if VirtIO
> > > > instructed
> > > > the kernel to resize the tty to 0x0.
> > > 
> > > If the chardev backends are defaulting to 0x0 for everything
> > > except
> > > the 'stdio' backend, then this series is surely going to break
> > > all
> > > existing usage of virtio-console for non-stdio backends ?
> > > 
> > > What am I missing here ?
> > 
> > Most applications fall back to 80x24 if the terminal size is 0x0 so
> > it's not as big of a dealbreaker as you might think.
> 
> I'm not convinced that its a good idea for QEMU to be relying on
> every
> application to be doing that. I can forsee the bug reports from
> situations
> where this doesn't happen and something ends up dividing by zero when
> doing an aspect ratio calculation.  Yes, we could point to the app
> code
> and call it buggy, but I think there's a strong case to be made that
> we
> shouldn't have been sending 0x0 to begin with.
> 
> > However, I think it would be even better if the patch-set could be
> > changed to account for that. After initializing the VirtIO console
> > a
> > resize event could be sent to set the initial size (80x24), which
> > might
> > later be changed or be left as is.
> 
> The problem with QEMU sending 80x24 instead of 0x0 is that the
> majority
> of Linux guests will then treat that as 24x80 due to the historical
> bug
> in Linux drivers. This will probably be even worse than the bugs we
> get
> from sending 0x0.

I agree that this is a much more significant issue and I like your idea
of adding an opt-in parameter to support resizing for the virtio-
console chardev. The smoothest solution would have been a spec-change.

> 
> > If the VIRTIO_CONSOLE_F_SIZE is negotiated(and this feature flag is
> > necessary for multiport resize messages not to be ignored) QEMU is
> > responsible for setting the initial terminal size.
> 
> With regards,
> Daniel

I will respond to all messages from this thread at once here.

Daniel P. Berrangé wrote:
> That we will always pass size info the guest regardless of the backend,
> across all ports, changes my view about whether it is  reasonable to
> enable resize by default given the known Linux guest bug.
>
> The impact of the guest bug is just about tolerable if we were only going
> to enable passing size information when the user had chosen 'stdio' backend
> as that is relatively rarely used and mostly by ad-hoc dev usage where it
> is perhaps easier for users to get a fixed guest kernel.
>
> If we enable this for all ports though, regardless of backend, then I think
> we're going to cause too much pain for users with the inverted rows/cols,
> as its going to apply in every single deployment of QEMU using virtioconsole.

Inverted rows/cols don't matter for 0x0 size, 0x0 is still 0x0 when
swapped. Sizes other than 0x0 will only be sent from supported backends
(but it will be supported by more than just stdio).

> If the chardev backends are defaulting to 0x0 for everything except
> the 'stdio' backend, then this series is surely going to break all
> existing usage of virtio-console for non-stdio backends ?

At least for Linux guests, if no size is sent, the kernel defaults to
0x0, so sending 0x0 is equivalent to not sending anything. Nothing new
will break by sending 0x0.

We still need to be careful not to reset the size to 0x0 after
userspace has changed it though. With the non-multiport interface, the
kernel will reapply the size every time it gets the config interrupt.
If some other use for the config interrupt is added in the future
besides resizing, it could become a problem.

Actually, from looking at Linux source, it appears that linux only
reads the size after getting the config interrupt. This seems to imply
that linux will ignore the initial value if no interrupt is sent. That
might be a bug.

Maximilian Immanuel Brandtner wrote:
> I agree that this is a much more significant issue and I like your idea
> of adding an opt-in parameter to support resizing for the virtio-
> console chardev.

The console-size property can already disable size support, so making
it an opt-in is only a matter of changing the default

> The smoothest solution would have been a spec-change.

Here is the spec change proposal (thanks to MST for submitting it!):
https://lore.kernel.org/virtio-comment/7b939d85ec0b532bae4c16bb927edddcf663bb48.1758212319.git.mst@redhat.com/

Best regards,
Filip Hejsek

On Mon, Sep 15, 2025 at 06:34:15PM +0200, Maximilian Immanuel Brandtner wrote:
> Update the terminal size upon SIGWINCH delivery.
> 
> Signed-off-by: Maximilian Immanuel Brandtner <maxbr@linux.ibm.com>


Maximilian, on  a related note, could you comment on the revert
of your patch that I posted to lkml? Thanks!

> ---
> 
> To be committed with the patch-set: [PATCH v4 00/10] virtio-console: notify about the terminal size
> 
> v1 -> v2: Move comment regarding patch dependencies to note section
> ---
>  chardev/char-pty.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index 674e9b3f14..802bae9037 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -28,6 +28,7 @@
>  #include "io/channel-file.h"
>  #include "qemu/sockets.h"
>  #include "qemu/error-report.h"
> +#include "qemu/main-loop.h"
>  #include "qemu/module.h"
>  #include "qemu/option.h"
>  #include "qemu/qemu-print.h"
> @@ -35,6 +36,8 @@
>  #include "chardev/char-io.h"
>  #include "qom/object.h"
>  
> +#include <sys/ioctl.h>
> +
>  struct PtyChardev {
>      Chardev parent;
>      QIOChannel *ioc;
> @@ -43,6 +46,8 @@ struct PtyChardev {
>      int connected;
>      GSource *timer_src;
>      char *path;
> +
> +    Notifier resize_notifier;
>  };
>  typedef struct PtyChardev PtyChardev;
>  
> @@ -85,6 +90,15 @@ static void pty_chr_rearm_timer(Chardev *chr, int ms)
>      g_free(name);
>  }
>  
> +static void pty_chr_resize(PtyChardev *s)
> +{
> +    struct winsize ws;
> +
> +    if (ioctl(QIO_CHANNEL_FILE(s->ioc)->fd, TIOCGWINSZ, &ws) != -1) {
> +        qemu_chr_resize(CHARDEV(s), ws.ws_col, ws.ws_row);
> +    }
> +}
> +
>  static void pty_chr_update_read_handler(Chardev *chr)
>  {
>      PtyChardev *s = PTY_CHARDEV(chr);
> @@ -331,6 +345,12 @@ static int qemu_openpty_raw(int *aslave, char *pty_name)
>      return amaster;
>  }
>  
> +static void term_resize_notify(Notifier *n, void *data)
> +{
> +    PtyChardev *s = container_of(n, PtyChardev, resize_notifier);
> +    pty_chr_resize(s);
> +}
> +
>  static void char_pty_open(Chardev *chr,
>                            ChardevBackend *backend,
>                            bool *be_opened,
> @@ -376,6 +396,10 @@ static void char_pty_open(Chardev *chr,
>              s->path = g_strdup(path);
>          }
>      }
> +
> +    pty_chr_resize(s);
> +    s->resize_notifier.notify = term_resize_notify;
> +    sigwinch_add_notifier(&s->resize_notifier);
>  }
>  
>  static void char_pty_parse(QemuOpts *opts, ChardevBackend *backend,
> -- 
> 2.50.1