[PATCH v3 00/10] chardev: Improve @docstring and clarify qemu_chr_write() uses

[PATCH v3 00/10] chardev: Improve @docstring and clarify qemu_chr_write() uses

[Philippe Mathieu-Daudé]

v3:
- Addressed Marc-André & Daniel comments
- Document qemu_chr_fe_write()
- Rename replay arguments

v2:
- Document ChardevClass::chr_write() and qemu_chr_write[_all]()

Few chardev fixes:
- preserve %errno
- allow partial writes in qemu_chr_write()

Improve chardev methods documentation.

While @c for frontend and @s for backend is accepted, it
confuses me, so I prefer to document for my own mental health.

Based-on: <20251022074612.1258413-1-marcandre.lureau@redhat.com>

Philippe Mathieu-Daudé (10):
  chardev/char-fe: Improve @docstrings
  chardev/char-io: Add @docstrings for io_channel_send[_full]()
  chardev/char: Improve ChardevClass::chr_write() docstring
  chardev/char: Document qemu_chr_write[_all]()
  chardev/char-pty: Do not ignore chr_write() failures
  chardev/char: Allow partial writes in qemu_chr_write()
  chardev/char: Preserve %errno in qemu_chr_write()
  chardev/char: Document qemu_chr_fe_write()'s return value
  hw/char: Simplify when qemu_chr_fe_write() could not write
  system/replay: Rename some method parameters

 include/chardev/char-fe.h | 33 ++++++++++++++++++++++++++-----
 include/chardev/char-io.h | 20 +++++++++++++++++++
 include/chardev/char.h    | 41 ++++++++++++++++++++++++++++++++++++++-
 include/system/replay.h   | 16 +++++++--------
 chardev/char-pty.c        |  2 +-
 chardev/char.c            | 11 +++++++++++
 hw/char/cadence_uart.c    |  2 +-
 hw/char/ibex_uart.c       |  2 +-
 hw/char/sifive_uart.c     |  2 +-
 replay/replay-char.c      | 14 ++++++-------
 stubs/replay-tools.c      | 10 +++++-----
 11 files changed, 123 insertions(+), 30 deletions(-)

-- 
2.51.0

[PATCH v3 01/10] chardev/char-fe: Improve @docstrings

[Philippe Mathieu-Daudé]

