[PATCH v4 00/10] virtio-console: notify about the terminal size

[PATCH v4 00/10] virtio-console: notify about the terminal size

[Filip Hejsek]

The goal of this series is to have a resizable terminal into a guest
without having to set up networking and using, e.g. ssh.

The virtio spec allows a virtio-console device to notify the guest about
terminal resizes in the host. Linux Kernel implements the driver part of
the spec. This series implement the device part in QEMU.

This series adds support for a resizable terminal if a virtio console
device is connected to the stdio backend.

This series also introduces resize messages that can be sent over QMP to
notify QEMU about the size of the terminal connented to some chardev.
In the libvirt setting, it will allow to implement a resizable terminal
for virsh console and other libvirt clients.

This patch series was originally authored by Szymon Lukasz and submitted
to qemu-devel about 5 years ago. The previous submission can be found at
<https://lists.nongnu.org/archive/html/qemu-devel/2020-06/msg09591.html>.
I have updated the patches to be compatible with latest master and made
a few small changes of my own, including the addition of Windows support.

Probably the most important change I made is the swapping of rows and
cols fields in resize messages. I would like to hear some feedback on
this change from reviewers. The problem is that for a long time, the
Linux kernel used a different field order from what was specified in the
virtio spec. The kernel implementation was apparently merged around 2010,
while the virtio spec came in 2014, so when a previous version of this
patch series was being discussed here on this mailing list in 2020, it
was decided that QEMU should match the Linux implementation, and ideally,
the virtio spec should be changed.

However, recently, the Linux kernel implementation was changed to conform
to the spec: <https://git.kernel.org/linus/5326ab737a47278dbd16ed3ee7380b26c7056ddd>.
As a result, to be compatible with latest kernel releases, QEMU needs to
also use the field order matching the specification. I have changed the
patch to use the spec-compliant order, so it works correctly with latest
kernels now.

That leaves the issue of older kernels. There are about 15 years' worth
of kernel versions with the swapped field order, including the kernel
currently shipped in Debian stable. The effects of the swapped dimensions
can sometimes be quite annoying - e.g. if you have a terminal with
24 rows, this will be interpreted as 24 columns, and your shell may limit 
line editing to this small space, most of which will be taken by your
prompt. The patch series in its current form provides no way to disable
the console size functionality, so users may end up worse off than if
the functionality were not implemented at all.

PS: One of the patches adds one additional noop switch case
    to a bunch of files. I didn't include the maintainers of these files
    in the Cc list. I hope that's OK. :)

v4:
changed order of rows and cols fields
added support for terminal size on Windows
trace event is also emitted for legacy (non-multiport) drivers
minor fixes required because of changes in QEMU (DECLARE_INSTANCE_CHECKER, qmp-example)
updated version numbers ('Since: 10.2', hw_compat_10_1)

v3:
add resize messages over QMP, as suggested by Daniel

v2:
fix adding a new virtio feature bit to the virtio console device

---
Filip Hejsek (1):
      char-win-stdio: add support for terminal size

Szymon Lukasz (9):
      chardev: add cols, rows fields
      chardev: add CHR_EVENT_RESIZE
      chardev: add qemu_chr_resize()
      char-mux: add support for the terminal size
      main-loop: change the handling of SIGWINCH
      char-stdio: add support for the terminal size
      qmp: add chardev-resize command
      virtio-serial-bus: add terminal resize messages
      virtio-console: notify the guest about terminal resizes

 backends/cryptodev-vhost-user.c   |  1 +
 chardev/char-fe.c                 | 13 ++++++++
 chardev/char-mux.c                | 18 ++++++++++-
 chardev/char-stdio.c              | 30 +++++++++++++++++++
 chardev/char-win-stdio.c          | 19 ++++++++++++
 chardev/char.c                    | 26 ++++++++++++++++
 hw/block/vhost-user-blk.c         |  1 +
 hw/char/terminal3270.c            |  1 +
 hw/char/trace-events              |  1 +
 hw/char/virtio-console.c          | 63 ++++++++++++++++++++++++++++++++++++---
 hw/char/virtio-serial-bus.c       | 43 ++++++++++++++++++++++++--
 hw/core/machine.c                 |  4 ++-
 hw/ipmi/ipmi_bmc_extern.c         |  1 +
 hw/scsi/vhost-user-scsi.c         |  1 +
 hw/usb/ccid-card-passthru.c       |  1 +
 hw/usb/dev-serial.c               |  1 +
 hw/usb/redirect.c                 |  1 +
 hw/virtio/vhost-user-base.c       |  1 +
 hw/virtio/vhost-user-scmi.c       |  1 +
 include/chardev/char-fe.h         | 10 +++++++
 include/chardev/char.h            |  7 +++++
 include/hw/virtio/virtio-serial.h |  5 ++++
 include/qemu/main-loop.h          |  4 +++
 monitor/hmp.c                     |  1 +
 monitor/qmp.c                     |  1 +
 net/passt.c                       |  1 +
 net/vhost-user.c                  |  1 +
 qapi/char.json                    | 22 ++++++++++++++
 ui/curses.c                       | 11 +++----
 util/main-loop.c                  | 21 +++++++++++++
 30 files changed, 298 insertions(+), 13 deletions(-)
---
base-commit: 190d5d7fd725ff754f94e8e0cbfb69f279c82b5d
change-id: 20250912-console-resize-96c42140ba08

Best regards,
-- 
Filip Hejsek <filip.hejsek@gmail.com>

[PATCH v4 01/10] chardev: add cols, rows fields

[Filip Hejsek]

From: Szymon Lukasz <noh4hss@gmail.com>

These fields should be interpreted as the size of the terminal connected
to a given chardev.

Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
 chardev/char-fe.c         | 13 +++++++++++++
 include/chardev/char-fe.h | 10 ++++++++++
 include/chardev/char.h    |  1 +
 3 files changed, 24 insertions(+)

diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 158a5f4f551ee49120eee6ebdf772fb450739f47..8622898bd414c208b5a0397b439e18a8bf0b8a92 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -329,6 +329,19 @@ void qemu_chr_fe_set_echo(CharBackend *be, bool echo)
     }
 }
 
+void qemu_chr_fe_get_winsize(CharBackend *be, uint16_t *cols, uint16_t *rows)
+{
+    Chardev *chr = be->chr;
+
+    if (chr) {
+        *cols = chr->cols;
+        *rows = chr->rows;
+    } else {
+        *cols = 0;
+        *rows = 0;
+    }
+}
+
 void qemu_chr_fe_set_open(CharBackend *be, bool is_open)
 {
     Chardev *chr = be->chr;
diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index 8ef05b3dd095bdcaa51b10482261a29b1e8233c7..02d5606fa343ac64a76f48dcd250b5431a0a7761 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -158,6 +158,16 @@ int qemu_chr_fe_wait_connected(CharBackend *be, Error **errp);
  */
 void qemu_chr_fe_set_echo(CharBackend *be, bool echo);
 
+/**
+ * qemu_chr_fe_get_winsize:
+ * @cols: the address for storing columns
+ * @rows: the address for storing rows
+ *
+ * Get the size of the terminal connected to the chardev backend.
+ * Returns *cols = *rows = 0, if no associated Chardev.
+ */
+void qemu_chr_fe_get_winsize(CharBackend *be, uint16_t *cols, uint16_t *rows);
+
 /**
  * qemu_chr_fe_set_open:
  * @be: a CharBackend
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 429852f8d9d3c5a7e061acea3561b019b15d658f..336b2e68d099e70a9abe71ef842b160bf12ea932 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -65,6 +65,7 @@ struct Chardev {
     char *filename;
     int logfd;
     int be_open;
+    uint16_t cols, rows;
     /* used to coordinate the chardev-change special-case: */
     bool handover_yank_instance;
     GSource *gsource;

-- 
2.51.0

[PATCH v4 02/10] chardev: add CHR_EVENT_RESIZE

[Filip Hejsek]

From: Szymon Lukasz <noh4hss@gmail.com>

Add a new chardev event, CHR_EVENT_RESIZE, which a backend should
trigger if detects the size of the connected terminal changed.

Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
 backends/cryptodev-vhost-user.c | 1 +
 chardev/char.c                  | 1 +
 hw/block/vhost-user-blk.c       | 1 +
 hw/char/terminal3270.c          | 1 +
 hw/char/virtio-console.c        | 1 +
 hw/ipmi/ipmi_bmc_extern.c       | 1 +
 hw/scsi/vhost-user-scsi.c       | 1 +
 hw/usb/ccid-card-passthru.c     | 1 +
 hw/usb/dev-serial.c             | 1 +
 hw/usb/redirect.c               | 1 +
 hw/virtio/vhost-user-base.c     | 1 +
 hw/virtio/vhost-user-scmi.c     | 1 +
 include/chardev/char.h          | 4 ++++
 monitor/hmp.c                   | 1 +
 monitor/qmp.c                   | 1 +
 net/passt.c                     | 1 +
 net/vhost-user.c                | 1 +
 17 files changed, 20 insertions(+)

diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c
index cb04e68b022abcc4d794df741dfc25181db660bc..9daca4b0f91e802f67cf3c7af1f8f26479026e1e 100644
--- a/backends/cryptodev-vhost-user.c
+++ b/backends/cryptodev-vhost-user.c
@@ -173,6 +173,7 @@ static void cryptodev_vhost_user_event(void *opaque, QEMUChrEvent event)
     case CHR_EVENT_BREAK:
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }
diff --git a/chardev/char.c b/chardev/char.c
index bbebd246c3a46c848847effc775f6390b5078e14..635d19fea4fd4bd0c7f171f055fe940f9f5ebed5 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -75,6 +75,7 @@ void qemu_chr_be_event(Chardev *s, QEMUChrEvent event)
     case CHR_EVENT_BREAK:
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index c0cc5f6942815aa5e8d972553b3c76a623523ba4..942bc46171824017c98774d6fe793059082385b6 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -412,6 +412,7 @@ static void vhost_user_blk_event(void *opaque, QEMUChrEvent event)
     case CHR_EVENT_BREAK:
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }
diff --git a/hw/char/terminal3270.c b/hw/char/terminal3270.c
index d950c172921f91e41fad2b86a8d2a3791f6b0330..2fdf823e3bab3187fa6a4d2214ecd991bac5f473 100644
--- a/hw/char/terminal3270.c
+++ b/hw/char/terminal3270.c
@@ -172,6 +172,7 @@ static void chr_event(void *opaque, QEMUChrEvent event)
     case CHR_EVENT_BREAK:
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }
diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 0932a3572b78eda866cd7680a32866f2077a91dd..881c862ce9d12027f392031bdea7dbe280ec5493 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -168,6 +168,7 @@ static void chr_event(void *opaque, QEMUChrEvent event)
     case CHR_EVENT_BREAK:
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }
diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
index 9f1ba7b2f8715daf3558fedde358c625c74da20e..b1f28c5b7db71d2cbd828227f176a484629a14f7 100644
--- a/hw/ipmi/ipmi_bmc_extern.c
+++ b/hw/ipmi/ipmi_bmc_extern.c
@@ -436,6 +436,7 @@ static void chr_event(void *opaque, QEMUChrEvent event)
     case CHR_EVENT_BREAK:
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 25f2d894e7c827744a41302473712e7b34672536..a8d40046b714e5899262af7382a02611cb7f9a81 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -226,6 +226,7 @@ static void vhost_user_scsi_event(void *opaque, QEMUChrEvent event)
     case CHR_EVENT_BREAK:
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }
diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c
index 1eea21a7337747667e9b1db97900c67b061f3729..08fec48dbe00cebef7e73053f2be76932175fb47 100644
--- a/hw/usb/ccid-card-passthru.c
+++ b/hw/usb/ccid-card-passthru.c
@@ -323,6 +323,7 @@ static void ccid_card_vscard_event(void *opaque, QEMUChrEvent event)
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
     case CHR_EVENT_CLOSED:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }
diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 1c116d8b0fef7b23dd076aa6e7e81e52bb51cdbb..dd70db7705def9b93bd69cf176ff0ed7d8cb2cad 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -576,6 +576,7 @@ static void usb_serial_event(void *opaque, QEMUChrEvent event)
         break;
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index f516ff42a114d014033ab9f2fce93d1ca48983a6..68c9b48f7a07558441c6d6cd3deccb98e833910b 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1390,6 +1390,7 @@ static void usbredir_chardev_event(void *opaque, QEMUChrEvent event)
     case CHR_EVENT_BREAK:
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }
diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
index ff67a020b471b2edfacc6d238ffc50b3ba36afc4..57ff26e775df46135b84449f22929fadd33c684a 100644
--- a/hw/virtio/vhost-user-base.c
+++ b/hw/virtio/vhost-user-base.c
@@ -267,6 +267,7 @@ static void vub_event(void *opaque, QEMUChrEvent event)
     case CHR_EVENT_BREAK:
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }
diff --git a/hw/virtio/vhost-user-scmi.c b/hw/virtio/vhost-user-scmi.c
index f9264c4374e87cb78f8937fda852089487b477e8..180787ec6d94793d4915a26389ae2bece231064e 100644
--- a/hw/virtio/vhost-user-scmi.c
+++ b/hw/virtio/vhost-user-scmi.c
@@ -214,6 +214,7 @@ static void vu_scmi_event(void *opaque, QEMUChrEvent event)
     case CHR_EVENT_BREAK:
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 336b2e68d099e70a9abe71ef842b160bf12ea932..45cb6349756ac8072dffab9354108caf90cd3565 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -22,6 +22,10 @@ typedef enum {
     CHR_EVENT_OPENED, /* new connection established */
     CHR_EVENT_MUX_IN, /* mux-focus was set to this terminal */
     CHR_EVENT_MUX_OUT, /* mux-focus will move on */
+    CHR_EVENT_RESIZE, /*
+                       * the size of the terminal connected to
+                       * the chardev changed
+                       */
     CHR_EVENT_CLOSED /* connection closed.  NOTE: currently this event
                       * is only bound to the read port of the chardev.
                       * Normally the read port and write port of a
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 34e2b8f748b425e1e4446e8e64aa25b1433d1162..be623a0ef0d0ebdf091d87e4d620175ad60d1566 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -1444,6 +1444,7 @@ static void monitor_event(void *opaque, QEMUChrEvent event)
         break;
 
     case CHR_EVENT_BREAK:
+    case CHR_EVENT_RESIZE:
         /* Ignored */
         break;
     }
diff --git a/monitor/qmp.c b/monitor/qmp.c
index cb99a12d94175b395033569e8ead908d9453e759..ff7548422bc60c64e47fc4f24adf092fc81c70b5 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -484,6 +484,7 @@ static void monitor_qmp_event(void *opaque, QEMUChrEvent event)
     case CHR_EVENT_BREAK:
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }
diff --git a/net/passt.c b/net/passt.c
index 32ecffb763b48af4e17b20b98537a4ffb3a6d3ea..2308e3b4d44aa410c374eb1b47e5081c2bef5cec 100644
--- a/net/passt.c
+++ b/net/passt.c
@@ -421,6 +421,7 @@ static void passt_vhost_user_event(void *opaque, QEMUChrEvent event)
     case CHR_EVENT_BREAK:
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 8b96157145a7ab719b15b1b70aadac1e88ebfd5f..1c7fdf8267d8c214dcb6ae92e0521747a9f59a6f 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -367,6 +367,7 @@ static void net_vhost_user_event(void *opaque, QEMUChrEvent event)
     case CHR_EVENT_BREAK:
     case CHR_EVENT_MUX_IN:
     case CHR_EVENT_MUX_OUT:
+    case CHR_EVENT_RESIZE:
         /* Ignore */
         break;
     }

-- 
2.51.0

[PATCH v4 03/10] chardev: add qemu_chr_resize()

[Filip Hejsek]

From: Szymon Lukasz <noh4hss@gmail.com>

This function should be called whenever we learn about a new size of
the terminal connected to a chardev.

Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
 chardev/char.c         | 11 +++++++++++
 include/chardev/char.h |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/chardev/char.c b/chardev/char.c
index 635d19fea4fd4bd0c7f171f055fe940f9f5ebed5..b45d79cb9b57643827eb7479257fdda2cf6b0434 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -351,6 +351,17 @@ int qemu_chr_wait_connected(Chardev *chr, Error **errp)
     return 0;
 }
 
+void qemu_chr_resize(Chardev *chr, uint16_t cols, uint16_t rows)
+{
+    if (cols != chr->cols || rows != chr->rows) {
+        chr->cols = cols;
+        chr->rows = rows;
+        if (chr->be_open) {
+            qemu_chr_be_event(chr, CHR_EVENT_RESIZE);
+        }
+    }
+}
+
 QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename,
                                 bool permit_mux_mon)
 {
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 45cb6349756ac8072dffab9354108caf90cd3565..1e69b038241074d627ebb7f096e98aee9953ebdf 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -232,6 +232,8 @@ int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all);
 #define qemu_chr_write_all(s, buf, len) qemu_chr_write(s, buf, len, true)
 int qemu_chr_wait_connected(Chardev *chr, Error **errp);
 
+void qemu_chr_resize(Chardev *chr, uint16_t cols, uint16_t rows);
+
 #define TYPE_CHARDEV "chardev"
 OBJECT_DECLARE_TYPE(Chardev, ChardevClass, CHARDEV)
 

-- 
2.51.0

[PATCH v4 04/10] char-mux: add support for the terminal size

[Filip Hejsek]

From: Szymon Lukasz <noh4hss@gmail.com>

The terminal size of a mux chardev should be the same as the real
chardev, so listen for CHR_EVENT_RESIZE to be up to date.

We forward CHR_EVENT_RESIZE only to the focused frontend. This means
frontends should probably update their view of the terminal size on
receiving CHR_EVENT_MUX_IN.

Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
 chardev/char-mux.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 6b36290e2c49f579580d2abb5aa552806f019d4a..4d3d05b82f13e002c766142f9d9c24977b8b9bd2 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -264,9 +264,24 @@ void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event)
     }
 }
 
+static void mux_update_winsize(Chardev *chr)
+{
+    MuxChardev *d = MUX_CHARDEV(chr);
+    uint16_t cols, rows;
+
+    qemu_chr_fe_get_winsize(&d->chr, &cols, &rows);
+    qemu_chr_resize(chr, cols, rows);
+}
+
 static void mux_chr_event(void *opaque, QEMUChrEvent event)
 {
-    mux_chr_send_all_event(CHARDEV(opaque), event);
+    Chardev *chr = CHARDEV(opaque);
+
+    if (event == CHR_EVENT_RESIZE) {
+        mux_update_winsize(chr);
+    } else {
+        mux_chr_send_all_event(chr, event);
+    }
 }
 
 static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
@@ -382,6 +397,7 @@ static void qemu_chr_open_mux(Chardev *chr,
      */
     *be_opened = muxes_opened;
     qemu_chr_fe_init(&d->chr, drv, errp);
+    mux_update_winsize(chr);
 }
 
 static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend,

-- 
2.51.0

[PATCH v4 05/10] main-loop: change the handling of SIGWINCH

[Filip Hejsek]

From: Szymon Lukasz <noh4hss@gmail.com>

Block SIGWINCH, so it is delivered only via signalfd.
Install a handler that uses NotifierList to tell
interested parties about SIGWINCH delivery.

Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
 include/qemu/main-loop.h |  4 ++++
 ui/curses.c              | 11 ++++++-----
 util/main-loop.c         | 21 +++++++++++++++++++++
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 4e2436b1968b5c513f7d4e84e010b0d4fb31a1b1..7cc45c3a274434020fe33b1ca0a4d839de994e97 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -431,4 +431,8 @@ typedef struct MainLoopPoll {
 void main_loop_poll_add_notifier(Notifier *notify);
 void main_loop_poll_remove_notifier(Notifier *notify);
 
+#ifndef _WIN32
+void sigwinch_add_notifier(Notifier *n);
+#endif
+
 #endif
diff --git a/ui/curses.c b/ui/curses.c
index 161f78c35c32fc03ad576d2bd8e91bdfe09b265d..d1b308d5f8051e99f12f4d32435a04e294060a10 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -33,6 +33,7 @@
 #include <iconv.h>
 
 #include "qapi/error.h"
+#include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "ui/console.h"
 #include "ui/input.h"
@@ -149,7 +150,7 @@ static void curses_resize(DisplayChangeListener *dcl,
 }
 
 #if !defined(_WIN32) && defined(SIGWINCH) && defined(KEY_RESIZE)
-static volatile sig_atomic_t got_sigwinch;
+static bool got_sigwinch;
 static void curses_winch_check(void)
 {
     struct winsize {
@@ -172,17 +173,17 @@ static void curses_winch_check(void)
     invalidate = 1;
 }
 
-static void curses_winch_handler(int signum)
+static void curses_winch_handler(Notifier *n, void *data)
 {
     got_sigwinch = true;
 }
 
 static void curses_winch_init(void)
 {
-    struct sigaction old, winch = {
-        .sa_handler  = curses_winch_handler,
+    static Notifier n = {
+        .notify = curses_winch_handler
     };
-    sigaction(SIGWINCH, &winch, &old);
+    sigwinch_add_notifier(&n);
 }
 #else
 static void curses_winch_check(void) {}
diff --git a/util/main-loop.c b/util/main-loop.c
index 51aeb2432e77eae7081c6945e21812acc71b5f37..db4bb9c88dade805bc98322c1a053c65e9e97f7e 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -100,6 +100,7 @@ static int qemu_signal_init(Error **errp)
     sigaddset(&set, SIGIO);
     sigaddset(&set, SIGALRM);
     sigaddset(&set, SIGBUS);
+    sigaddset(&set, SIGWINCH);
     /* SIGINT cannot be handled via signalfd, so that ^C can be used
      * to interrupt QEMU when it is being run under gdb.  SIGHUP and
      * SIGTERM are also handled asynchronously, even though it is not
@@ -121,6 +122,26 @@ static int qemu_signal_init(Error **errp)
     return 0;
 }
 
+static NotifierList sigwinch_notifiers =
+    NOTIFIER_LIST_INITIALIZER(sigwinch_notifiers);
+
+static void sigwinch_handler(int signum)
+{
+    notifier_list_notify(&sigwinch_notifiers, NULL);
+}
+
+void sigwinch_add_notifier(Notifier *n)
+{
+    if (notifier_list_empty(&sigwinch_notifiers)) {
+        struct sigaction action = {
+            .sa_handler = sigwinch_handler,
+        };
+        sigaction(SIGWINCH, &action, NULL);
+    }
+
+    notifier_list_add(&sigwinch_notifiers, n);
+}
+
 #else /* _WIN32 */
 
 static int qemu_signal_init(Error **errp)

-- 
2.51.0

[PATCH v4 06/10] char-stdio: add support for the terminal size

[Filip Hejsek]

From: Szymon Lukasz <noh4hss@gmail.com>

Update the terminal size upon SIGWINCH delivery.

Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
 chardev/char-stdio.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/chardev/char-stdio.c b/chardev/char-stdio.c
index 48db8d2f30fcf0b481c79ea69aab720454596a05..b3475391f088f1e570b74cc40e30f679dbe9b574 100644
--- a/chardev/char-stdio.c
+++ b/chardev/char-stdio.c
@@ -34,7 +34,9 @@
 #include "chardev/char-win-stdio.h"
 #else
 #include <termios.h>
+#include <sys/ioctl.h>
 #include "chardev/char-fd.h"
+#include "qemu/main-loop.h"
 #endif
 
 #ifndef _WIN32
@@ -46,6 +48,14 @@ static bool stdio_in_use;
 static bool stdio_allow_signal;
 static bool stdio_echo_state;
 
+typedef struct {
+    FDChardev parent;
+    Notifier resize_notifier;
+} StdioChardev;
+
+DECLARE_INSTANCE_CHECKER(StdioChardev, STDIO_CHARDEV,
+                         TYPE_CHARDEV_STDIO)
+
 static void term_exit(void)
 {
     if (stdio_in_use) {
@@ -85,11 +95,26 @@ static void term_stdio_handler(int sig)
     qemu_chr_set_echo_stdio(NULL, stdio_echo_state);
 }
 
+static void qemu_chr_resize_stdio(Chardev *chr)
+{
+    struct winsize ws;
+    if (ioctl(1, TIOCGWINSZ, &ws) != -1) {
+        qemu_chr_resize(chr, ws.ws_col, ws.ws_row);
+    }
+}
+
+static void term_resize_notify(Notifier *n, void *data)
+{
+    StdioChardev *s = container_of(n, StdioChardev, resize_notifier);
+    qemu_chr_resize_stdio(CHARDEV(s));
+}
+
 static void qemu_chr_open_stdio(Chardev *chr,
                                 ChardevBackend *backend,
                                 bool *be_opened,
                                 Error **errp)
 {
+    StdioChardev *s = STDIO_CHARDEV(chr);
     ChardevStdio *opts = backend->u.stdio.data;
     struct sigaction act;
 
@@ -121,6 +146,10 @@ static void qemu_chr_open_stdio(Chardev *chr,
 
     stdio_allow_signal = !opts->has_signal || opts->signal;
     qemu_chr_set_echo_stdio(chr, false);
+
+    qemu_chr_resize_stdio(chr);
+    s->resize_notifier.notify = term_resize_notify;
+    sigwinch_add_notifier(&s->resize_notifier);
 }
 #endif
 
@@ -160,6 +189,7 @@ static const TypeInfo char_stdio_type_info = {
     .parent = TYPE_CHARDEV_WIN_STDIO,
 #else
     .parent = TYPE_CHARDEV_FD,
+    .instance_size = sizeof(StdioChardev),
 #endif
     .instance_finalize = char_stdio_finalize,
     .class_init = char_stdio_class_init,

-- 
2.51.0

[PATCH v4 07/10] qmp: add chardev-resize command

[Filip Hejsek]

From: Szymon Lukasz <noh4hss@gmail.com>

The managment software can use this command to notify QEMU about the
size of the terminal connected to a chardev, QEMU can then forward this
information to the guest if the chardev is connected to a virtio console
device.

Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
 chardev/char.c | 14 ++++++++++++++
 qapi/char.json | 22 ++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/chardev/char.c b/chardev/char.c
index b45d79cb9b57643827eb7479257fdda2cf6b0434..6e3ade98614c949be8041ec5905a490ff536dee9 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -1269,6 +1269,20 @@ bool qmp_add_client_char(int fd, bool has_skipauth, bool skipauth,
     return true;
 }
 
+void qmp_chardev_resize(const char *id, uint16_t cols, uint16_t rows,
+                        Error **errp)
+{
+    Chardev *chr;
+
+    chr = qemu_chr_find(id);
+    if (chr == NULL) {
+        error_setg(errp, "Chardev '%s' not found", id);
+        return;
+    }
+
+    qemu_chr_resize(chr, cols, rows);
+}
+
 /*
  * Add a timeout callback for the chardev (in milliseconds), return
  * the GSource object created. Please use this to add timeout hook for
diff --git a/qapi/char.json b/qapi/char.json
index f0a53f742c8bee24c377551803a864fd36ac78cf..0a26c5eee6b71bc5de127a91b253cc69a9fe8ce6 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -874,6 +874,28 @@
 { 'command': 'chardev-send-break',
   'data': { 'id': 'str' } }
 
+##
+# @chardev-resize:
+#
+# Notifies a chardev about the current size of the terminal connected
+# to this chardev.
+#
+# @id: the chardev's ID, must exist
+# @cols: the number of columns
+# @rows: the number of rows
+#
+# Since: 10.2
+#
+# .. qmp-example::
+#
+#     -> { "execute": "chardev-resize", "arguments": { "id": "foo", "cols": 80, "rows": 24 } }
+#     <- { "return": {} }
+##
+{ 'command': 'chardev-resize',
+  'data': { 'id': 'str',
+            'cols': 'uint16',
+            'rows': 'uint16' } }
+
 ##
 # @VSERPORT_CHANGE:
 #

-- 
2.51.0

[PATCH v4 08/10] virtio-serial-bus: add terminal resize messages

[Filip Hejsek]

From: Szymon Lukasz <noh4hss@gmail.com>

Implement the part of the virtio spec that allows to notify the virtio
driver about terminal resizes. The virtio spec contains two methods to
achieve that:

For legacy drivers, we have only one port and we put the terminal size
in the config space and inject the config changed interrupt.

For multiport devices, we use the control virtqueue to send a packet
containing the terminal size. Note that old versions of the Linux kernel
used an incorrect order for the fields (rows then cols instead of cols
then rows), until it was fixed by commit 5326ab737a47278dbd16ed3ee7380b26c7056ddd.

As a result, when using a Linux kernel older than 6.15, the number of rows
and columns will be swapped.

Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
 hw/char/trace-events              |  1 +
 hw/char/virtio-serial-bus.c       | 43 +++++++++++++++++++++++++++++++++++++--
 hw/core/machine.c                 |  4 +++-
 include/hw/virtio/virtio-serial.h |  5 +++++
 4 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/hw/char/trace-events b/hw/char/trace-events
index 05a33036c12070242c2b193c19011839d623bec4..9a975ab1e2a525a9391d0f0a85ddbe80aa6361fc 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -11,6 +11,7 @@ serial_update_parameters(uint64_t baudrate, char parity, int data_bits, int stop
 
 # virtio-serial-bus.c
 virtio_serial_send_control_event(unsigned int port, uint16_t event, uint16_t value) "port %u, event %u, value %u"
+virtio_serial_send_console_resize(unsigned int port, uint16_t cols, uint16_t rows) "port %u, cols %u, rows %u"
 virtio_serial_throttle_port(unsigned int port, bool throttle) "port %u, throttle %d"
 virtio_serial_handle_control_message(uint16_t event, uint16_t value) "event %u, value %u"
 virtio_serial_handle_control_message_port(unsigned int port) "port %u"
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 673c50f0be08ef9b7142c16eaf8e6e31c7a00ca5..1d2963efcd74494a1f0d428f8ace0e72bb4c6647 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -260,6 +260,43 @@ static size_t send_control_event(VirtIOSerial *vser, uint32_t port_id,
     return send_control_msg(vser, &cpkt, sizeof(cpkt));
 }
 
+/*
+ * This struct should be added to the Linux kernel uapi headers
+ * and later imported to standard-headers/linux/virtio_console.h
+ */
+struct virtio_console_resize {
+    __virtio16 cols;
+    __virtio16 rows;
+};
+
+void virtio_serial_send_console_resize(VirtIOSerialPort *port,
+                                       uint16_t cols, uint16_t rows)
+{
+    VirtIOSerial *vser = port->vser;
+    VirtIODevice *vdev = VIRTIO_DEVICE(vser);
+
+    trace_virtio_serial_send_console_resize(port->id, cols, rows);
+
+    if (virtio_vdev_has_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT)) {
+        struct {
+            struct virtio_console_control control;
+            struct virtio_console_resize resize;
+        } buffer;
+
+        virtio_stl_p(vdev, &buffer.control.id, port->id);
+        virtio_stw_p(vdev, &buffer.control.event, VIRTIO_CONSOLE_RESIZE);
+        virtio_stw_p(vdev, &buffer.resize.cols, cols);
+        virtio_stw_p(vdev, &buffer.resize.rows, rows);
+
+        send_control_msg(vser, &buffer, sizeof(buffer));
+
+    } else if (virtio_vdev_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE)) {
+        vser->port0_cols = cols;
+        vser->port0_rows = rows;
+        virtio_notify_config(vdev);
+    }
+}
+
 /* Functions for use inside qemu to open and read from/write to ports */
 int virtio_serial_open(VirtIOSerialPort *port)
 {
@@ -571,8 +608,8 @@ static void get_config(VirtIODevice *vdev, uint8_t *config_data)
     struct virtio_console_config *config =
         (struct virtio_console_config *)config_data;
 
-    config->cols = 0;
-    config->rows = 0;
+    config->cols = virtio_tswap16(vdev, vser->port0_cols);
+    config->rows = virtio_tswap16(vdev, vser->port0_rows);
     config->max_nr_ports = virtio_tswap32(vdev,
                                           vser->serial.max_virtserial_ports);
 }
@@ -1158,6 +1195,8 @@ static const Property virtio_serial_properties[] = {
                                                   31),
     DEFINE_PROP_BIT64("emergency-write", VirtIOSerial, host_features,
                       VIRTIO_CONSOLE_F_EMERG_WRITE, true),
+    DEFINE_PROP_BIT64("console-size", VirtIOSerial, host_features,
+                      VIRTIO_CONSOLE_F_SIZE, true),
 };
 
 static void virtio_serial_class_init(ObjectClass *klass, const void *data)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 38c949c4f2ce4a117cbfca62f56919711ce392b4..74a747ec6578c958b35e1f9712e5dbed7bca72e8 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -37,7 +37,9 @@
 #include "hw/virtio/virtio-iommu.h"
 #include "audio/audio.h"
 
-GlobalProperty hw_compat_10_1[] = {};
+GlobalProperty hw_compat_10_1[] = {
+    { "virtio-serial-device", "console-size", "off" },
+};
 const size_t hw_compat_10_1_len = G_N_ELEMENTS(hw_compat_10_1);
 
 GlobalProperty hw_compat_10_0[] = {
diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
index d87c62eab7a270809daf47f932a73dd1fa3d5a6e..81efa853f804a52866890a9ec2c71bfbcabca4a0 100644
--- a/include/hw/virtio/virtio-serial.h
+++ b/include/hw/virtio/virtio-serial.h
@@ -187,6 +187,8 @@ struct VirtIOSerial {
     virtio_serial_conf serial;
 
     uint64_t host_features;
+
+    uint16_t port0_cols, port0_rows;
 };
 
 /* Interface to the virtio-serial bus */
@@ -221,6 +223,9 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port);
  */
 void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle);
 
+void virtio_serial_send_console_resize(VirtIOSerialPort *port,
+                                       uint16_t cols, uint16_t rows);
+
 #define TYPE_VIRTIO_SERIAL "virtio-serial-device"
 OBJECT_DECLARE_SIMPLE_TYPE(VirtIOSerial, VIRTIO_SERIAL)
 

-- 
2.51.0

[PATCH v4 09/10] virtio-console: notify the guest about terminal resizes

[Filip Hejsek]

From: Szymon Lukasz <noh4hss@gmail.com>

If a virtio serial port is a console port forward terminal resize
messages from the chardev backend to the guest.

Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
 hw/char/virtio-console.c | 62 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 58 insertions(+), 4 deletions(-)

diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 881c862ce9d12027f392031bdea7dbe280ec5493..0dd10a81f151b0606f6060ab2b4936117d81dd83 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -32,6 +32,7 @@ struct VirtConsole {
 
     CharBackend chr;
     guint watch;
+    uint16_t cols, rows;
 };
 
 /*
@@ -107,6 +108,33 @@ static ssize_t flush_buf(VirtIOSerialPort *port,
     return ret;
 }
 
+static void virtconsole_send_resize(VirtIOSerialPort *port)
+{
+    uint16_t cols, rows;
+    VirtConsole *vcon = VIRTIO_CONSOLE(port);
+
+    /*
+     * We probably shouldn't send these messages before
+     * we told the guest it is a console port (which we do
+     * by sending VIRTIO_CONSOLE_CONSOLE_PORT message).
+     * Instead of adding a new field to the device state
+     * lets just use the guest_connected field for that purpose
+     * since the guest should not care about the terminal size
+     * before opening the port.
+     */
+    if (!port->guest_connected) {
+        return;
+    }
+
+    qemu_chr_fe_get_winsize(&vcon->chr, &cols, &rows);
+
+    if (cols != vcon->cols || rows != vcon->rows) {
+        vcon->cols = cols;
+        vcon->rows = rows;
+        virtio_serial_send_console_resize(port, cols, rows);
+    }
+}
+
 /* Callback function that's called when the guest opens/closes the port */
 static void set_guest_connected(VirtIOSerialPort *port, int guest_connected)
 {
@@ -114,7 +142,9 @@ static void set_guest_connected(VirtIOSerialPort *port, int guest_connected)
     DeviceState *dev = DEVICE(port);
     VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
 
-    if (!k->is_console) {
+    if (k->is_console) {
+        virtconsole_send_resize(port);
+    } else {
         qemu_chr_fe_set_open(&vcon->chr, guest_connected);
     }
 
@@ -174,6 +204,23 @@ static void chr_event(void *opaque, QEMUChrEvent event)
     }
 }
 
+static void chr_event_console(void *opaque, QEMUChrEvent event)
+{
+    VirtConsole *vcon = opaque;
+    VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(vcon);
+
+    trace_virtio_console_chr_event(port->id, event);
+    switch (event) {
+    case CHR_EVENT_OPENED:
+    case CHR_EVENT_MUX_IN:
+    case CHR_EVENT_RESIZE:
+        virtconsole_send_resize(port);
+        break;
+    default:
+        break;
+    }
+}
+
 static int chr_be_change(void *opaque)
 {
     VirtConsole *vcon = opaque;
@@ -182,7 +229,9 @@ static int chr_be_change(void *opaque)
 
     if (k->is_console) {
         qemu_chr_fe_set_handlers(&vcon->chr, chr_can_read, chr_read,
-                                 NULL, chr_be_change, vcon, NULL, true);
+                                 chr_event_console, chr_be_change,
+                                 vcon, NULL, true);
+        virtconsole_send_resize(port);
     } else {
         qemu_chr_fe_set_handlers(&vcon->chr, chr_can_read, chr_read,
                                  chr_event, chr_be_change, vcon, NULL, false);
@@ -210,7 +259,7 @@ static void virtconsole_enable_backend(VirtIOSerialPort *port, bool enable)
         VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
 
         qemu_chr_fe_set_handlers(&vcon->chr, chr_can_read, chr_read,
-                                 k->is_console ? NULL : chr_event,
+                                 k->is_console ? chr_event_console : chr_event,
                                  chr_be_change, vcon, NULL, false);
     } else {
         qemu_chr_fe_set_handlers(&vcon->chr, NULL, NULL, NULL,
@@ -230,6 +279,11 @@ static void virtconsole_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    if (k->is_console) {
+        vcon->cols = (uint16_t) -1;
+        vcon->rows = (uint16_t) -1;
+    }
+
     if (qemu_chr_fe_backend_connected(&vcon->chr)) {
         /*
          * For consoles we don't block guest data transfer just
@@ -242,7 +296,7 @@ static void virtconsole_realize(DeviceState *dev, Error **errp)
          */
         if (k->is_console) {
             qemu_chr_fe_set_handlers(&vcon->chr, chr_can_read, chr_read,
-                                     NULL, chr_be_change,
+                                     chr_event_console, chr_be_change,
                                      vcon, NULL, true);
             virtio_serial_open(port);
         } else {

-- 
2.51.0

[PATCH v4 10/10] char-win-stdio: add support for terminal size

[Filip Hejsek]

Use GetConsoleScreenBufferInfo to obtain terminal size
and set ENABLE_WINDOW_INPUT to receive resize notifications.

Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
 chardev/char-win-stdio.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/chardev/char-win-stdio.c b/chardev/char-win-stdio.c
index fb802a00b13ac4089abf3bd4f8c4198d8325764b..dbffbe1d11702367d4ecfcb63fad7c3c18324ad6 100644
--- a/chardev/char-win-stdio.c
+++ b/chardev/char-win-stdio.c
@@ -44,6 +44,20 @@ typedef struct WinStdioChardev WinStdioChardev;
 DECLARE_INSTANCE_CHECKER(WinStdioChardev, WIN_STDIO_CHARDEV,
                          TYPE_CHARDEV_WIN_STDIO)
 
+static void char_win_stdio_resize(Chardev *chr)
+{
+    HANDLE hStdOut = GetStdHandle(STD_OUTPUT_HANDLE);
+    CONSOLE_SCREEN_BUFFER_INFO ScreenBufferInfo;
+
+    if (GetConsoleScreenBufferInfo(hStdOut, &ScreenBufferInfo)) {
+        uint16_t rows = ScreenBufferInfo.srWindow.Right + 1
+                        - ScreenBufferInfo.srWindow.Left;
+        uint16_t cols = ScreenBufferInfo.srWindow.Bottom + 1
+                        - ScreenBufferInfo.srWindow.Top;
+        qemu_chr_resize(chr, rows, cols);
+    }
+}
+
 static void win_stdio_wait_func(void *opaque)
 {
     Chardev *chr = CHARDEV(opaque);
@@ -75,6 +89,9 @@ static void win_stdio_wait_func(void *opaque)
                 }
             }
         }
+        if (buf[i].EventType == WINDOW_BUFFER_SIZE_EVENT) {
+            char_win_stdio_resize(chr);
+        }
     }
 }
 
@@ -202,6 +219,8 @@ static void qemu_chr_open_stdio(Chardev *chr,
         } else {
             dwMode &= ~ENABLE_PROCESSED_INPUT;
         }
+        dwMode |= ENABLE_WINDOW_INPUT;
+        char_win_stdio_resize(chr);
     }
 
     SetConsoleMode(stdio->hStdIn, dwMode);