Describe the @c (this is the *frontend*) and @s (the *backend*)
parameters. Fill qemu_chr_fe_[gs]et_msgfds() method docstrings.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/chardev/char-fe.h | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index 5f8a6df17dc..36bcf1f88a6 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -26,6 +26,8 @@ struct CharFrontend {
 
 /**
  * qemu_chr_fe_init:
+ * @c: the character frontend
+ * @s: the character backend
  *
  * Initializes the frontend @c for the given Chardev backend @s. Call
  * qemu_chr_fe_deinit() to remove the association and release the backend.
@@ -47,6 +49,7 @@ void qemu_chr_fe_deinit(CharFrontend *c, bool del);
 
 /**
  * qemu_chr_fe_get_driver:
+ * @c: the character frontend
  *
  * Returns: the driver associated with a CharFrontend or NULL if no
  * associated Chardev.
@@ -58,6 +61,7 @@ Chardev *qemu_chr_fe_get_driver(CharFrontend *c);
 
 /**
  * qemu_chr_fe_backend_connected:
+ * @c: the character frontend
  *
  * Returns: true if there is a backend associated with @c.
  */
@@ -102,6 +106,7 @@ void qemu_chr_fe_set_handlers_full(CharFrontend *c,
 
 /**
  * qemu_chr_fe_set_handlers:
+ * @c: the character frontend
  *
  * Version of qemu_chr_fe_set_handlers_full() with sync_state = true.
  */
@@ -116,6 +121,7 @@ void qemu_chr_fe_set_handlers(CharFrontend *c,
 
 /**
  * qemu_chr_fe_take_focus:
+ * @c: the character frontend
  *
  * Take the focus (if the front end is muxed).
  *
@@ -125,6 +131,7 @@ void qemu_chr_fe_take_focus(CharFrontend *c);
 
 /**
  * qemu_chr_fe_accept_input:
+ * @c: the character frontend
  *
  * Notify that the frontend is ready to receive data
  */
@@ -132,6 +139,7 @@ void qemu_chr_fe_accept_input(CharFrontend *c);
 
 /**
  * qemu_chr_fe_disconnect:
+ * @c: the character frontend
  *
  * Close a fd accepted by character backend.
  * Without associated Chardev, do nothing.
@@ -148,6 +156,7 @@ int qemu_chr_fe_wait_connected(CharFrontend *c, Error **errp);
 
 /**
  * qemu_chr_fe_set_echo:
+ * @c: the character frontend
  * @echo: true to enable echo, false to disable echo
  *
  * Ask the backend to override its normal echo setting.  This only really
@@ -169,6 +178,7 @@ void qemu_chr_fe_set_open(CharFrontend *c, bool is_open);
 
 /**
  * qemu_chr_fe_printf:
+ * @c: the character frontend
  * @fmt: see #printf
  *
  * Write to a character backend using a printf style interface.  This
@@ -197,6 +207,7 @@ typedef gboolean (*FEWatchFunc)(void *do_not_use, GIOCondition condition, void *
 
 /**
  * qemu_chr_fe_add_watch:
+ * @c: the character frontend
  * @cond: the condition to poll for
  * @func: the function to call when the condition happens
  * @user_data: the opaque pointer to pass to @func
@@ -219,6 +230,7 @@ guint qemu_chr_fe_add_watch(CharFrontend *c, GIOCondition cond,
 
 /**
  * qemu_chr_fe_write:
+ * @c: the character frontend to write to
  * @buf: the data
  * @len: the number of bytes to send
  *
@@ -233,6 +245,7 @@ int qemu_chr_fe_write(CharFrontend *c, const uint8_t *buf, int len);
 
 /**
  * qemu_chr_fe_write_all:
+ * @c: the character frontend to write to
  * @buf: the data
  * @len: the number of bytes to send
  *
@@ -248,6 +261,7 @@ int qemu_chr_fe_write_all(CharFrontend *c, const uint8_t *buf, int len);
 
 /**
  * qemu_chr_fe_read_all:
+ * @c: the character frontend to read from
  * @buf: the data buffer
  * @len: the number of bytes to read
  *
@@ -260,6 +274,7 @@ int qemu_chr_fe_read_all(CharFrontend *c, uint8_t *buf, int len);
 
 /**
  * qemu_chr_fe_ioctl:
+ * @c: the character frontend to control
  * @cmd: see CHR_IOCTL_*
  * @arg: the data associated with @cmd
  *
@@ -273,6 +288,7 @@ int qemu_chr_fe_ioctl(CharFrontend *c, int cmd, void *arg);
 
 /**
  * qemu_chr_fe_get_msgfd:
+ * @c: the character frontend to access
  *
  * For backends capable of fd passing, return the latest file descriptor passed
  * by a client.
@@ -286,9 +302,12 @@ int qemu_chr_fe_get_msgfd(CharFrontend *c);
 
 /**
  * qemu_chr_fe_get_msgfds:
+ * @c: the character frontend
+ * @fds: an array of ancillary file descriptors to get
+ * @num: the maximum number of ancillary file descriptors to get in @fds
  *
  * For backends capable of fd passing, return the number of file received
- * descriptors and fills the fds array up to num elements
+ * descriptors and fills the fds array up to @num elements
  *
  * Returns: -1 if fd passing isn't supported or there are no pending file
  *          descriptors.  If file descriptors are returned, subsequent calls to
@@ -299,6 +318,9 @@ int qemu_chr_fe_get_msgfds(CharFrontend *c, int *fds, int num);
 
 /**
  * qemu_chr_fe_set_msgfds:
+ * @c: the character frontend
+ * @fds: an array of ancillary file descriptors to set
+ * @num: the number of ancillary file descriptors to set
  *
  * For backends capable of fd passing, set an array of fds to be passed with
  * the next send operation.
-- 
2.51.0

[PATCH v3 02/10] chardev/char-io: Add @docstrings for io_channel_send[_full]()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/chardev/char-io.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/chardev/char-io.h b/include/chardev/char-io.h
index ac379ea70e3..5b181be6a0b 100644
--- a/include/chardev/char-io.h
+++ b/include/chardev/char-io.h
@@ -38,8 +38,28 @@ GSource *io_add_watch_poll(Chardev *chr,
 
 void remove_fd_in_watch(Chardev *chr);
 
+/**
+ * io_channel_send:
+ * @ioc: the IO channel object
+ * @buf: the data
+ * @len: the number of bytes to send
+ *
+ * Returns: the number of bytes consumed or -1 on error.
+ * On error, %errno is also set as appropriate.
+ */
 int io_channel_send(QIOChannel *ioc, const void *buf, size_t len);
 
+/**
+ * io_channel_send_full:
+ * @ioc: the IO channel object
+ * @buf: the data
+ * @len: the number of bytes to send
+ * @fds: an array of file handles to send
+ * @nfds: number of file handles in @fds
+ *
+ * Returns: the number of bytes consumed or -1 on error.
+ * On error, %errno is also set as appropriate.
+ */
 int io_channel_send_full(QIOChannel *ioc, const void *buf, size_t len,
                          int *fds, size_t nfds);
 
-- 
2.51.0

[PATCH v3 03/10] chardev/char: Improve ChardevClass::chr_write() docstring

[Philippe Mathieu-Daudé]

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/chardev/char.h | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/chardev/char.h b/include/chardev/char.h
index b65e9981c14..df55c7f4e67 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -263,7 +263,17 @@ struct ChardevClass {
     void (*open)(Chardev *chr, ChardevBackend *backend,
                  bool *be_opened, Error **errp);
 
-    /* write buf to the backend */
+    /**
+     * chr_write: Write data to a character backend
+     * @s: the character backend to write to
+     * @buf: the data to write
+     * @len: the number of bytes to write
+     *
+     * Called with chr_write_lock held.
+     *
+     * Returns: the number of bytes consumed or -1 on error.
+     * On error, %errno is also set as appropriate.
+     */
     int (*chr_write)(Chardev *s, const uint8_t *buf, int len);
 
     /*
-- 
2.51.0

[PATCH v3 04/10] chardev/char: Document qemu_chr_write[_all]()

[Philippe Mathieu-Daudé]

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/chardev/char.h | 29 +++++++++++++++++++++++++++++
 chardev/char.c         |  1 +
 2 files changed, 30 insertions(+)

diff --git a/include/chardev/char.h b/include/chardev/char.h
index df55c7f4e67..4aab018938d 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -223,8 +223,37 @@ void qemu_chr_set_feature(Chardev *chr,
                           ChardevFeature feature);
 QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename,
                                 bool permit_mux_mon);
+
+/**
+ * qemu_chr_write: Write data to a character backend
+ * @s: the character backend to write to
+ * @buf: the data
+ * @len: the number of bytes to write
+ * @write_all: whether to block until all chars are written
+ *
+ * Attempt to write all the data to the backend. If not all
+ * data can be consumed and @write_all is %true, keep retrying
+ * while the backend return EAGAIN, effectively blocking the caller.
+ *
+ * Returns: the number of bytes consumed or -1 on error.
+ * On error, %errno is also set as appropriate.
+ */
 int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all);
+
+/**
+ * qemu_chr_write_all: Write data to a character backend
+ * @s: the character backend to write to
+ * @buf: the data
+ * @len: the number of bytes to write
+ *
+ * Unlike @qemu_chr_write, this call will block if the backend
+ * cannot consume all of the data attempted to be written.
+ *
+ * Returns: the number of bytes consumed or -1 on error.
+ * On error, %errno is also set as appropriate.
+ */
 #define qemu_chr_write_all(s, buf, len) qemu_chr_write(s, buf, len, true)
+
 int qemu_chr_wait_connected(Chardev *chr, Error **errp);
 
 #define TYPE_CHARDEV "chardev"
diff --git a/chardev/char.c b/chardev/char.c
index 3e432195a5a..105b0d53184 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -108,6 +108,7 @@ static void qemu_chr_write_log(Chardev *s, const uint8_t *buf, size_t len)
     }
 }
 
+/* qemu_chr_write_buffer: Return the number of bytes consumed or -1 on error. */
 static int qemu_chr_write_buffer(Chardev *s,
                                  const uint8_t *buf, int len,
                                  int *offset, bool write_all)
-- 
2.51.0

[PATCH v3 05/10] chardev/char-pty: Do not ignore chr_write() failures

[Philippe Mathieu-Daudé]

Cc: qemu-stable@nongnu.org
Fixes: 4f7689f0817 ("Avoid losing bytes when on (re-)connect")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 chardev/char-pty.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index b066f014126..652b0bd9e73 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -125,7 +125,7 @@ static int char_pty_chr_write(Chardev *chr, const uint8_t *buf, int len)
     rc = RETRY_ON_EINTR(g_poll(&pfd, 1, 0));
     g_assert(rc >= 0);
     if (!(pfd.revents & G_IO_HUP) && (pfd.revents & G_IO_OUT)) {
-        io_channel_send(s->ioc, buf, len);
+        return io_channel_send(s->ioc, buf, len);
     }
 
     return len;
-- 
2.51.0

[PATCH v3 06/10] chardev/char: Allow partial writes in qemu_chr_write()

[Philippe Mathieu-Daudé]

If qemu_chr_write_buffer() returned an error, but could
write some characters, return the number of character
written. Otherwise frontends able to recover and resume
writes re-write the partial chars already written.

Cc: qemu-stable@nongnu.org
Suggested-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 chardev/char.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/chardev/char.c b/chardev/char.c
index 105b0d53184..7931f4e0832 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -186,6 +186,11 @@ int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all)
 
     res = qemu_chr_write_buffer(s, buf, len, &offset, write_all);
 