-- 
2.51.0

Re: [PATCH v4 00/10] virtio-console: notify about the terminal size

[Michael S. Tsirkin]

On Fri, Sep 12, 2025 at 05:39:45AM +0200, Filip Hejsek wrote:
> The goal of this series is to have a resizable terminal into a guest
> without having to set up networking and using, e.g. ssh.
> 
> The virtio spec allows a virtio-console device to notify the guest about
> terminal resizes in the host. Linux Kernel implements the driver part of
> the spec. This series implement the device part in QEMU.
> 
> This series adds support for a resizable terminal if a virtio console
> device is connected to the stdio backend.
> 
> This series also introduces resize messages that can be sent over QMP to
> notify QEMU about the size of the terminal connented to some chardev.
> In the libvirt setting, it will allow to implement a resizable terminal
> for virsh console and other libvirt clients.
> 
> This patch series was originally authored by Szymon Lukasz and submitted
> to qemu-devel about 5 years ago. The previous submission can be found at
> <https://lists.nongnu.org/archive/html/qemu-devel/2020-06/msg09591.html>.
> I have updated the patches to be compatible with latest master and made
> a few small changes of my own, including the addition of Windows support.
> 
> Probably the most important change I made is the swapping of rows and
> cols fields in resize messages. I would like to hear some feedback on
> this change from reviewers. The problem is that for a long time, the
> Linux kernel used a different field order from what was specified in the
> virtio spec. The kernel implementation was apparently merged around 2010,
> while the virtio spec came in 2014, so when a previous version of this
> patch series was being discussed here on this mailing list in 2020, it
> was decided that QEMU should match the Linux implementation, and ideally,
> the virtio spec should be changed.
> 
> However, recently, the Linux kernel implementation was changed to conform
> to the spec: <https://git.kernel.org/linus/5326ab737a47278dbd16ed3ee7380b26c7056ddd>.
> As a result, to be compatible with latest kernel releases, QEMU needs to
> also use the field order matching the specification. I have changed the
> patch to use the spec-compliant order, so it works correctly with latest
> kernels now.
>

Well this is not in any release yet. If you want me to revert that
one, let me know ASAP. Maximilian?

> That leaves the issue of older kernels. There are about 15 years' worth
> of kernel versions with the swapped field order, including the kernel
> currently shipped in Debian stable. The effects of the swapped dimensions
> can sometimes be quite annoying - e.g. if you have a terminal with
> 24 rows, this will be interpreted as 24 columns, and your shell may limit 
> line editing to this small space, most of which will be taken by your
> prompt. The patch series in its current form provides no way to disable
> the console size functionality,

Well I see the console-size property, no?

> so users may end up worse off than if
> the functionality were not implemented at all.

If we want to keep the Linux patch, the straight forward way is to send
the fix to stable@ then poke at Debian at al to fix their kernels.

Another option is to make the property default to off, have
management turn it on when guest is up to date.

But it really sounds like we should revert that Linux patch.
I posted a revert, pls comment.

> PS: One of the patches adds one additional noop switch case
>     to a bunch of files. I didn't include the maintainers of these files
>     in the Cc list. I hope that's OK. :)
> 
> v4:
> changed order of rows and cols fields
> added support for terminal size on Windows
> trace event is also emitted for legacy (non-multiport) drivers
> minor fixes required because of changes in QEMU (DECLARE_INSTANCE_CHECKER, qmp-example)
> updated version numbers ('Since: 10.2', hw_compat_10_1)
> 
> v3:
> add resize messages over QMP, as suggested by Daniel
> 
> v2:
> fix adding a new virtio feature bit to the virtio console device
> 
> ---
> Filip Hejsek (1):
>       char-win-stdio: add support for terminal size
> 
> Szymon Lukasz (9):
>       chardev: add cols, rows fields
>       chardev: add CHR_EVENT_RESIZE
>       chardev: add qemu_chr_resize()
>       char-mux: add support for the terminal size
>       main-loop: change the handling of SIGWINCH
>       char-stdio: add support for the terminal size
>       qmp: add chardev-resize command
>       virtio-serial-bus: add terminal resize messages
>       virtio-console: notify the guest about terminal resizes
> 
>  backends/cryptodev-vhost-user.c   |  1 +
>  chardev/char-fe.c                 | 13 ++++++++
>  chardev/char-mux.c                | 18 ++++++++++-
>  chardev/char-stdio.c              | 30 +++++++++++++++++++
>  chardev/char-win-stdio.c          | 19 ++++++++++++
>  chardev/char.c                    | 26 ++++++++++++++++
>  hw/block/vhost-user-blk.c         |  1 +
>  hw/char/terminal3270.c            |  1 +
>  hw/char/trace-events              |  1 +
>  hw/char/virtio-console.c          | 63 ++++++++++++++++++++++++++++++++++++---
>  hw/char/virtio-serial-bus.c       | 43 ++++++++++++++++++++++++--
>  hw/core/machine.c                 |  4 ++-
>  hw/ipmi/ipmi_bmc_extern.c         |  1 +
>  hw/scsi/vhost-user-scsi.c         |  1 +
>  hw/usb/ccid-card-passthru.c       |  1 +
>  hw/usb/dev-serial.c               |  1 +
>  hw/usb/redirect.c                 |  1 +
>  hw/virtio/vhost-user-base.c       |  1 +
>  hw/virtio/vhost-user-scmi.c       |  1 +
>  include/chardev/char-fe.h         | 10 +++++++
>  include/chardev/char.h            |  7 +++++
>  include/hw/virtio/virtio-serial.h |  5 ++++
>  include/qemu/main-loop.h          |  4 +++
>  monitor/hmp.c                     |  1 +
>  monitor/qmp.c                     |  1 +
>  net/passt.c                       |  1 +
>  net/vhost-user.c                  |  1 +
>  qapi/char.json                    | 22 ++++++++++++++
>  ui/curses.c                       | 11 +++----
>  util/main-loop.c                  | 21 +++++++++++++
>  30 files changed, 298 insertions(+), 13 deletions(-)
> ---
> base-commit: 190d5d7fd725ff754f94e8e0cbfb69f279c82b5d
> change-id: 20250912-console-resize-96c42140ba08
> 
> Best regards,
> -- 
> Filip Hejsek <filip.hejsek@gmail.com>

Re: [PATCH v4 00/10] virtio-console: notify about the terminal size

[Michael S. Tsirkin]

On Fri, Sep 12, 2025 at 04:41:05AM -0400, Michael S. Tsirkin wrote:
> On Fri, Sep 12, 2025 at 05:39:45AM +0200, Filip Hejsek wrote:
> > The goal of this series is to have a resizable terminal into a guest
> > without having to set up networking and using, e.g. ssh.
> > 
> > The virtio spec allows a virtio-console device to notify the guest about
> > terminal resizes in the host. Linux Kernel implements the driver part of
> > the spec. This series implement the device part in QEMU.
> > 
> > This series adds support for a resizable terminal if a virtio console
> > device is connected to the stdio backend.
> > 
> > This series also introduces resize messages that can be sent over QMP to
> > notify QEMU about the size of the terminal connented to some chardev.
> > In the libvirt setting, it will allow to implement a resizable terminal
> > for virsh console and other libvirt clients.
> > 
> > This patch series was originally authored by Szymon Lukasz and submitted
> > to qemu-devel about 5 years ago. The previous submission can be found at
> > <https://lists.nongnu.org/archive/html/qemu-devel/2020-06/msg09591.html>.
> > I have updated the patches to be compatible with latest master and made
> > a few small changes of my own, including the addition of Windows support.
> > 
> > Probably the most important change I made is the swapping of rows and
> > cols fields in resize messages. I would like to hear some feedback on
> > this change from reviewers. The problem is that for a long time, the
> > Linux kernel used a different field order from what was specified in the
> > virtio spec. The kernel implementation was apparently merged around 2010,
> > while the virtio spec came in 2014, so when a previous version of this
> > patch series was being discussed here on this mailing list in 2020, it
> > was decided that QEMU should match the Linux implementation, and ideally,
> > the virtio spec should be changed.
> > 
> > However, recently, the Linux kernel implementation was changed to conform
> > to the spec: <https://git.kernel.org/linus/5326ab737a47278dbd16ed3ee7380b26c7056ddd>.
> > As a result, to be compatible with latest kernel releases, QEMU needs to
> > also use the field order matching the specification. I have changed the
> > patch to use the spec-compliant order, so it works correctly with latest
> > kernels now.
> >
> 
> Well this is not in any release yet. If you want me to revert that
> one, let me know ASAP. Maximilian?
> 
> > That leaves the issue of older kernels. There are about 15 years' worth
> > of kernel versions with the swapped field order, including the kernel
> > currently shipped in Debian stable. The effects of the swapped dimensions
> > can sometimes be quite annoying - e.g. if you have a terminal with
> > 24 rows, this will be interpreted as 24 columns, and your shell may limit 
> > line editing to this small space, most of which will be taken by your
> > prompt. The patch series in its current form provides no way to disable
> > the console size functionality,
> 
> Well I see the console-size property, no?
> 
> > so users may end up worse off than if
> > the functionality were not implemented at all.
> 
> If we want to keep the Linux patch, the straight forward way is to send
> the fix to stable@ then poke at Debian at al to fix their kernels.
> 
> Another option is to make the property default to off, have
> management turn it on when guest is up to date.
> 
> But it really sounds like we should revert that Linux patch.
> I posted a revert, pls comment.

Oh no, I was confused. This was in 6.15 and 6.16 :(


Pls ignore this, I will reply separately.




> > PS: One of the patches adds one additional noop switch case
> >     to a bunch of files. I didn't include the maintainers of these files
> >     in the Cc list. I hope that's OK. :)
> > 
> > v4:
> > changed order of rows and cols fields
> > added support for terminal size on Windows
> > trace event is also emitted for legacy (non-multiport) drivers
> > minor fixes required because of changes in QEMU (DECLARE_INSTANCE_CHECKER, qmp-example)
> > updated version numbers ('Since: 10.2', hw_compat_10_1)
> > 
> > v3:
> > add resize messages over QMP, as suggested by Daniel
> > 
> > v2:
> > fix adding a new virtio feature bit to the virtio console device
> > 
> > ---
> > Filip Hejsek (1):
> >       char-win-stdio: add support for terminal size
> > 
> > Szymon Lukasz (9):
> >       chardev: add cols, rows fields
> >       chardev: add CHR_EVENT_RESIZE
> >       chardev: add qemu_chr_resize()
> >       char-mux: add support for the terminal size
> >       main-loop: change the handling of SIGWINCH
> >       char-stdio: add support for the terminal size
> >       qmp: add chardev-resize command
> >       virtio-serial-bus: add terminal resize messages
> >       virtio-console: notify the guest about terminal resizes
> > 
> >  backends/cryptodev-vhost-user.c   |  1 +
> >  chardev/char-fe.c                 | 13 ++++++++
> >  chardev/char-mux.c                | 18 ++++++++++-
> >  chardev/char-stdio.c              | 30 +++++++++++++++++++
> >  chardev/char-win-stdio.c          | 19 ++++++++++++
> >  chardev/char.c                    | 26 ++++++++++++++++
> >  hw/block/vhost-user-blk.c         |  1 +
> >  hw/char/terminal3270.c            |  1 +
> >  hw/char/trace-events              |  1 +
> >  hw/char/virtio-console.c          | 63 ++++++++++++++++++++++++++++++++++++---
> >  hw/char/virtio-serial-bus.c       | 43 ++++++++++++++++++++++++--
> >  hw/core/machine.c                 |  4 ++-
> >  hw/ipmi/ipmi_bmc_extern.c         |  1 +
> >  hw/scsi/vhost-user-scsi.c         |  1 +
> >  hw/usb/ccid-card-passthru.c       |  1 +
> >  hw/usb/dev-serial.c               |  1 +
> >  hw/usb/redirect.c                 |  1 +
> >  hw/virtio/vhost-user-base.c       |  1 +
> >  hw/virtio/vhost-user-scmi.c       |  1 +
> >  include/chardev/char-fe.h         | 10 +++++++
> >  include/chardev/char.h            |  7 +++++
> >  include/hw/virtio/virtio-serial.h |  5 ++++
> >  include/qemu/main-loop.h          |  4 +++
> >  monitor/hmp.c                     |  1 +
> >  monitor/qmp.c                     |  1 +
> >  net/passt.c                       |  1 +
> >  net/vhost-user.c                  |  1 +
> >  qapi/char.json                    | 22 ++++++++++++++
> >  ui/curses.c                       | 11 +++----
> >  util/main-loop.c                  | 21 +++++++++++++
> >  30 files changed, 298 insertions(+), 13 deletions(-)
> > ---
> > base-commit: 190d5d7fd725ff754f94e8e0cbfb69f279c82b5d
> > change-id: 20250912-console-resize-96c42140ba08
> > 
> > Best regards,
> > -- 
> > Filip Hejsek <filip.hejsek@gmail.com>

Re: [PATCH v4 00/10] virtio-console: notify about the terminal size

[Daniel P. Berrangé]

On Fri, Sep 12, 2025 at 04:41:02AM -0400, Michael S. Tsirkin wrote:
> On Fri, Sep 12, 2025 at 05:39:45AM +0200, Filip Hejsek wrote:
> > The goal of this series is to have a resizable terminal into a guest
> > without having to set up networking and using, e.g. ssh.
> > 
> > The virtio spec allows a virtio-console device to notify the guest about
> > terminal resizes in the host. Linux Kernel implements the driver part of
> > the spec. This series implement the device part in QEMU.
> > 
> > This series adds support for a resizable terminal if a virtio console
> > device is connected to the stdio backend.
> > 
> > This series also introduces resize messages that can be sent over QMP to
> > notify QEMU about the size of the terminal connented to some chardev.
> > In the libvirt setting, it will allow to implement a resizable terminal
> > for virsh console and other libvirt clients.
> > 
> > This patch series was originally authored by Szymon Lukasz and submitted
> > to qemu-devel about 5 years ago. The previous submission can be found at
> > <https://lists.nongnu.org/archive/html/qemu-devel/2020-06/msg09591.html>.
> > I have updated the patches to be compatible with latest master and made
> > a few small changes of my own, including the addition of Windows support.
> > 
> > Probably the most important change I made is the swapping of rows and
> > cols fields in resize messages. I would like to hear some feedback on
> > this change from reviewers. The problem is that for a long time, the
> > Linux kernel used a different field order from what was specified in the
> > virtio spec. The kernel implementation was apparently merged around 2010,
> > while the virtio spec came in 2014, so when a previous version of this
> > patch series was being discussed here on this mailing list in 2020, it
> > was decided that QEMU should match the Linux implementation, and ideally,
> > the virtio spec should be changed.
> > 
> > However, recently, the Linux kernel implementation was changed to conform
> > to the spec: <https://git.kernel.org/linus/5326ab737a47278dbd16ed3ee7380b26c7056ddd>.
> > As a result, to be compatible with latest kernel releases, QEMU needs to
> > also use the field order matching the specification. I have changed the
> > patch to use the spec-compliant order, so it works correctly with latest
> > kernels now.
> >
> 
> Well this is not in any release yet. If you want me to revert that
> one, let me know ASAP. Maximilian?
> 
> > That leaves the issue of older kernels. There are about 15 years' worth
> > of kernel versions with the swapped field order, including the kernel
> > currently shipped in Debian stable. The effects of the swapped dimensions
> > can sometimes be quite annoying - e.g. if you have a terminal with
> > 24 rows, this will be interpreted as 24 columns, and your shell may limit 
> > line editing to this small space, most of which will be taken by your
> > prompt. The patch series in its current form provides no way to disable
> > the console size functionality,
> 
> Well I see the console-size property, no?

At least in the case of libvirt managed VMs, this series does
nothin by default, as they won't be using the 'stdio' chardev,
they'll require libvirt to first wire up the new QMP command,
and then apps using libvirt to call it. So in that sense, it'll
take a while before the effects are seen by users.

> > so users may end up worse off than if
> > the functionality were not implemented at all.
> 
> If we want to keep the Linux patch, the straight forward way is to send
> the fix to stable@ then poke at Debian at al to fix their kernels.
> 
> Another option is to make the property default to off, have
> management turn it on when guest is up to date.
> 
> But it really sounds like we should revert that Linux patch.
> I posted a revert, pls comment.

What about other non-Linux guest OS that may have correctly
implemented the virtio spec ?

At least FreeBSD appears to /not/ implemenmt resize at all:

  https://github.com/freebsd/freebsd-src/blob/main/sys/dev/virtio/console/virtio_console.c#L884

Do we have a Windows impl of virtio-console with resize support ?

Any other places we should check ?

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

Re: [PATCH v4 00/10] virtio-console: notify about the terminal size

[Michael S. Tsirkin]

On Fri, Sep 12, 2025 at 09:50:30AM +0100, Daniel P. Berrangé wrote:
> On Fri, Sep 12, 2025 at 04:41:02AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Sep 12, 2025 at 05:39:45AM +0200, Filip Hejsek wrote:
> > > The goal of this series is to have a resizable terminal into a guest
> > > without having to set up networking and using, e.g. ssh.
> > > 
> > > The virtio spec allows a virtio-console device to notify the guest about
> > > terminal resizes in the host. Linux Kernel implements the driver part of
> > > the spec. This series implement the device part in QEMU.
> > > 
> > > This series adds support for a resizable terminal if a virtio console
> > > device is connected to the stdio backend.
> > > 
> > > This series also introduces resize messages that can be sent over QMP to
> > > notify QEMU about the size of the terminal connented to some chardev.
> > > In the libvirt setting, it will allow to implement a resizable terminal
> > > for virsh console and other libvirt clients.
> > > 
> > > This patch series was originally authored by Szymon Lukasz and submitted
> > > to qemu-devel about 5 years ago. The previous submission can be found at
> > > <https://lists.nongnu.org/archive/html/qemu-devel/2020-06/msg09591.html>.
> > > I have updated the patches to be compatible with latest master and made
> > > a few small changes of my own, including the addition of Windows support.
> > > 
> > > Probably the most important change I made is the swapping of rows and
> > > cols fields in resize messages. I would like to hear some feedback on
> > > this change from reviewers. The problem is that for a long time, the
> > > Linux kernel used a different field order from what was specified in the
> > > virtio spec. The kernel implementation was apparently merged around 2010,
> > > while the virtio spec came in 2014, so when a previous version of this
> > > patch series was being discussed here on this mailing list in 2020, it
> > > was decided that QEMU should match the Linux implementation, and ideally,
> > > the virtio spec should be changed.
> > > 
> > > However, recently, the Linux kernel implementation was changed to conform
> > > to the spec: <https://git.kernel.org/linus/5326ab737a47278dbd16ed3ee7380b26c7056ddd>.
> > > As a result, to be compatible with latest kernel releases, QEMU needs to
> > > also use the field order matching the specification. I have changed the
> > > patch to use the spec-compliant order, so it works correctly with latest
> > > kernels now.
> > >
> > 
> > Well this is not in any release yet. If you want me to revert that
> > one, let me know ASAP. Maximilian?
> > 
> > > That leaves the issue of older kernels. There are about 15 years' worth
> > > of kernel versions with the swapped field order, including the kernel
> > > currently shipped in Debian stable. The effects of the swapped dimensions
> > > can sometimes be quite annoying - e.g. if you have a terminal with
> > > 24 rows, this will be interpreted as 24 columns, and your shell may limit 
> > > line editing to this small space, most of which will be taken by your
> > > prompt. The patch series in its current form provides no way to disable
> > > the console size functionality,
> > 
> > Well I see the console-size property, no?
> 
> At least in the case of libvirt managed VMs, this series does
> nothin by default, as they won't be using the 'stdio' chardev,
> they'll require libvirt to first wire up the new QMP command,
> and then apps using libvirt to call it. So in that sense, it'll
> take a while before the effects are seen by users.
> 
> > > so users may end up worse off than if
> > > the functionality were not implemented at all.
> > 
> > If we want to keep the Linux patch, the straight forward way is to send
> > the fix to stable@ then poke at Debian at al to fix their kernels.
> > 
> > Another option is to make the property default to off, have
> > management turn it on when guest is up to date.
> > 
> > But it really sounds like we should revert that Linux patch.
> > I posted a revert, pls comment.
> 
> What about other non-Linux guest OS that may have correctly
> implemented the virtio spec ?
> 
> At least FreeBSD appears to /not/ implemenmt resize at all:
> 
>   https://github.com/freebsd/freebsd-src/blob/main/sys/dev/virtio/console/virtio_console.c#L884
> 
> Do we have a Windows impl of virtio-console with resize support ?

Windows seems to ignore it:

        case VIRTIO_CONSOLE_RESIZE:
            TraceEvents(TRACE_LEVEL_INFORMATION, DBG_PNP, "VIRTIO_CONSOLE_RESIZE id = %d\n", cpkt->id);
            break;



> Any other places we should check ?
> 
> 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 :|

Re: [PATCH v4 08/10] virtio-serial-bus: add terminal resize messages

[Markus Armbruster]

Filip Hejsek <filip.hejsek@gmail.com> writes:

> From: Szymon Lukasz <noh4hss@gmail.com>
>
> Implement the part of the virtio spec that allows to notify the virtio
> driver about terminal resizes. The virtio spec contains two methods to
> achieve that:
>
> For legacy drivers, we have only one port and we put the terminal size
> in the config space and inject the config changed interrupt.
>
> For multiport devices, we use the control virtqueue to send a packet
> containing the terminal size. Note that old versions of the Linux kernel
> used an incorrect order for the fields (rows then cols instead of cols
> then rows), until it was fixed by commit 5326ab737a47278dbd16ed3ee7380b26c7056ddd.
>
> As a result, when using a Linux kernel older than 6.15, the number of rows
> and columns will be swapped.

Should this note be added to user documentation?

> Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>

Re: [PATCH v4 07/10] qmp: add chardev-resize command

[Markus Armbruster]

Cc: libvirt

Filip Hejsek <filip.hejsek@gmail.com> writes:

> From: Szymon Lukasz <noh4hss@gmail.com>
>
> The managment software can use this command to notify QEMU about the
> size of the terminal connected to a chardev, QEMU can then forward this
> information to the guest if the chardev is connected to a virtio console
> device.
>
> Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> ---
>  chardev/char.c | 14 ++++++++++++++
>  qapi/char.json | 22 ++++++++++++++++++++++
>  2 files changed, 36 insertions(+)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index b45d79cb9b57643827eb7479257fdda2cf6b0434..6e3ade98614c949be8041ec5905a490ff536dee9 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -1269,6 +1269,20 @@ bool qmp_add_client_char(int fd, bool has_skipauth, bool skipauth,
>      return true;
>  }
>  
> +void qmp_chardev_resize(const char *id, uint16_t cols, uint16_t rows,
> +                        Error **errp)
> +{
> +    Chardev *chr;
> +
> +    chr = qemu_chr_find(id);
> +    if (chr == NULL) {
> +        error_setg(errp, "Chardev '%s' not found", id);
> +        return;
> +    }
> +
> +    qemu_chr_resize(chr, cols, rows);
> +}
> +
>  /*
>   * Add a timeout callback for the chardev (in milliseconds), return
>   * the GSource object created. Please use this to add timeout hook for
> diff --git a/qapi/char.json b/qapi/char.json
> index f0a53f742c8bee24c377551803a864fd36ac78cf..0a26c5eee6b71bc5de127a91b253cc69a9fe8ce6 100644
> --- a/qapi/char.json
> +++ b/qapi/char.json
> @@ -874,6 +874,28 @@
>  { 'command': 'chardev-send-break',
>    'data': { 'id': 'str' } }
>  
> +##
> +# @chardev-resize:

This name doesn't tell me what is being resized.  PATCH 04 uses
"winsize", which is better.  The (losely) related SIGWINCH suggests
"window change" or "window size change".  Below, you use "terminal
size".

> +#
> +# Notifies a chardev about the current size of the terminal connected
> +# to this chardev.

Yes, but what is it good for?  Your commit message tells: "managment
software can use this command to notify QEMU about the size of the
terminal connected to a chardev, QEMU can then forward this information
to the guest if the chardev is connected to a virtio console device."

> +#
> +# @id: the chardev's ID, must exist
> +# @cols: the number of columns
> +# @rows: the number of rows

Blank lines between the argument descriptions, bease.

What's the initial size?

Do we need a way to query the size?

> +#
> +# Since: 10.2
> +#
> +# .. qmp-example::
> +#
> +#     -> { "execute": "chardev-resize", "arguments": { "id": "foo", "cols": 80, "rows": 24 } }
> +#     <- { "return": {} }
> +##
> +{ 'command': 'chardev-resize',
> +  'data': { 'id': 'str',
> +            'cols': 'uint16',
> +            'rows': 'uint16' } }
> +
>  ##
>  # @VSERPORT_CHANGE:
>  #

Re: [PATCH v4 00/10] virtio-console: notify about the terminal size

[Filip Hejsek]

On Fri, 2025-09-12 at 04:41 -0400, Michael S. Tsirkin wrote:

> > The patch series in its current form provides no way to disable
> > the console size functionality,
> 
> Well I see the console-size property, no?

That only disables the VIRTIO_CONSOLE_F_SIZE feature flag, but
VIRTIO_CONSOLE_RESIZE messages will still be sent to multiport devices.

I could of course change it to dislable those messages too. Shall I do
so?

Re: [PATCH v4 08/10] virtio-serial-bus: add terminal resize messages

[Filip Hejsek]

On Fri, 2025-09-12 at 15:50 +0200, Markus Armbruster wrote:
> > As a result, when using a Linux kernel older than 6.15, the number of rows
> > and columns will be swapped.
> 
> Should this note be added to user documentation?

Agreed, but I want first to reach a decision about how to deal with
compatibility with older kernels (e.g. option to disable the feature,
option to swap the fields, etc), plus the Linux patch may end up being
reverted so I want to wait until that is decided.

Re: [PATCH v4 07/10] qmp: add chardev-resize command

[Filip Hejsek]

On Fri, 2025-09-12 at 16:01 +0200, Markus Armbruster wrote:
> Cc: libvirt
> 
> Filip Hejsek <filip.hejsek@gmail.com> writes:
> 
> > From: Szymon Lukasz <noh4hss@gmail.com>
> > 
> > The managment software can use this command to notify QEMU about the
> > size of the terminal connected to a chardev, QEMU can then forward this
> > information to the guest if the chardev is connected to a virtio console
> > device.
> > 
> > Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
> > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> > ---
> >  chardev/char.c | 14 ++++++++++++++
> >  qapi/char.json | 22 ++++++++++++++++++++++
> >  2 files changed, 36 insertions(+)
> > 
> > diff --git a/chardev/char.c b/chardev/char.c
> > index b45d79cb9b57643827eb7479257fdda2cf6b0434..6e3ade98614c949be8041ec5905a490ff536dee9 100644
> > --- a/chardev/char.c
> > +++ b/chardev/char.c
> > @@ -1269,6 +1269,20 @@ bool qmp_add_client_char(int fd, bool has_skipauth, bool skipauth,
> >      return true;
> >  }
> >  
> > +void qmp_chardev_resize(const char *id, uint16_t cols, uint16_t rows,
> > +                        Error **errp)
> > +{
> > +    Chardev *chr;
> > +
> > +    chr = qemu_chr_find(id);
> > +    if (chr == NULL) {
> > +        error_setg(errp, "Chardev '%s' not found", id);
> > +        return;
> > +    }
> > +
> > +    qemu_chr_resize(chr, cols, rows);
> > +}
> > +
> >  /*
> >   * Add a timeout callback for the chardev (in milliseconds), return
> >   * the GSource object created. Please use this to add timeout hook for
> > diff --git a/qapi/char.json b/qapi/char.json
> > index f0a53f742c8bee24c377551803a864fd36ac78cf..0a26c5eee6b71bc5de127a91b253cc69a9fe8ce6 100644
> > --- a/qapi/char.json
> > +++ b/qapi/char.json
> > @@ -874,6 +874,28 @@
> >  { 'command': 'chardev-send-break',
> >    'data': { 'id': 'str' } }
> >  
> > +##
> > +# @chardev-resize:
> 
> This name doesn't tell me what is being resized.  PATCH 04 uses
> "winsize", which is better.  The (losely) related SIGWINCH suggests
> "window change" or "window size change".  Below, you use "terminal
> size".

How about chardev-console-resize? That would match the name of the
virtio event (VIRTIO_CONSOLE_RESIZE).

> > +#
> > +# Notifies a chardev about the current size of the terminal connected
> > +# to this chardev.
> 
> Yes, but what is it good for?  Your commit message tells: "managment
> software can use this command to notify QEMU about the size of the
> terminal connected to a chardev, QEMU can then forward this information
> to the guest if the chardev is connected to a virtio console device."

How about:

   Notifies a chardev about the current size of the terminal connected
   to this chardev. The information will be forwarded to the guest if
   the chardev is connected to a virtio console device.

> > +#
> > +# @id: the chardev's ID, must exist
> > +# @cols: the number of columns
> > +# @rows: the number of rows
> 
> Blank lines between the argument descriptions, bease.
> 
> What's the initial size?

0x0

> Do we need a way to query the size?

I don't think it is necessary. What would be the usecase for that?

> > +#
> > +# Since: 10.2
> > +#
> > +# .. qmp-example::
> > +#
> > +#     -> { "execute": "chardev-resize", "arguments": { "id": "foo", "cols": 80, "rows": 24 } }
> > +#     <- { "return": {} }
> > +##
> > +{ 'command': 'chardev-resize',
> > +  'data': { 'id': 'str',
> > +            'cols': 'uint16',
> > +            'rows': 'uint16' } }
> > +
> >  ##
> >  # @VSERPORT_CHANGE:
> >  #

Re: [PATCH v4 07/10] qmp: add chardev-resize command

[Markus Armbruster]

Filip Hejsek <filip.hejsek@gmail.com> writes:

> On Fri, 2025-09-12 at 16:01 +0200, Markus Armbruster wrote:
>> Cc: libvirt
>> 
>> Filip Hejsek <filip.hejsek@gmail.com> writes:
>> 
>> > From: Szymon Lukasz <noh4hss@gmail.com>
>> > 
>> > The managment software can use this command to notify QEMU about the
>> > size of the terminal connected to a chardev, QEMU can then forward this
>> > information to the guest if the chardev is connected to a virtio console
>> > device.
>> > 
>> > Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
>> > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
>> > Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
>> > ---
>> >  chardev/char.c | 14 ++++++++++++++
>> >  qapi/char.json | 22 ++++++++++++++++++++++
>> >  2 files changed, 36 insertions(+)
>> > 
>> > diff --git a/chardev/char.c b/chardev/char.c
>> > index b45d79cb9b57643827eb7479257fdda2cf6b0434..6e3ade98614c949be8041ec5905a490ff536dee9 100644
>> > --- a/chardev/char.c
>> > +++ b/chardev/char.c
>> > @@ -1269,6 +1269,20 @@ bool qmp_add_client_char(int fd, bool has_skipauth, bool skipauth,
>> >      return true;
>> >  }
>> >  
>> > +void qmp_chardev_resize(const char *id, uint16_t cols, uint16_t rows,
>> > +                        Error **errp)
>> > +{
>> > +    Chardev *chr;
>> > +
>> > +    chr = qemu_chr_find(id);
>> > +    if (chr == NULL) {
>> > +        error_setg(errp, "Chardev '%s' not found", id);
>> > +        return;
>> > +    }
>> > +
>> > +    qemu_chr_resize(chr, cols, rows);
>> > +}
>> > +
>> >  /*
>> >   * Add a timeout callback for the chardev (in milliseconds), return
>> >   * the GSource object created. Please use this to add timeout hook for
>> > diff --git a/qapi/char.json b/qapi/char.json
>> > index f0a53f742c8bee24c377551803a864fd36ac78cf..0a26c5eee6b71bc5de127a91b253cc69a9fe8ce6 100644
>> > --- a/qapi/char.json
>> > +++ b/qapi/char.json
>> > @@ -874,6 +874,28 @@
>> >  { 'command': 'chardev-send-break',
>> >    'data': { 'id': 'str' } }
>> >  
>> > +##
>> > +# @chardev-resize:
>> 
>> This name doesn't tell me what is being resized.  PATCH 04 uses
>> "winsize", which is better.  The (losely) related SIGWINCH suggests
>> "window change" or "window size change".  Below, you use "terminal
>> size".
>
> How about chardev-console-resize? That would match the name of the
> virtio event (VIRTIO_CONSOLE_RESIZE).

Not bad.  It could become slightly bad if we make devices other than
"consoles" make us of it.  Would that be possible?

>> > +#
>> > +# Notifies a chardev about the current size of the terminal connected
>> > +# to this chardev.
>> 
>> Yes, but what is it good for?  Your commit message tells: "managment
>> software can use this command to notify QEMU about the size of the
>> terminal connected to a chardev, QEMU can then forward this information
>> to the guest if the chardev is connected to a virtio console device."
>
> How about:
>
>    Notifies a chardev about the current size of the terminal connected
>    to this chardev. The information will be forwarded to the guest if
>    the chardev is connected to a virtio console device.

Works for me.

>> > +#
>> > +# @id: the chardev's ID, must exist
>> > +# @cols: the number of columns
>> > +# @rows: the number of rows
>> 
>> Blank lines between the argument descriptions, bease.
>> 
>> What's the initial size?
>
> 0x0

A clearly invalid size.  I guess it effectively means "unknown size".
Should we document that?

>> Do we need a way to query the size?
>
> I don't think it is necessary. What would be the usecase for that?

I don't know, but it's my standard question when I see an interface to
set something without an interface to get it.  Its purpose is to make us
think, not to make us at the get blindly.

>> > +#
>> > +# Since: 10.2
>> > +#
>> > +# .. qmp-example::
>> > +#
>> > +#     -> { "execute": "chardev-resize", "arguments": { "id": "foo", "cols": 80, "rows": 24 } }
>> > +#     <- { "return": {} }
>> > +##
>> > +{ 'command': 'chardev-resize',
>> > +  'data': { 'id': 'str',
>> > +            'cols': 'uint16',
>> > +            'rows': 'uint16' } }
>> > +
>> >  ##
>> >  # @VSERPORT_CHANGE:
>> >  #

Re: [PATCH v4 00/10] virtio-console: notify about the terminal size

[Maximilian Immanuel Brandtner]

On Fri, 2025-09-12 at 09:50 +0100, Daniel P. Berrangé wrote:
> On Fri, Sep 12, 2025 at 04:41:02AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Sep 12, 2025 at 05:39:45AM +0200, Filip Hejsek wrote:
> > > The goal of this series is to have a resizable terminal into a
> > > guest
> > > without having to set up networking and using, e.g. ssh.
> > > 
> > > The virtio spec allows a virtio-console device to notify the
> > > guest about
> > > terminal resizes in the host. Linux Kernel implements the driver
> > > part of
> > > the spec. This series implement the device part in QEMU.
> > > 
> > > This series adds support for a resizable terminal if a virtio
> > > console
> > > device is connected to the stdio backend.
> > > 
> > > This series also introduces resize messages that can be sent over
> > > QMP to
> > > notify QEMU about the size of the terminal connented to some
> > > chardev.
> > > In the libvirt setting, it will allow to implement a resizable
> > > terminal
> > > for virsh console and other libvirt clients.
> > > 
> > > This patch series was originally authored by Szymon Lukasz and
> > > submitted
> > > to qemu-devel about 5 years ago. The previous submission can be
> > > found at
> > > <
> > > https://lists.nongnu.org/archive/html/qemu-devel/2020-06/msg09591.
> > > html>.
> > > I have updated the patches to be compatible with latest master
> > > and made
> > > a few small changes of my own, including the addition of Windows
> > > support.
> > > 
> > > Probably the most important change I made is the swapping of rows
> > > and
> > > cols fields in resize messages. I would like to hear some
> > > feedback on
> > > this change from reviewers. The problem is that for a long time,
> > > the
> > > Linux kernel used a different field order from what was specified
> > > in the
> > > virtio spec. The kernel implementation was apparently merged
> > > around 2010,
> > > while the virtio spec came in 2014, so when a previous version of
> > > this
> > > patch series was being discussed here on this mailing list in
> > > 2020, it
> > > was decided that QEMU should match the Linux implementation, and
> > > ideally,
> > > the virtio spec should be changed.
> > > 
> > > However, recently, the Linux kernel implementation was changed to
> > > conform
> > > to the spec:
> > > <https://git.kernel.org/linus/5326ab737a47278dbd16ed3ee7380b26c70
> > > 56ddd>.
> > > As a result, to be compatible with latest kernel releases, QEMU
> > > needs to
> > > also use the field order matching the specification. I have
> > > changed the
> > > patch to use the spec-compliant order, so it works correctly with
> > > latest
> > > kernels now.
> > > 
> > 
> > Well this is not in any release yet. If you want me to revert that
> > one, let me know ASAP. Maximilian?
> > 
> > > That leaves the issue of older kernels. There are about 15 years'
> > > worth
> > > of kernel versions with the swapped field order, including the
> > > kernel
> > > currently shipped in Debian stable. The effects of the swapped
> > > dimensions
> > > can sometimes be quite annoying - e.g. if you have a terminal
> > > with
> > > 24 rows, this will be interpreted as 24 columns, and your shell
> > > may limit 
> > > line editing to this small space, most of which will be taken by
> > > your
> > > prompt. The patch series in its current form provides no way to
> > > disable
> > > the console size functionality,
> > 
> > Well I see the console-size property, no?
> 
> At least in the case of libvirt managed VMs, this series does
> nothin by default, as they won't be using the 'stdio' chardev,
> they'll require libvirt to first wire up the new QMP command,
> and then apps using libvirt to call it. So in that sense, it'll
> take a while before the effects are seen by users.

Correct me if I'm wrong on this, but shouldn't the 'pty' chardev also
be able to take advantage of the same size change mechanisms as the
'stdio' chardev (receiving SIGWINCH and being able to use the ioctl
TIOCGWINSZ)? If so the work for the 'stdio' chardev should probably be
replicated for the 'pty' chardev. 

> 
> > > so users may end up worse off than if
> > > the functionality were not implemented at all.
> > 
> > If we want to keep the Linux patch, the straight forward way is to
> > send
> > the fix to stable@ then poke at Debian at al to fix their kernels.
> > 
> > Another option is to make the property default to off, have
> > management turn it on when guest is up to date.
> > 
> > But it really sounds like we should revert that Linux patch.
> > I posted a revert, pls comment.
> 
> What about other non-Linux guest OS that may have correctly
> implemented the virtio spec ?
> 
> At least FreeBSD appears to /not/ implemenmt resize at all:
> 
>  
> https://github.com/freebsd/freebsd-src/blob/main/sys/dev/virtio/console/virtio_console.c#L884
> 
> Do we have a Windows impl of virtio-console with resize support ?
> 
> Any other places we should check ?
> 
> With regards,
> Daniel

Re: [PATCH v4 00/10] virtio-console: notify about the terminal size

[Daniel P. Berrangé]

On Mon, Sep 15, 2025 at 10:41:44AM +0200, Maximilian Immanuel Brandtner wrote:
> On Fri, 2025-09-12 at 09:50 +0100, Daniel P. Berrangé wrote:
> > On Fri, Sep 12, 2025 at 04:41:02AM -0400, Michael S. Tsirkin wrote:
> > > On Fri, Sep 12, 2025 at 05:39:45AM +0200, Filip Hejsek wrote:
> > > > The goal of this series is to have a resizable terminal into a
> > > > guest
> > > > without having to set up networking and using, e.g. ssh.
> > > > 
> > > > The virtio spec allows a virtio-console device to notify the
> > > > guest about
> > > > terminal resizes in the host. Linux Kernel implements the driver
> > > > part of
> > > > the spec. This series implement the device part in QEMU.
> > > > 
> > > > This series adds support for a resizable terminal if a virtio
> > > > console
> > > > device is connected to the stdio backend.
> > > > 
> > > > This series also introduces resize messages that can be sent over
> > > > QMP to
> > > > notify QEMU about the size of the terminal connented to some
> > > > chardev.
> > > > In the libvirt setting, it will allow to implement a resizable
> > > > terminal
> > > > for virsh console and other libvirt clients.
> > > > 
> > > > This patch series was originally authored by Szymon Lukasz and
> > > > submitted
> > > > to qemu-devel about 5 years ago. The previous submission can be
> > > > found at
> > > > <
> > > > https://lists.nongnu.org/archive/html/qemu-devel/2020-06/msg09591.
> > > > html>.
> > > > I have updated the patches to be compatible with latest master
> > > > and made
> > > > a few small changes of my own, including the addition of Windows
> > > > support.
> > > > 
> > > > Probably the most important change I made is the swapping of rows
> > > > and
> > > > cols fields in resize messages. I would like to hear some
> > > > feedback on
> > > > this change from reviewers. The problem is that for a long time,
> > > > the
> > > > Linux kernel used a different field order from what was specified
> > > > in the
> > > > virtio spec. The kernel implementation was apparently merged
> > > > around 2010,
> > > > while the virtio spec came in 2014, so when a previous version of
> > > > this
> > > > patch series was being discussed here on this mailing list in
> > > > 2020, it
> > > > was decided that QEMU should match the Linux implementation, and
> > > > ideally,
> > > > the virtio spec should be changed.
> > > > 
> > > > However, recently, the Linux kernel implementation was changed to
> > > > conform
> > > > to the spec:
> > > > <https://git.kernel.org/linus/5326ab737a47278dbd16ed3ee7380b26c70
> > > > 56ddd>.
> > > > As a result, to be compatible with latest kernel releases, QEMU
> > > > needs to
> > > > also use the field order matching the specification. I have
> > > > changed the
> > > > patch to use the spec-compliant order, so it works correctly with
> > > > latest
> > > > kernels now.
> > > > 
> > > 
> > > Well this is not in any release yet. If you want me to revert that
> > > one, let me know ASAP. Maximilian?
> > > 
> > > > That leaves the issue of older kernels. There are about 15 years'
> > > > worth
> > > > of kernel versions with the swapped field order, including the
> > > > kernel
> > > > currently shipped in Debian stable. The effects of the swapped
> > > > dimensions
> > > > can sometimes be quite annoying - e.g. if you have a terminal
> > > > with
> > > > 24 rows, this will be interpreted as 24 columns, and your shell
> > > > may limit 
> > > > line editing to this small space, most of which will be taken by
> > > > your
> > > > prompt. The patch series in its current form provides no way to
> > > > disable
> > > > the console size functionality,
> > > 
> > > Well I see the console-size property, no?
> > 
> > At least in the case of libvirt managed VMs, this series does
> > nothin by default, as they won't be using the 'stdio' chardev,
> > they'll require libvirt to first wire up the new QMP command,
> > and then apps using libvirt to call it. So in that sense, it'll
> > take a while before the effects are seen by users.
> 
> Correct me if I'm wrong on this, but shouldn't the 'pty' chardev also
> be able to take advantage of the same size change mechanisms as the
> 'stdio' chardev (receiving SIGWINCH and being able to use the ioctl
> TIOCGWINSZ)? If so the work for the 'stdio' chardev should probably be
> replicated for the 'pty' chardev.

Yes, if using a suitable client app, it ought to work for
'pty' I think.


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

[PATCH] char-pty: add support for the terminal size

[Maximilian Immanuel Brandtner]

Update the terminal size upon SIGWINCH delivery.

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

Signed-off-by: Maximilian Immanuel Brandtner <maxbr@linux.ibm.com>
---
 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

[PATCH v2] char-pty: add support for the terminal size

[Maximilian Immanuel Brandtner]

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

Re: [PATCH v2] char-pty: add support for the terminal size

[Michael S. Tsirkin]

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

Re: [PATCH v2] char-pty: add support for the terminal size

[Filip Hejsek]

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.

Re: [PATCH v4 07/10] qmp: add chardev-resize command

[Filip Hejsek]

On Mon, 2025-09-15 at 08:35 +0200, Markus Armbruster wrote:
> Filip Hejsek <filip.hejsek@gmail.com> writes:
> 
> > On Fri, 2025-09-12 at 16:01 +0200, Markus Armbruster wrote:
> > > Cc: libvirt
> > > 
> > > Filip Hejsek <filip.hejsek@gmail.com> writes:
> > > 
> > > > From: Szymon Lukasz <noh4hss@gmail.com>
> > > > 
> > > > The managment software can use this command to notify QEMU about the
> > > > size of the terminal connected to a chardev, QEMU can then forward this
> > > > information to the guest if the chardev is connected to a virtio console
> > > > device.
> > > > 
> > > > Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
> > > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > > > Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> > > > ---
> > > >  chardev/char.c | 14 ++++++++++++++
> > > >  qapi/char.json | 22 ++++++++++++++++++++++
> > > >  2 files changed, 36 insertions(+)
> > > > 
> > > > diff --git a/chardev/char.c b/chardev/char.c
> > > > index b45d79cb9b57643827eb7479257fdda2cf6b0434..6e3ade98614c949be8041ec5905a490ff536dee9 100644
> > > > --- a/chardev/char.c
> > > > +++ b/chardev/char.c
> > > > @@ -1269,6 +1269,20 @@ bool qmp_add_client_char(int fd, bool has_skipauth, bool skipauth,
> > > >      return true;
> > > >  }
> > > >  
> > > > +void qmp_chardev_resize(const char *id, uint16_t cols, uint16_t rows,
> > > > +                        Error **errp)
> > > > +{
> > > > +    Chardev *chr;
> > > > +
> > > > +    chr = qemu_chr_find(id);
> > > > +    if (chr == NULL) {
> > > > +        error_setg(errp, "Chardev '%s' not found", id);
> > > > +        return;
> > > > +    }
> > > > +
> > > > +    qemu_chr_resize(chr, cols, rows);
> > > > +}
> > > > +
> > > >  /*
> > > >   * Add a timeout callback for the chardev (in milliseconds), return
> > > >   * the GSource object created. Please use this to add timeout hook for
> > > > diff --git a/qapi/char.json b/qapi/char.json
> > > > index f0a53f742c8bee24c377551803a864fd36ac78cf..0a26c5eee6b71bc5de127a91b253cc69a9fe8ce6 100644
> > > > --- a/qapi/char.json
> > > > +++ b/qapi/char.json
> > > > @@ -874,6 +874,28 @@
> > > >  { 'command': 'chardev-send-break',
> > > >    'data': { 'id': 'str' } }
> > > >  
> > > > +##
> > > > +# @chardev-resize:
> > > 
> > > This name doesn't tell me what is being resized.  PATCH 04 uses
> > > "winsize", which is better.  The (losely) related SIGWINCH suggests
> > > "window change" or "window size change".  Below, you use "terminal
> > > size".
> > 
> > How about chardev-console-resize? That would match the name of the
> > virtio event (VIRTIO_CONSOLE_RESIZE).
> 
> Not bad.  It could become slightly bad if we make devices other than
> "consoles" make us of it.  Would that be possible?

I don't think the size has any meaning for devices that are not
connected to a console, although the code does not care whether it
actually is a console and simply has a size for every chardev.

I guess I could also rename it to chardev-window-resize
or chardev-set-window-size. Let me know if you prefer one of these.


> > > > +#
> > > > +# Notifies a chardev about the current size of the terminal connected
> > > > +# to this chardev.
> > > 
> > > Yes, but what is it good for?  Your commit message tells: "managment
> > > software can use this command to notify QEMU about the size of the
> > > terminal connected to a chardev, QEMU can then forward this information
> > > to the guest if the chardev is connected to a virtio console device."
> > 
> > How about:
> > 
> >    Notifies a chardev about the current size of the terminal connected
> >    to this chardev. The information will be forwarded to the guest if
> >    the chardev is connected to a virtio console device.
> 
> Works for me.
> 
> > > > +#
> > > > +# @id: the chardev's ID, must exist
> > > > +# @cols: the number of columns
> > > > +# @rows: the number of rows
> > > 
> > > Blank lines between the argument descriptions, bease.
> > > 
> > > What's the initial size?
> > 
> > 0x0
> 
> A clearly invalid size.  I guess it effectively means "unknown size".
> Should we document that?

Probably. 0x0 is I think also the default size in the Linux kernel, but
I don't think the Linux kernel documents this. Another question is if
the 0x0 size should be propagated to the guest over virtio. I think it
should be, although the virtio spec says nothing about 0x0 size.

I'm not sure what is the right place to document this.

> > > Do we need a way to query the size?
> > 
> > I don't think it is necessary. What would be the usecase for that?
> 
> I don't know, but it's my standard question when I see an interface to
> set something without an interface to get it.  Its purpose is to make us
> think, not to make us at the get blindly.

I guess it might be useful for debugging. If the size is not propagated
correctly, one might query it to find out on which side the problem is.

> > > > +#
> > > > +# Since: 10.2
> > > > +#
> > > > +# .. qmp-example::
> > > > +#
> > > > +#     -> { "execute": "chardev-resize", "arguments": { "id": "foo", "cols": 80, "rows": 24 } }
> > > > +#     <- { "return": {} }
> > > > +##
> > > > +{ 'command': 'chardev-resize',
> > > > +  'data': { 'id': 'str',
> > > > +            'cols': 'uint16',
> > > > +            'rows': 'uint16' } }
> > > > +
> > > >  ##
> > > >  # @VSERPORT_CHANGE:
> > > >  #

Re: [PATCH v4 00/10] virtio-console: notify about the terminal size

[Filip Hejsek]

While thinking about the patches, a few questions about the virtio spec
have popped into my head.

1. Should the config space size also be updated (for port 0) when
multiport is used? Based on my reading of the spec, I think yes.

2. Can VIRTIO_CONSOLE_RESIZE be sent if VIRTIO_CONSOLE_F_SIZE is not
negotiated? The spec does not say, which I think means it can.

3. The spec says that reading from config space fields that are
conditional on features should be allowed even if the driver has not
(yet) accepted the feature. Does it mean that we have to update the
size even if the feature is not accepted (yet), or is it OK if the
reads return 0?

Thanks for any answers or opinions,
Filip Hejsek

Re: [PATCH v4 00/10] virtio-console: notify about the terminal size

[Michael S. Tsirkin]

On Tue, Sep 16, 2025 at 01:02:02AM +0200, Filip Hejsek wrote:
> While thinking about the patches, a few questions about the virtio spec
> have popped into my head.
> 
> 1. Should the config space size also be updated (for port 0) when
> multiport is used? Based on my reading of the spec, I think yes.
> 
> 2. Can VIRTIO_CONSOLE_RESIZE be sent if VIRTIO_CONSOLE_F_SIZE is not
> negotiated? The spec does not say, which I think means it can.

But the guest can't do anything useful here.

> 3. The spec says that reading from config space fields that are
> conditional on features should be allowed even if the driver has not
> (yet) accepted the feature. Does it mean that we have to update the
> size even if the feature is not accepted (yet), or is it OK if the
> reads return 0?

This is talking about the window before FEATURES_OK (and so DRIVER_OK)
is set.  It is best to update the size. There's no interrupt to send
though.


> Thanks for any answers or opinions,
> Filip Hejsek

Re: [PATCH v4 07/10] qmp: add chardev-resize command

[Markus Armbruster]

Filip Hejsek <filip.hejsek@gmail.com> writes:

> On Mon, 2025-09-15 at 08:35 +0200, Markus Armbruster wrote:
>> Filip Hejsek <filip.hejsek@gmail.com> writes:
>> 
>> > On Fri, 2025-09-12 at 16:01 +0200, Markus Armbruster wrote:
>> > > Cc: libvirt
>> > > 
>> > > Filip Hejsek <filip.hejsek@gmail.com> writes:
>> > > 
>> > > > From: Szymon Lukasz <noh4hss@gmail.com>
>> > > > 
>> > > > The managment software can use this command to notify QEMU about the
>> > > > size of the terminal connected to a chardev, QEMU can then forward this
>> > > > information to the guest if the chardev is connected to a virtio console
>> > > > device.
>> > > > 
>> > > > Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
>> > > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
>> > > > Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
>> > > > ---
>> > > >  chardev/char.c | 14 ++++++++++++++
>> > > >  qapi/char.json | 22 ++++++++++++++++++++++
>> > > >  2 files changed, 36 insertions(+)
>> > > > 
>> > > > diff --git a/chardev/char.c b/chardev/char.c
>> > > > index b45d79cb9b57643827eb7479257fdda2cf6b0434..6e3ade98614c949be8041ec5905a490ff536dee9 100644
>> > > > --- a/chardev/char.c
>> > > > +++ b/chardev/char.c
>> > > > @@ -1269,6 +1269,20 @@ bool qmp_add_client_char(int fd, bool has_skipauth, bool skipauth,
>> > > >      return true;
>> > > >  }
>> > > >  
>> > > > +void qmp_chardev_resize(const char *id, uint16_t cols, uint16_t rows,
>> > > > +                        Error **errp)
>> > > > +{
>> > > > +    Chardev *chr;
>> > > > +
>> > > > +    chr = qemu_chr_find(id);
>> > > > +    if (chr == NULL) {
>> > > > +        error_setg(errp, "Chardev '%s' not found", id);
>> > > > +        return;
>> > > > +    }
>> > > > +
>> > > > +    qemu_chr_resize(chr, cols, rows);
>> > > > +}
>> > > > +
>> > > >  /*
>> > > >   * Add a timeout callback for the chardev (in milliseconds), return
>> > > >   * the GSource object created. Please use this to add timeout hook for
>> > > > diff --git a/qapi/char.json b/qapi/char.json
>> > > > index f0a53f742c8bee24c377551803a864fd36ac78cf..0a26c5eee6b71bc5de127a91b253cc69a9fe8ce6 100644
>> > > > --- a/qapi/char.json
>> > > > +++ b/qapi/char.json
>> > > > @@ -874,6 +874,28 @@
>> > > >  { 'command': 'chardev-send-break',
>> > > >    'data': { 'id': 'str' } }
>> > > >  
>> > > > +##
>> > > > +# @chardev-resize:
>> > > 
>> > > This name doesn't tell me what is being resized.  PATCH 04 uses
>> > > "winsize", which is better.  The (losely) related SIGWINCH suggests
>> > > "window change" or "window size change".  Below, you use "terminal
>> > > size".
>> > 
>> > How about chardev-console-resize? That would match the name of the
>> > virtio event (VIRTIO_CONSOLE_RESIZE).
>> 
>> Not bad.  It could become slightly bad if we make devices other than
>> "consoles" make us of it.  Would that be possible?
>
> I don't think the size has any meaning for devices that are not
> connected to a console, although the code does not care whether it
> actually is a console and simply has a size for every chardev.

Double-checking: the command works for any ChardevBackendKind, doesn't
it?

> I guess I could also rename it to chardev-window-resize
> or chardev-set-window-size. Let me know if you prefer one of these.

I think I'd prefer "window" or "terminal".

"resize" and "set size" suggest that the command initiates a size
change.  Not true, it notifies of a size change.  Maybe
"chardev-window-size-changed", "chardev-terminal-size-changed",
"chardev-window-resized", or "chardev-terminal-resized".

>> > > > +#
>> > > > +# Notifies a chardev about the current size of the terminal connected
>> > > > +# to this chardev.
>> > > 
>> > > Yes, but what is it good for?  Your commit message tells: "managment
>> > > software can use this command to notify QEMU about the size of the
>> > > terminal connected to a chardev, QEMU can then forward this information
>> > > to the guest if the chardev is connected to a virtio console device."
>> > 
>> > How about:
>> > 
>> >    Notifies a chardev about the current size of the terminal connected
>> >    to this chardev. The information will be forwarded to the guest if
>> >    the chardev is connected to a virtio console device.
>> 
>> Works for me.
>> 
>> > > > +#
>> > > > +# @id: the chardev's ID, must exist
>> > > > +# @cols: the number of columns
>> > > > +# @rows: the number of rows
>> > > 
>> > > Blank lines between the argument descriptions, bease.
>> > > 
>> > > What's the initial size?
>> > 
>> > 0x0

Another question...  'vc' chardevs accept optional @rows, @cols (see
ChardevVC).  Is this the same size or something else?

>> A clearly invalid size.  I guess it effectively means "unknown size".
>> Should we document that?
>
> Probably. 0x0 is I think also the default size in the Linux kernel, but
> I don't think the Linux kernel documents this.

How does 0 x 0 behave compared to a valid size like 80 x 24?

>                                                Another question is if
> the 0x0 size should be propagated to the guest over virtio. I think it
> should be, although the virtio spec says nothing about 0x0 size.
>
> I'm not sure what is the right place to document this.

I think the QAPI schema doc comment is as good a place as any.

>> > > Do we need a way to query the size?
>> > 
>> > I don't think it is necessary. What would be the usecase for that?
>> 
>> I don't know, but it's my standard question when I see an interface to
>> set something without an interface to get it.  Its purpose is to make us
>> think, not to make us at the get blindly.
>
> I guess it might be useful for debugging. If the size is not propagated
> correctly, one might query it to find out on which side the problem is.

We have query-chardev.  It doesn't return much.

>> > > > +#
>> > > > +# Since: 10.2
>> > > > +#
>> > > > +# .. qmp-example::
>> > > > +#
>> > > > +#     -> { "execute": "chardev-resize", "arguments": { "id": "foo", "cols": 80, "rows": 24 } }
>> > > > +#     <- { "return": {} }
>> > > > +##
>> > > > +{ 'command': 'chardev-resize',
>> > > > +  'data': { 'id': 'str',
>> > > > +            'cols': 'uint16',
>> > > > +            'rows': 'uint16' } }
>> > > > +
>> > > >  ##
>> > > >  # @VSERPORT_CHANGE:
>> > > >  #

Re: [PATCH v4 07/10] qmp: add chardev-resize command

[Filip Hejsek]

On Tue, 2025-09-16 at 15:07 +0200, Markus Armbruster wrote:
> Filip Hejsek <filip.hejsek@gmail.com> writes:
> 
> > On Mon, 2025-09-15 at 08:35 +0200, Markus Armbruster wrote:
> > > Filip Hejsek <filip.hejsek@gmail.com> writes:
> > > 
> > > > On Fri, 2025-09-12 at 16:01 +0200, Markus Armbruster wrote:
> > > > > Cc: libvirt
> > > > > 
> > > > > Filip Hejsek <filip.hejsek@gmail.com> writes:
> > > > > 
> > > > > > From: Szymon Lukasz <noh4hss@gmail.com>
> > > > > > 
> > > > > > [...]
> > > > > >  
> > > > > > +##
> > > > > > +# @chardev-resize:
> > > > > 
> > > > > This name doesn't tell me what is being resized.  PATCH 04 uses
> > > > > "winsize", which is better.  The (losely) related SIGWINCH suggests
> > > > > "window change" or "window size change".  Below, you use "terminal
> > > > > size".
> > > > 
> > > > How about chardev-console-resize? That would match the name of the
> > > > virtio event (VIRTIO_CONSOLE_RESIZE).
> > > 
> > > Not bad.  It could become slightly bad if we make devices other than
> > > "consoles" make us of it.  Would that be possible?
> > 
> > I don't think the size has any meaning for devices that are not
> > connected to a console, although the code does not care whether it
> > actually is a console and simply has a size for every chardev.
> 
> Double-checking: the command works for any ChardevBackendKind, doesn't
> it?

Yes. For some (e.g. stdio) it will clash with builtin resize detection,
but it can still be used (last update wins).

Maybe using the command should be prohibited for some device types?

> > I guess I could also rename it to chardev-window-resize
> > or chardev-set-window-size. Let me know if you prefer one of these.
> 
> I think I'd prefer "window" or "terminal".
> 
> "resize" and "set size" suggest that the command initiates a size
> change.  Not true, it notifies of a size change.  Maybe
> "chardev-window-size-changed", "chardev-terminal-size-changed",
> "chardev-window-resized", or "chardev-terminal-resized".

OK, then I'll use "chardev-window-size-changed".

> > > > > 
> > > > 
> [...]
> Another question...  'vc' chardevs accept optional @rows, @cols (see
> ChardevVC).  Is this the same size or something else?

Well, yes and no. @cols + @rows control the actual size of the console
screen buffer, while the chardev size is only used to inform the guest
about the size. @cols and @rows can also be unset, in which case the
size will be determined automatically from display and font size.

This patch series does not yet implement size propagation for the 'vc'
device. I have WIP patches for that, but there is something I'm not
sure how to do, so I will likely send an RFC first.

> > > A clearly invalid size.  I guess it effectively means "unknown size".
> > > Should we document that?
> > 
> > Probably. 0x0 is I think also the default size in the Linux kernel, but
> > I don't think the Linux kernel documents this.
> 
> How does 0 x 0 behave compared to a valid size like 80 x 24?

In these patches it is not treated specially (apart from being the
default). I think the Linux kernel doesn't treat it specially either.
Terminal programs generally interpret it as unknown size and use other
methods to obtain the size like environment variables, the terminfo
database, or defaulting to 80x24. Example:

   $ python -c 'import termios; termios.tcsetwinsize(0, (0,0))'
   $ tput cols
   80

> 
> [...]
> > > > > Do we need a way to query the size?
> > > > 
> > > > I don't think it is necessary. What would be the usecase for that?
> > > 
> > > I don't know, but it's my standard question when I see an interface to
> > > set something without an interface to get it.  Its purpose is to make us
> > > think, not to make us at the get blindly.
> > 
> > I guess it might be useful for debugging. If the size is not propagated
> > correctly, one might query it to find out on which side the problem is.
> 
> We have query-chardev.  It doesn't return much.

I'm not sure what you're implying. Shall I add the size there?
> > > > > 

Re: [PATCH v4 07/10] qmp: add chardev-resize command

[Markus Armbruster]

Filip Hejsek <filip.hejsek@gmail.com> writes:

> On Tue, 2025-09-16 at 15:07 +0200, Markus Armbruster wrote:
>> Filip Hejsek <filip.hejsek@gmail.com> writes:
>> 
>> > On Mon, 2025-09-15 at 08:35 +0200, Markus Armbruster wrote:
>> > > Filip Hejsek <filip.hejsek@gmail.com> writes:
>> > > 
>> > > > On Fri, 2025-09-12 at 16:01 +0200, Markus Armbruster wrote:
>> > > > > Cc: libvirt
>> > > > > 
>> > > > > Filip Hejsek <filip.hejsek@gmail.com> writes:
>> > > > > 
>> > > > > > From: Szymon Lukasz <noh4hss@gmail.com>
>> > > > > > 
>> > > > > > [...]
>> > > > > >  
>> > > > > > +##
>> > > > > > +# @chardev-resize:
>> > > > > 
>> > > > > This name doesn't tell me what is being resized.  PATCH 04 uses
>> > > > > "winsize", which is better.  The (losely) related SIGWINCH suggests
>> > > > > "window change" or "window size change".  Below, you use "terminal
>> > > > > size".
>> > > > 
>> > > > How about chardev-console-resize? That would match the name of the
>> > > > virtio event (VIRTIO_CONSOLE_RESIZE).
>> > > 
>> > > Not bad.  It could become slightly bad if we make devices other than
>> > > "consoles" make us of it.  Would that be possible?
>> > 
>> > I don't think the size has any meaning for devices that are not
>> > connected to a console, although the code does not care whether it
>> > actually is a console and simply has a size for every chardev.
>> 
>> Double-checking: the command works for any ChardevBackendKind, doesn't
>> it?
>
> Yes. For some (e.g. stdio) it will clash with builtin resize detection,
> but it can still be used (last update wins).
>
> Maybe using the command should be prohibited for some device types?

Depends on the command's intended use.

In general, failing a command is better than silently not doing what it
commands.  If the command is "tell the guest the window size changed",
and we can't, then failing the command is better than silently doing
nothing.

Howver, other considerations can trump this.  If we want to use this
command on any window size change without having to know the device
type, having it silently do nothing for device types that don't support
it can be more convenient.  Fine as long as the behavior is clearly
documented.

>> > I guess I could also rename it to chardev-window-resize
>> > or chardev-set-window-size. Let me know if you prefer one of these.
>> 
>> I think I'd prefer "window" or "terminal".
>> 
>> "resize" and "set size" suggest that the command initiates a size
>> change.  Not true, it notifies of a size change.  Maybe
>> "chardev-window-size-changed", "chardev-terminal-size-changed",
>> "chardev-window-resized", or "chardev-terminal-resized".
>
> OK, then I'll use "chardev-window-size-changed".
>
>> > > > > 
>> > > > 
>> [...]
>> Another question...  'vc' chardevs accept optional @rows, @cols (see
>> ChardevVC).  Is this the same size or something else?
>
> Well, yes and no. @cols + @rows control the actual size of the console
> screen buffer, while the chardev size is only used to inform the guest
> about the size. @cols and @rows can also be unset, in which case the
> size will be determined automatically from display and font size.

Thanks!

> This patch series does not yet implement size propagation for the 'vc'
> device. I have WIP patches for that, but there is something I'm not
> sure how to do, so I will likely send an RFC first.

Would it make sense to mention this gap in a commit message?

>> > > A clearly invalid size.  I guess it effectively means "unknown size".
>> > > Should we document that?
>> > 
>> > Probably. 0x0 is I think also the default size in the Linux kernel, but
>> > I don't think the Linux kernel documents this.
>> 
>> How does 0 x 0 behave compared to a valid size like 80 x 24?
>
> In these patches it is not treated specially (apart from being the
> default). I think the Linux kernel doesn't treat it specially either.
> Terminal programs generally interpret it as unknown size and use other
> methods to obtain the size like environment variables, the terminfo
> database, or defaulting to 80x24. Example:
>
>    $ python -c 'import termios; termios.tcsetwinsize(0, (0,0))'
>    $ tput cols
>    80

Do you think working this into the documentation would be useful?

>> [...]
>> > > > > Do we need a way to query the size?
>> > > > 
>> > > > I don't think it is necessary. What would be the usecase for that?
>> > > 
>> > > I don't know, but it's my standard question when I see an interface to
>> > > set something without an interface to get it.  Its purpose is to make us
>> > > think, not to make us at the get blindly.
>> > 
>> > I guess it might be useful for debugging. If the size is not propagated
>> > correctly, one might query it to find out on which side the problem is.
>> 
>> We have query-chardev.  It doesn't return much.
>
> I'm not sure what you're implying. Shall I add the size there?

If we could already query chardev configuration / state comprehensively,
then I'd ask you to make it cover your new state bits, too.

Since we don't, and there is no clear use case at this time, I'm not
asking you to do that.

Re: [PATCH v2] char-pty: add support for the terminal size

[Maximilian Immanuel Brandtner]

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?

Re: [PATCH v2] char-pty: add support for the terminal size

[Filip Hejsek]

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.

Re: [PATCH v2] char-pty: add support for the terminal size

[Daniel P. Berrangé]

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

Re: [PATCH v2] char-pty: add support for the terminal size

[Filip Hejsek]

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

Re: [PATCH v2] char-pty: add support for the terminal size

[Daniel P. Berrangé]

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

Re: [PATCH v2] char-pty: add support for the terminal size

[Filip Hejsek]

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.

Re: [PATCH v2] char-pty: add support for the terminal size

[Daniel P. Berrangé]

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

Re: [PATCH v2] char-pty: add support for the terminal size

[Filip Hejsek]

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

Re: [PATCH v4 00/10] virtio-console: notify about the terminal size

[Filip Hejsek]

On Mon, 2025-09-15 at 19:08 -0400, Michael S. Tsirkin wrote:
> On Tue, Sep 16, 2025 at 01:02:02AM +0200, Filip Hejsek wrote:
> > While thinking about the patches, a few questions about the virtio spec
> > have popped into my head.
> > 
> > 1. Should the config space size also be updated (for port 0) when
> > multiport is used? Based on my reading of the spec, I think yes.
> > 
> > 2. Can VIRTIO_CONSOLE_RESIZE be sent if VIRTIO_CONSOLE_F_SIZE is not
> > negotiated? The spec does not say, which I think means it can.
> 
> But the guest can't do anything useful here.

It can if the reason VIRTIO_CONSOLE_F_SIZE was not negotiated is that
the host only supports sending size for multiport devices (e.g. because
port 0 does not have size).

> > 3. The spec says that reading from config space fields that are
> > conditional on features should be allowed even if the driver has not
> > (yet) accepted the feature. Does it mean that we have to update the
> > size even if the feature is not accepted (yet), or is it OK if the
> > reads return 0?
> 
> This is talking about the window before FEATURES_OK (and so DRIVER_OK)
> is set.  It is best to update the size. There's no interrupt to send
> though.
> 
> 
> > Thanks for any answers or opinions,
> > Filip Hejsek

Re: [PATCH v2] char-pty: add support for the terminal size

[Maximilian Immanuel Brandtner]

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.

Re: [PATCH v2] char-pty: add support for the terminal size

[Filip Hejsek]

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

Re: [PATCH v4 08/10] virtio-serial-bus: add terminal resize messages

[Daniel P. Berrangé]

On Fri, Sep 12, 2025 at 05:39:53AM +0200, Filip Hejsek wrote:
> From: Szymon Lukasz <noh4hss@gmail.com>
> 
> Implement the part of the virtio spec that allows to notify the virtio
> driver about terminal resizes. The virtio spec contains two methods to
> achieve that:
> 
> For legacy drivers, we have only one port and we put the terminal size
> in the config space and inject the config changed interrupt.
> 
> For multiport devices, we use the control virtqueue to send a packet
> containing the terminal size. Note that old versions of the Linux kernel
> used an incorrect order for the fields (rows then cols instead of cols
> then rows), until it was fixed by commit 5326ab737a47278dbd16ed3ee7380b26c7056ddd.
> 
> As a result, when using a Linux kernel older than 6.15, the number of rows
> and columns will be swapped.
> 
> Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> ---
>  hw/char/trace-events              |  1 +
>  hw/char/virtio-serial-bus.c       | 43 +++++++++++++++++++++++++++++++++++++--
>  hw/core/machine.c                 |  4 +++-
>  include/hw/virtio/virtio-serial.h |  5 +++++
>  4 files changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/char/trace-events b/hw/char/trace-events
> index 05a33036c12070242c2b193c19011839d623bec4..9a975ab1e2a525a9391d0f0a85ddbe80aa6361fc 100644
> --- a/hw/char/trace-events
> +++ b/hw/char/trace-events
> @@ -11,6 +11,7 @@ serial_update_parameters(uint64_t baudrate, char parity, int data_bits, int stop
>  
>  # virtio-serial-bus.c
>  virtio_serial_send_control_event(unsigned int port, uint16_t event, uint16_t value) "port %u, event %u, value %u"
> +virtio_serial_send_console_resize(unsigned int port, uint16_t cols, uint16_t rows) "port %u, cols %u, rows %u"
>  virtio_serial_throttle_port(unsigned int port, bool throttle) "port %u, throttle %d"
>  virtio_serial_handle_control_message(uint16_t event, uint16_t value) "event %u, value %u"
>  virtio_serial_handle_control_message_port(unsigned int port) "port %u"
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index 673c50f0be08ef9b7142c16eaf8e6e31c7a00ca5..1d2963efcd74494a1f0d428f8ace0e72bb4c6647 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -260,6 +260,43 @@ static size_t send_control_event(VirtIOSerial *vser, uint32_t port_id,
>      return send_control_msg(vser, &cpkt, sizeof(cpkt));
>  }
>  
> +/*
> + * This struct should be added to the Linux kernel uapi headers
> + * and later imported to standard-headers/linux/virtio_console.h
> + */
> +struct virtio_console_resize {
> +    __virtio16 cols;
> +    __virtio16 rows;
> +};
> +
> +void virtio_serial_send_console_resize(VirtIOSerialPort *port,
> +                                       uint16_t cols, uint16_t rows)
> +{
> +    VirtIOSerial *vser = port->vser;
> +    VirtIODevice *vdev = VIRTIO_DEVICE(vser);
> +
> +    trace_virtio_serial_send_console_resize(port->id, cols, rows);
> +
> +    if (virtio_vdev_has_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT)) {
> +        struct {
> +            struct virtio_console_control control;
> +            struct virtio_console_resize resize;
> +        } buffer;
> +
> +        virtio_stl_p(vdev, &buffer.control.id, port->id);
> +        virtio_stw_p(vdev, &buffer.control.event, VIRTIO_CONSOLE_RESIZE);
> +        virtio_stw_p(vdev, &buffer.resize.cols, cols);
> +        virtio_stw_p(vdev, &buffer.resize.rows, rows);
> +
> +        send_control_msg(vser, &buffer, sizeof(buffer));
> +
> +    } else if (virtio_vdev_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE)) {
> +        vser->port0_cols = cols;
> +        vser->port0_rows = rows;
> +        virtio_notify_config(vdev);

IIUC, we should skip sending this if port->id does NOT reflect
the primary port 0, as the virtio spec indicates F_SIZE only
applies to port 0 unless we have F_MULTIPORT  set.

> +    }
> +}
> +
>  /* Functions for use inside qemu to open and read from/write to ports */
>  int virtio_serial_open(VirtIOSerialPort *port)
>  {
> @@ -571,8 +608,8 @@ static void get_config(VirtIODevice *vdev, uint8_t *config_data)
>      struct virtio_console_config *config =
>          (struct virtio_console_config *)config_data;
>  
> -    config->cols = 0;
> -    config->rows = 0;
> +    config->cols = virtio_tswap16(vdev, vser->port0_cols);
> +    config->rows = virtio_tswap16(vdev, vser->port0_rows);
>      config->max_nr_ports = virtio_tswap32(vdev,
>                                            vser->serial.max_virtserial_ports);
>  }
> @@ -1158,6 +1195,8 @@ static const Property virtio_serial_properties[] = {
>                                                    31),
>      DEFINE_PROP_BIT64("emergency-write", VirtIOSerial, host_features,
>                        VIRTIO_CONSOLE_F_EMERG_WRITE, true),
> +    DEFINE_PROP_BIT64("console-size", VirtIOSerial, host_features,
> +                      VIRTIO_CONSOLE_F_SIZE, true),
>  };
>  
>  static void virtio_serial_class_init(ObjectClass *klass, const void *data)
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 38c949c4f2ce4a117cbfca62f56919711ce392b4..74a747ec6578c958b35e1f9712e5dbed7bca72e8 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -37,7 +37,9 @@
>  #include "hw/virtio/virtio-iommu.h"
>  #include "audio/audio.h"
>  
> -GlobalProperty hw_compat_10_1[] = {};
> +GlobalProperty hw_compat_10_1[] = {
> +    { "virtio-serial-device", "console-size", "off" },
> +};
>  const size_t hw_compat_10_1_len = G_N_ELEMENTS(hw_compat_10_1);
>  
>  GlobalProperty hw_compat_10_0[] = {
> diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
> index d87c62eab7a270809daf47f932a73dd1fa3d5a6e..81efa853f804a52866890a9ec2c71bfbcabca4a0 100644
> --- a/include/hw/virtio/virtio-serial.h
> +++ b/include/hw/virtio/virtio-serial.h
> @@ -187,6 +187,8 @@ struct VirtIOSerial {
>      virtio_serial_conf serial;
>  
>      uint64_t host_features;
> +
> +    uint16_t port0_cols, port0_rows;
>  };
>  
>  /* Interface to the virtio-serial bus */
> @@ -221,6 +223,9 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port);
>   */
>  void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle);
>  
> +void virtio_serial_send_console_resize(VirtIOSerialPort *port,
> +                                       uint16_t cols, uint16_t rows);
> +
>  #define TYPE_VIRTIO_SERIAL "virtio-serial-device"
>  OBJECT_DECLARE_SIMPLE_TYPE(VirtIOSerial, VIRTIO_SERIAL)
>  
> 
> -- 
> 2.51.0
> 
> 

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