+    if (!write_all && res < 0 && offset == 0) {
+        /* Allow partial writes */
+        return res;
+    }
+
     if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_RECORD) {
         replay_char_write_event_save(res, offset);
     }
-- 
2.51.0

[PATCH v3 07/10] chardev/char: Preserve %errno in qemu_chr_write()

[Philippe Mathieu-Daudé]

qemu_chr_write() dispatches to ChardevClass::chr_write(),
and is expected to propagate the backend error, not some
unrelated one produce by "best effort" logfile or replay.
Preserve and return the relevant %errno.

Cc: qemu-stable@nongnu.org
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 chardev/char.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/chardev/char.c b/chardev/char.c
index 7931f4e0832..46a2dbfb5a3 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -114,6 +114,7 @@ static int qemu_chr_write_buffer(Chardev *s,
                                  int *offset, bool write_all)
 {
     ChardevClass *cc = CHARDEV_GET_CLASS(s);
+    int saved_errno;
     int res = 0;
     *offset = 0;
 
@@ -139,6 +140,7 @@ static int qemu_chr_write_buffer(Chardev *s,
             break;
         }
     }
+    saved_errno = errno;
     if (*offset > 0) {
         /*
          * If some data was written by backend, we should
@@ -156,6 +158,7 @@ static int qemu_chr_write_buffer(Chardev *s,
         qemu_chr_write_log(s, buf, len);
     }
     qemu_mutex_unlock(&s->chr_write_lock);
+    errno = saved_errno;
 
     return res;
 }
@@ -192,7 +195,9 @@ int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all)
     }
 
     if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_RECORD) {
+        int saved_errno = errno;
         replay_char_write_event_save(res, offset);
+        errno = saved_errno;
     }
 
     if (res < 0) {
-- 
2.51.0

[PATCH v3 08/10] chardev/char: Document qemu_chr_fe_write()'s return value

[Philippe Mathieu-Daudé]

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/chardev/char-fe.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index 36bcf1f88a6..fbec6a73dbb 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -238,8 +238,9 @@ guint qemu_chr_fe_add_watch(CharFrontend *c, GIOCondition cond,
  * will send data from the front end to the back end.  This function
  * is thread-safe.
  *
- * Returns: the number of bytes consumed (0 if no associated Chardev)
- *          or -1 on error.
+ * Returns: 0 if no associated Chardev or no data could be written,
+ *          a positive value indicating the number of bytes consumed,
+ *          or a negative %errno indicating a definite error.
  */
 int qemu_chr_fe_write(CharFrontend *c, const uint8_t *buf, int len);
 
@@ -254,8 +255,8 @@ int qemu_chr_fe_write(CharFrontend *c, const uint8_t *buf, int len);
  * this function will block if the back end cannot consume all of the data
  * attempted to be written.  This function is thread-safe.
  *
- * Returns: the number of bytes consumed (0 if no associated Chardev)
- *          or -1 on error.
+ * Returns: 0 if no associated Chardev or no data could be written,
+ *          @len on success or a negative %errno indicating a definite error.
  */
 int qemu_chr_fe_write_all(CharFrontend *c, const uint8_t *buf, int len);
 
-- 
2.51.0

[PATCH v3 09/10] hw/char: Simplify when qemu_chr_fe_write() could not write

[Philippe Mathieu-Daudé]

If no chars were written, avoid to access the FIFO.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/char/cadence_uart.c | 2 +-
 hw/char/ibex_uart.c    | 2 +-
 hw/char/sifive_uart.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index 0dfa356b6d0..8908ebbe34a 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -316,7 +316,7 @@ static gboolean cadence_uart_xmit(void *do_not_use, GIOCondition cond,
 
     ret = qemu_chr_fe_write(&s->chr, s->tx_fifo, s->tx_count);
 
-    if (ret >= 0) {
+    if (ret > 0) {
         s->tx_count -= ret;
         memmove(s->tx_fifo, s->tx_fifo + ret, s->tx_count);
     }
diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
index d6f0d18c777..b7843c7a741 100644
--- a/hw/char/ibex_uart.c
+++ b/hw/char/ibex_uart.c
@@ -161,7 +161,7 @@ static gboolean ibex_uart_xmit(void *do_not_use, GIOCondition cond,
 
     ret = qemu_chr_fe_write(&s->chr, s->tx_fifo, s->tx_level);
 
-    if (ret >= 0) {
+    if (ret > 0) {
         s->tx_level -= ret;
         memmove(s->tx_fifo, s->tx_fifo + ret, s->tx_level);
     }
diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
index e7357d585a1..e5b381425a9 100644
--- a/hw/char/sifive_uart.c
+++ b/hw/char/sifive_uart.c
@@ -83,7 +83,7 @@ static gboolean sifive_uart_xmit(void *do_not_use, GIOCondition cond,
                                    fifo8_num_used(&s->tx_fifo), &numptr);
     ret = qemu_chr_fe_write(&s->chr, characters, numptr);
 
-    if (ret >= 0) {
+    if (ret > 0) {
         /* We wrote the data, actually pop the fifo */
         fifo8_pop_bufptr(&s->tx_fifo, ret, NULL);
     }
-- 
2.51.0

[PATCH v3 10/10] system/replay: Rename some method parameters

[Philippe Mathieu-Daudé]

replay_char_read_all_save_error() saves %errno, so rename as @errcode,
replay_char_write_event_save() saves the number of bytes written, so
rename as @nbytes (same for replay_char_write_event_load).

Use the forward declaration in both replay_register_char_driver() and
replay_chr_be_write() declarations, matching their definitions.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/system/replay.h | 16 ++++++++--------
 replay/replay-char.c    | 14 +++++++-------
 stubs/replay-tools.c    | 10 +++++-----
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/include/system/replay.h b/include/system/replay.h
index 68f91bdfbf2..56d2aebac7f 100644
--- a/include/system/replay.h
+++ b/include/system/replay.h
@@ -136,18 +136,18 @@ uint64_t blkreplay_next_id(void);
 
 /* Character device */
 
-/*! Registers char driver to save it's events */
-void replay_register_char_driver(struct Chardev *chr);
-/*! Saves write to char device event to the log */
-void replay_chr_be_write(struct Chardev *s, const uint8_t *buf, int len);
+/*! Registers @chr char driver to save it's events */
+void replay_register_char_driver(Chardev *chr);
+/*! Saves write to char device @s event to the log */
+void replay_chr_be_write(Chardev *s, const uint8_t *buf, int len);
 /*! Writes char write return value to the replay log. */
-void replay_char_write_event_save(int res, int offset);
+void replay_char_write_event_save(int nbytes, int offset);
 /*! Reads char write return value from the replay log. */
-void replay_char_write_event_load(int *res, int *offset);
+void replay_char_write_event_load(int *nbytes, int *offset);
 /*! Reads information about read_all character event. */
 int replay_char_read_all_load(uint8_t *buf);
-/*! Writes character read_all error code into the replay log. */
-void replay_char_read_all_save_error(int res);
+/*! Writes character read_all error code @errcode into the replay log. */
+void replay_char_read_all_save_error(int errcode);
 /*! Writes character read_all execution result into the replay log. */
 void replay_char_read_all_save_buf(uint8_t *buf, int offset);
 
diff --git a/replay/replay-char.c b/replay/replay-char.c
index 81dc416e988..023159b155f 100644
--- a/replay/replay-char.c
+++ b/replay/replay-char.c
@@ -93,23 +93,23 @@ void *replay_event_char_read_load(void)
     return event;
 }
 
-void replay_char_write_event_save(int res, int offset)
+void replay_char_write_event_save(int nbytes, int offset)
 {
     g_assert(replay_mutex_locked());
 
     replay_save_instructions();
     replay_put_event(EVENT_CHAR_WRITE);
-    replay_put_dword(res);
+    replay_put_dword(nbytes);
     replay_put_dword(offset);
 }
 
-void replay_char_write_event_load(int *res, int *offset)
+void replay_char_write_event_load(int *nbytes, int *offset)
 {
     g_assert(replay_mutex_locked());
 
     replay_account_executed_instructions();
     if (replay_next_event_is(EVENT_CHAR_WRITE)) {
-        *res = replay_get_dword();
+        *nbytes = replay_get_dword();
         *offset = replay_get_dword();
         replay_finish_event();
     } else {
@@ -138,13 +138,13 @@ int replay_char_read_all_load(uint8_t *buf)
     }
 }
 
-void replay_char_read_all_save_error(int res)
+void replay_char_read_all_save_error(int errcode)
 {
     g_assert(replay_mutex_locked());
-    assert(res < 0);
+    assert(errcode < 0);
     replay_save_instructions();
     replay_put_event(EVENT_CHAR_READ_ALL_ERROR);
-    replay_put_dword(res);
+    replay_put_dword(errcode);
 }
 
 void replay_char_read_all_save_buf(uint8_t *buf, int offset)
diff --git a/stubs/replay-tools.c b/stubs/replay-tools.c
index c537485f401..df21475cec5 100644
--- a/stubs/replay-tools.c
+++ b/stubs/replay-tools.c
@@ -49,21 +49,21 @@ void replay_mutex_unlock(void)
 {
 }
 
-void replay_register_char_driver(struct Chardev *chr)
+void replay_register_char_driver(Chardev *chr)
 {
 }
 
-void replay_chr_be_write(struct Chardev *s, const uint8_t *buf, int len)
+void replay_chr_be_write(Chardev *s, const uint8_t *buf, int len)
 {
     abort();
 }
 
-void replay_char_write_event_save(int res, int offset)
+void replay_char_write_event_save(int nbytes, int offset)
 {
     abort();
 }
 
-void replay_char_write_event_load(int *res, int *offset)
+void replay_char_write_event_load(int *nbytes, int *offset)
 {
     abort();
 }
@@ -73,7 +73,7 @@ int replay_char_read_all_load(uint8_t *buf)
     abort();
 }
 
-void replay_char_read_all_save_error(int res)
+void replay_char_read_all_save_error(int errcode)
 {
     abort();
 }
-- 
2.51.0

Re: [PATCH v3 00/10] chardev: Improve @docstring and clarify qemu_chr_write() uses

[Philippe Mathieu-Daudé]

Missing review: 1, 2, 6, 8, 10

On 12/11/25 14:41, Philippe Mathieu-Daudé wrote:
> v3:
> - Addressed Marc-André & Daniel comments
> - Document qemu_chr_fe_write()
> - Rename replay arguments


> Philippe Mathieu-Daudé (10):
>    chardev/char-fe: Improve @docstrings
>    chardev/char-io: Add @docstrings for io_channel_send[_full]()
>    chardev/char: Improve ChardevClass::chr_write() docstring
>    chardev/char: Document qemu_chr_write[_all]()
>    chardev/char-pty: Do not ignore chr_write() failures
>    chardev/char: Allow partial writes in qemu_chr_write()
>    chardev/char: Preserve %errno in qemu_chr_write()
>    chardev/char: Document qemu_chr_fe_write()'s return value
>    hw/char: Simplify when qemu_chr_fe_write() could not write
>    system/replay: Rename some method parameters

Re: [PATCH v3 01/10] chardev/char-fe: Improve @docstrings

[Daniel P. Berrangé]

On Wed, Nov 12, 2025 at 02:41:34PM +0100, Philippe Mathieu-Daudé wrote:
> Describe the @c (this is the *frontend*) and @s (the *backend*)
> parameters. Fill qemu_chr_fe_[gs]et_msgfds() method docstrings.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/chardev/char-fe.h | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
> index 5f8a6df17dc..36bcf1f88a6 100644
> --- a/include/chardev/char-fe.h
> +++ b/include/chardev/char-fe.h

>  
>  /**
>   * qemu_chr_fe_get_msgfd:
> + * @c: the character frontend to access
>   *
>   * For backends capable of fd passing, return the latest file descriptor passed
>   * by a client.
> @@ -286,9 +302,12 @@ int qemu_chr_fe_get_msgfd(CharFrontend *c);

We should note here that if there is more than 1 pending FD,
all except the first pending FD will be closed

>  
>  /**
>   * qemu_chr_fe_get_msgfds:
> + * @c: the character frontend
> + * @fds: an array of ancillary file descriptors to get
> + * @num: the maximum number of ancillary file descriptors to get in @fds
>   *
>   * For backends capable of fd passing, return the number of file received
> - * descriptors and fills the fds array up to num elements
> + * descriptors and fills the fds array up to @num elements

Here we should note that if there are more pending FDs than @num,
then all further FDs will be closed.

>   *
>   * Returns: -1 if fd passing isn't supported or there are no pending file
>   *          descriptors.  If file descriptors are returned, subsequent calls to

Those are minor changes, so consider this tohave

  Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 v3 06/10] chardev/char: Allow partial writes in qemu_chr_write()

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1351 bytes --]

Hi

On Wed, Nov 12, 2025 at 5:42 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> If qemu_chr_write_buffer() returned an error, but could
> write some characters, return the number of character
> written. Otherwise frontends able to recover and resume
> writes re-write the partial chars already written.
>
> Cc: qemu-stable@nongnu.org
> Suggested-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  chardev/char.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 105b0d53184..7931f4e0832 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -186,6 +186,11 @@ int qemu_chr_write(Chardev *s, const uint8_t *buf,
> int len, bool write_all)
>
>      res = qemu_chr_write_buffer(s, buf, len, &offset, write_all);
>
> +    if (!write_all && res < 0 && offset == 0) {
> +        /* Allow partial writes */
> +        return res;
> +    }
> +
>

I don't understand this, as below it has:
    if (res < 0) {
        return res;
    }

Is it just the final return value that should be changed? "offset &&
!write_all ? offset : res" ?

     if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_RECORD) {
>          replay_char_write_event_save(res, offset);
>      }
> --
> 2.51.0
>
>

[-- Attachment #2: Type: text/html, Size: 2202 bytes --]

Re: [PATCH v3 01/10] chardev/char-fe: Improve @docstrings

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 5673 bytes --]

On Wed, Nov 12, 2025 at 5:41 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> Describe the @c (this is the *frontend*) and @s (the *backend*)
> parameters. Fill qemu_chr_fe_[gs]et_msgfds() method docstrings.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>

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


> ---
>  include/chardev/char-fe.h | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
> index 5f8a6df17dc..36bcf1f88a6 100644
> --- a/include/chardev/char-fe.h
> +++ b/include/chardev/char-fe.h
> @@ -26,6 +26,8 @@ struct CharFrontend {
>
>  /**
>   * qemu_chr_fe_init:
> + * @c: the character frontend
> + * @s: the character backend
>   *
>   * Initializes the frontend @c for the given Chardev backend @s. Call
>   * qemu_chr_fe_deinit() to remove the association and release the backend.
> @@ -47,6 +49,7 @@ void qemu_chr_fe_deinit(CharFrontend *c, bool del);
>
>  /**
>   * qemu_chr_fe_get_driver:
> + * @c: the character frontend
>   *
>   * Returns: the driver associated with a CharFrontend or NULL if no
>   * associated Chardev.
> @@ -58,6 +61,7 @@ Chardev *qemu_chr_fe_get_driver(CharFrontend *c);
>
>  /**
>   * qemu_chr_fe_backend_connected:
> + * @c: the character frontend
>   *
>   * Returns: true if there is a backend associated with @c.
>   */
> @@ -102,6 +106,7 @@ void qemu_chr_fe_set_handlers_full(CharFrontend *c,
>
>  /**
>   * qemu_chr_fe_set_handlers:
> + * @c: the character frontend
>   *
>   * Version of qemu_chr_fe_set_handlers_full() with sync_state = true.
>   */
> @@ -116,6 +121,7 @@ void qemu_chr_fe_set_handlers(CharFrontend *c,
>
>  /**
>   * qemu_chr_fe_take_focus:
> + * @c: the character frontend
>   *
>   * Take the focus (if the front end is muxed).
>   *
> @@ -125,6 +131,7 @@ void qemu_chr_fe_take_focus(CharFrontend *c);
>
>  /**
>   * qemu_chr_fe_accept_input:
> + * @c: the character frontend
>   *
>   * Notify that the frontend is ready to receive data
>   */
> @@ -132,6 +139,7 @@ void qemu_chr_fe_accept_input(CharFrontend *c);
>
>  /**
>   * qemu_chr_fe_disconnect:
> + * @c: the character frontend
>   *
>   * Close a fd accepted by character backend.
>   * Without associated Chardev, do nothing.
> @@ -148,6 +156,7 @@ int qemu_chr_fe_wait_connected(CharFrontend *c, Error
> **errp);
>
>  /**
>   * qemu_chr_fe_set_echo:
> + * @c: the character frontend
>   * @echo: true to enable echo, false to disable echo
>   *
>   * Ask the backend to override its normal echo setting.  This only really
> @@ -169,6 +178,7 @@ void qemu_chr_fe_set_open(CharFrontend *c, bool
> is_open);
>
>  /**
>   * qemu_chr_fe_printf:
> + * @c: the character frontend
>   * @fmt: see #printf
>   *
>   * Write to a character backend using a printf style interface.  This
> @@ -197,6 +207,7 @@ typedef gboolean (*FEWatchFunc)(void *do_not_use,
> GIOCondition condition, void *
>
>  /**
>   * qemu_chr_fe_add_watch:
> + * @c: the character frontend
>   * @cond: the condition to poll for
>   * @func: the function to call when the condition happens
>   * @user_data: the opaque pointer to pass to @func
> @@ -219,6 +230,7 @@ guint qemu_chr_fe_add_watch(CharFrontend *c,
> GIOCondition cond,
>
>  /**
>   * qemu_chr_fe_write:
> + * @c: the character frontend to write to
>   * @buf: the data
>   * @len: the number of bytes to send
>   *
> @@ -233,6 +245,7 @@ int qemu_chr_fe_write(CharFrontend *c, const uint8_t
> *buf, int len);
>
>  /**
>   * qemu_chr_fe_write_all:
> + * @c: the character frontend to write to
>   * @buf: the data
>   * @len: the number of bytes to send
>   *
> @@ -248,6 +261,7 @@ int qemu_chr_fe_write_all(CharFrontend *c, const
> uint8_t *buf, int len);
>
>  /**
>   * qemu_chr_fe_read_all:
> + * @c: the character frontend to read from
>   * @buf: the data buffer
>   * @len: the number of bytes to read
>   *
> @@ -260,6 +274,7 @@ int qemu_chr_fe_read_all(CharFrontend *c, uint8_t
> *buf, int len);
>
>  /**
>   * qemu_chr_fe_ioctl:
> + * @c: the character frontend to control
>   * @cmd: see CHR_IOCTL_*
>   * @arg: the data associated with @cmd
>   *
> @@ -273,6 +288,7 @@ int qemu_chr_fe_ioctl(CharFrontend *c, int cmd, void
> *arg);
>
>  /**
>   * qemu_chr_fe_get_msgfd:
> + * @c: the character frontend to access
>   *
>   * For backends capable of fd passing, return the latest file descriptor
> passed
>   * by a client.
> @@ -286,9 +302,12 @@ int qemu_chr_fe_get_msgfd(CharFrontend *c);
>
>  /**
>   * qemu_chr_fe_get_msgfds:
> + * @c: the character frontend
> + * @fds: an array of ancillary file descriptors to get
> + * @num: the maximum number of ancillary file descriptors to get in @fds
>   *
>   * For backends capable of fd passing, return the number of file received
> - * descriptors and fills the fds array up to num elements
> + * descriptors and fills the fds array up to @num elements
>   *
>   * Returns: -1 if fd passing isn't supported or there are no pending file
>   *          descriptors.  If file descriptors are returned, subsequent
> calls to
> @@ -299,6 +318,9 @@ int qemu_chr_fe_get_msgfds(CharFrontend *c, int *fds,
> int num);
>
>  /**
>   * qemu_chr_fe_set_msgfds:
> + * @c: the character frontend
> + * @fds: an array of ancillary file descriptors to set
> + * @num: the number of ancillary file descriptors to set
>   *
>   * For backends capable of fd passing, set an array of fds to be passed
> with
>   * the next send operation.
> --
> 2.51.0
>
>

[-- Attachment #2: Type: text/html, Size: 6677 bytes --]

Re: [PATCH v3 02/10] chardev/char-io: Add @docstrings for io_channel_send[_full]()

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1458 bytes --]

On Wed, Nov 12, 2025 at 5:42 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>

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


> ---
>  include/chardev/char-io.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/include/chardev/char-io.h b/include/chardev/char-io.h
> index ac379ea70e3..5b181be6a0b 100644
> --- a/include/chardev/char-io.h
> +++ b/include/chardev/char-io.h
> @@ -38,8 +38,28 @@ GSource *io_add_watch_poll(Chardev *chr,
>
>  void remove_fd_in_watch(Chardev *chr);
>
> +/**
> + * io_channel_send:
> + * @ioc: the IO channel object
> + * @buf: the data
> + * @len: the number of bytes to send
> + *
> + * Returns: the number of bytes consumed or -1 on error.
> + * On error, %errno is also set as appropriate.
> + */
>  int io_channel_send(QIOChannel *ioc, const void *buf, size_t len);
>
> +/**
> + * io_channel_send_full:
> + * @ioc: the IO channel object
> + * @buf: the data
> + * @len: the number of bytes to send
> + * @fds: an array of file handles to send
> + * @nfds: number of file handles in @fds
> + *
> + * Returns: the number of bytes consumed or -1 on error.
> + * On error, %errno is also set as appropriate.
> + */
>  int io_channel_send_full(QIOChannel *ioc, const void *buf, size_t len,
>                           int *fds, size_t nfds);
>
> --
> 2.51.0
>
>

[-- Attachment #2: Type: text/html, Size: 2163 bytes --]

Re: [PATCH v3 08/10] chardev/char: Document qemu_chr_fe_write()'s return value

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1780 bytes --]

On Wed, Nov 12, 2025 at 5:42 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>

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


> ---
>  include/chardev/char-fe.h | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
> index 36bcf1f88a6..fbec6a73dbb 100644
> --- a/include/chardev/char-fe.h
> +++ b/include/chardev/char-fe.h
> @@ -238,8 +238,9 @@ guint qemu_chr_fe_add_watch(CharFrontend *c,
> GIOCondition cond,
>   * will send data from the front end to the back end.  This function
>   * is thread-safe.
>   *
> - * Returns: the number of bytes consumed (0 if no associated Chardev)
> - *          or -1 on error.
> + * Returns: 0 if no associated Chardev or no data could be written,
> + *          a positive value indicating the number of bytes consumed,
> + *          or a negative %errno indicating a definite error.
>   */
>  int qemu_chr_fe_write(CharFrontend *c, const uint8_t *buf, int len);
>
> @@ -254,8 +255,8 @@ int qemu_chr_fe_write(CharFrontend *c, const uint8_t
> *buf, int len);
>   * this function will block if the back end cannot consume all of the data
>   * attempted to be written.  This function is thread-safe.
>   *
> - * Returns: the number of bytes consumed (0 if no associated Chardev)
> - *          or -1 on error.
> + * Returns: 0 if no associated Chardev or no data could be written,
> + *          @len on success or a negative %errno indicating a definite
> error.
>   */
>  int qemu_chr_fe_write_all(CharFrontend *c, const uint8_t *buf, int len);
>
> --
> 2.51.0
>
>

[-- Attachment #2: Type: text/html, Size: 2553 bytes --]