Re: [PATCH v4 04/10] char-mux: add support for the terminal size

[Maximilian Immanuel Brandtner]

On Fri, 2025-09-12 at 05:39 +0200, Filip Hejsek wrote:
> From: Szymon Lukasz <noh4hss@gmail.com>
> 
> The terminal size of a mux chardev should be the same as the real
> chardev, so listen for CHR_EVENT_RESIZE to be up to date.
> 
> We forward CHR_EVENT_RESIZE only to the focused frontend. This means
> frontends should probably update their view of the terminal size on
> receiving CHR_EVENT_MUX_IN.
> 
> Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
> Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> ---
>  chardev/char-mux.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> index
> 6b36290e2c49f579580d2abb5aa552806f019d4a..4d3d05b82f13e002c766142f9d9
> c24977b8b9bd2 100644
> --- a/chardev/char-mux.c
> +++ b/chardev/char-mux.c
> @@ -264,9 +264,24 @@ void mux_chr_send_all_event(Chardev *chr,
> QEMUChrEvent event)
>      }
>  }
>  
> +static void mux_update_winsize(Chardev *chr)
> +{
> +    MuxChardev *d = MUX_CHARDEV(chr);
> +    uint16_t cols, rows;
> +
> +    qemu_chr_fe_get_winsize(&d->chr, &cols, &rows);
> +    qemu_chr_resize(chr, cols, rows);
> +}
> +
>  static void mux_chr_event(void *opaque, QEMUChrEvent event)
>  {
> -    mux_chr_send_all_event(CHARDEV(opaque), event);
> +    Chardev *chr = CHARDEV(opaque);
> +
> +    if (event == CHR_EVENT_RESIZE) {
> +        mux_update_winsize(chr);
> +    } else {
> +        mux_chr_send_all_event(chr, event);
> +    }
>  }
>  
>  static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
> @@ -382,6 +397,7 @@ static void qemu_chr_open_mux(Chardev *chr,
>       */
>      *be_opened = muxes_opened;
>      qemu_chr_fe_init(&d->chr, drv, errp);
> +    mux_update_winsize(chr);
>  }
>  
>  static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend
> *backend,
> 

When changing the focussed chardev, the MuxChardev should send a resize
event to the newly focussed chardev. Otherwise the size information of
the focussed chardev might be outdated if it wasn't the focussed
chardev at the time of the resize event.

Theoretically, the resize event could also just be sent to all
character devices focussed or not, however as this causes a lot of
needless redrawing I prefer the approach of only resizing the focussed
chardev.

Kind regards,
Max Brandtner

Re: [PATCH v2] char-pty: add support for the terminal size

[Daniel P. Berrangé]

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

Re: [PATCH v2] char-pty: add support for the terminal size

[Maximilian Immanuel Brandtner]

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

Re: [PATCH v4 03/10] chardev: add qemu_chr_resize()

[Maximilian Immanuel Brandtner]

On Fri, 2025-09-12 at 05:39 +0200, Filip Hejsek wrote:
> From: Szymon Lukasz <noh4hss@gmail.com>
> 
> This function should be called whenever we learn about a new size of
> the terminal connected to a chardev.
> 
> Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
> Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> ---
>  chardev/char.c         | 11 +++++++++++
>  include/chardev/char.h |  2 ++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/chardev/char.c b/chardev/char.c
> index
> 635d19fea4fd4bd0c7f171f055fe940f9f5ebed5..b45d79cb9b57643827eb7479257
> fdda2cf6b0434 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -351,6 +351,17 @@ int qemu_chr_wait_connected(Chardev *chr, Error
> **errp)
>      return 0;
>  }
>  
> +void qemu_chr_resize(Chardev *chr, uint16_t cols, uint16_t rows)
> +{
> +    if (cols != chr->cols || rows != chr->rows) {

Perhaps it would be better to discard resize events if the requested
cols or rows is 0 as it indicates that an error has occurred at some
point during the process of receiving the winsize.

> +        chr->cols = cols;
> +        chr->rows = rows;
> +        if (chr->be_open) {
> +            qemu_chr_be_event(chr, CHR_EVENT_RESIZE);
> +        }
> +    }
> +}
> +
>  QemuOpts *qemu_chr_parse_compat(const char *label, const char
> *filename,
>                                  bool permit_mux_mon)
>  {
> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index
> 45cb6349756ac8072dffab9354108caf90cd3565..1e69b038241074d627ebb7f096e
> 98aee9953ebdf 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -232,6 +232,8 @@ int qemu_chr_write(Chardev *s, const uint8_t
> *buf, int len, bool write_all);
>  #define qemu_chr_write_all(s, buf, len) qemu_chr_write(s, buf, len,
> true)
>  int qemu_chr_wait_connected(Chardev *chr, Error **errp);
>  
> +void qemu_chr_resize(Chardev *chr, uint16_t cols, uint16_t rows);
> +
>  #define TYPE_CHARDEV "chardev"
>  OBJECT_DECLARE_TYPE(Chardev, ChardevClass, CHARDEV)
>  
> 

Re: [PATCH v2] char-pty: add support for the terminal size

[Daniel P. Berrangé]

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

Re: [PATCH v4 08/10] virtio-serial-bus: add terminal resize messages

[Filip Hejsek]

On September 18, 2025 10:23:33 AM GMT+02:00, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
> On Fri, Sep 12, 2025 at 05:39:53AM +0200, Filip Hejsek wrote:
> > From: Szymon Lukasz <noh4hss@gmail.com>
> > 
> > Implement the part of the virtio spec that allows to notify the virtio
> > driver about terminal resizes. The virtio spec contains two methods to
> > achieve that:
> > 
> > For legacy drivers, we have only one port and we put the terminal size
> > in the config space and inject the config changed interrupt.
> > 
> > For multiport devices, we use the control virtqueue to send a packet
> > containing the terminal size. Note that old versions of the Linux kernel
> > used an incorrect order for the fields (rows then cols instead of cols
> > then rows), until it was fixed by commit 5326ab737a47278dbd16ed3ee7380b26c7056ddd.
> > 
> > As a result, when using a Linux kernel older than 6.15, the number of rows
> > and columns will be swapped.
> > 
> > Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> > ---
> >  [...]
> > +        vser->port0_cols = cols;
> > +        vser->port0_rows = rows;
> > +        virtio_notify_config(vdev);
> 
> IIUC, we should skip sending this if port->id does NOT reflect
> the primary port 0, as the virtio spec indicates F_SIZE only
> applies to port 0 unless we have F_MULTIPORT  set.

I have already changed this in my working version. I'm on a phone
and don't have access to it right now, but it was something like this pseudocode:

if "console-size" enabled:
  if port id == 0:
    port0_{cols,rows} = {cols,rows}
  if multiport:
    send VIRTIO_CONSOLE_RESIZE
  else:
    notify config

I guess I should make the config notification conditional on port 0 too.

Re: [PATCH v2] char-pty: add support for the terminal size

[Maximilian Immanuel Brandtner]

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

Re: [PATCH v2] char-pty: add support for the terminal size

[Daniel P. Berrangé]

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

Re: [PATCH v2] char-pty: add support for the terminal size

[Maximilian Immanuel Brandtner]

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

Re: [PATCH v4 04/10] char-mux: add support for the terminal size

[Filip Hejsek]

On September 18, 2025 10:32:57 AM GMT+02:00, Maximilian Immanuel Brandtner <maxbr@linux.ibm.com> wrote:
> On Fri, 2025-09-12 at 05:39 +0200, Filip Hejsek wrote:
> > From: Szymon Lukasz <noh4hss@gmail.com>
> > 
> > The terminal size of a mux chardev should be the same as the real
> > chardev, so listen for CHR_EVENT_RESIZE to be up to date.
> > 
> > We forward CHR_EVENT_RESIZE only to the focused frontend. This means
> > frontends should probably update their view of the terminal size on
> > receiving CHR_EVENT_MUX_IN.
> > 
> > Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
> > Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> > ---
> >  chardev/char-mux.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> > index
> > 6b36290e2c49f579580d2abb5aa552806f019d4a..4d3d05b82f13e002c766142f9d9
> > c24977b8b9bd2 100644
> > --- a/chardev/char-mux.c
> > +++ b/chardev/char-mux.c
> > @@ -264,9 +264,24 @@ void mux_chr_send_all_event(Chardev *chr,
> > QEMUChrEvent event)
> >      }
> >  }
> >  
> > +static void mux_update_winsize(Chardev *chr)
> > +{
> > +    MuxChardev *d = MUX_CHARDEV(chr);
> > +    uint16_t cols, rows;
> > +
> > +    qemu_chr_fe_get_winsize(&d->chr, &cols, &rows);
> > +    qemu_chr_resize(chr, cols, rows);
> > +}
> > +
> >  static void mux_chr_event(void *opaque, QEMUChrEvent event)
> >  {
> > -    mux_chr_send_all_event(CHARDEV(opaque), event);
> > +    Chardev *chr = CHARDEV(opaque);
> > +
> > +    if (event == CHR_EVENT_RESIZE) {
> > +        mux_update_winsize(chr);
> > +    } else {
> > +        mux_chr_send_all_event(chr, event);
> > +    }
> >  }
> >  
> >  static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
> > @@ -382,6 +397,7 @@ static void qemu_chr_open_mux(Chardev *chr,
> >       */
> >      *be_opened = muxes_opened;
> >      qemu_chr_fe_init(&d->chr, drv, errp);
> > +    mux_update_winsize(chr);
> >  }
> >  
> >  static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend
> > *backend,
> > 
> 
> When changing the focussed chardev, the MuxChardev should send a resize
> event to the newly focussed chardev. Otherwise the size information of
> the focussed chardev might be outdated if it wasn't the focussed
> chardev at the time of the resize event.
> 
> Theoretically, the resize event could also just be sent to all
> character devices focussed or not, however as this causes a lot of
> needless redrawing I prefer the approach of only resizing the focussed
> chardev.
> 
> Kind regards,
> Max Brandtner
> 

Right now, this is handled by frontends also updating the size
on CHR_EVENT_MUX_IN, as mentioned in the commit message.
I could make it so that CHR_EVENT_MUX_IN is always followed
by CHR_EVENT_RESIZE, if that is preferred. A more complicated
option is to remember for each frontend if it missed a resize event,
and only send it then, but that seems like needless complexity.

Kind regards,
Filip

Re: [PATCH v4 03/10] chardev: add qemu_chr_resize()

[Filip Hejsek]

On September 18, 2025 10:45:13 AM GMT+02:00, Maximilian Immanuel Brandtner <maxbr@linux.ibm.com> wrote:
> On Fri, 2025-09-12 at 05:39 +0200, Filip Hejsek wrote:
> > From: Szymon Lukasz <noh4hss@gmail.com>
> > 
> > This function should be called whenever we learn about a new size of
> > the terminal connected to a chardev.
> > 
> > Signed-off-by: Szymon Lukasz <noh4hss@gmail.com>
> > Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> > ---
> >  chardev/char.c         | 11 +++++++++++
> >  include/chardev/char.h |  2 ++
> >  2 files changed, 13 insertions(+)
> > 
> > diff --git a/chardev/char.c b/chardev/char.c
> > index
> > 635d19fea4fd4bd0c7f171f055fe940f9f5ebed5..b45d79cb9b57643827eb7479257
> > fdda2cf6b0434 100644
> > --- a/chardev/char.c
> > +++ b/chardev/char.c
> > @@ -351,6 +351,17 @@ int qemu_chr_wait_connected(Chardev *chr, Error
> > **errp)
> >      return 0;
> >  }
> >  
> > +void qemu_chr_resize(Chardev *chr, uint16_t cols, uint16_t rows)
> > +{
> > +    if (cols != chr->cols || rows != chr->rows) {
> 
> Perhaps it would be better to discard resize events if the requested
> cols or rows is 0 as it indicates that an error has occurred at some
> point during the process of receiving the winsize.

Maybe. But then if the size actually changes from known
to unknown, it wouldn't be possible to get it back to the unknown
state. And what if only
one of the values is 0, and the other differs from the last value?

What I want to say is, I don't think QEMU should be in the business
of trying to fix broken size info. The kernel gave us zero,
so we should forward that and let the guest handle it.

> > +        chr->cols = cols;
> > +        chr->rows = rows;
> > +        if (chr->be_open) {
> > +            qemu_chr_be_event(chr, CHR_EVENT_RESIZE);
> > +        }
> > +    }
> > +}
> > +
> >  QemuOpts *qemu_chr_parse_compat(const char *label, const char
> > *filename,
> >                                  bool permit_mux_mon)
> >  {
> > diff --git a/include/chardev/char.h b/include/chardev/char.h
> > index
> > 45cb6349756ac8072dffab9354108caf90cd3565..1e69b038241074d627ebb7f096e
> > 98aee9953ebdf 100644
> > --- a/include/chardev/char.h
> > +++ b/include/chardev/char.h
> > @@ -232,6 +232,8 @@ int qemu_chr_write(Chardev *s, const uint8_t
> > *buf, int len, bool write_all);
> >  #define qemu_chr_write_all(s, buf, len) qemu_chr_write(s, buf, len,
> > true)
> >  int qemu_chr_wait_connected(Chardev *chr, Error **errp);
> >  
> > +void qemu_chr_resize(Chardev *chr, uint16_t cols, uint16_t rows);
> > +
> >  #define TYPE_CHARDEV "chardev"
> >  OBJECT_DECLARE_TYPE(Chardev, ChardevClass, CHARDEV)
> >  
> > 
> 

Re: [PATCH v2] char-pty: add support for the terminal size

[Filip Hejsek]

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