On 02.02.26 14:26, Daniel P. Berrangé wrote:
On Sun, Feb 01, 2026 at 08:36:29PM +0300, Vladimir Sementsov-Ogievskiy wrote:
logfd is blocking, so we don't need to care about EAGAIN.
Let's simply use qemu_write_full().
Are you sure logfd is always blocking ? It can be an FD passed in
from outside QEMU, and off-hand, I'm not sure where we force that to
be blocking. Though we could argue it is the client't fault if they
passed a non-blocking FD to QEMU.
NB, we can *not* assume logfd is a plain file as libvirt will pass
in a pipe to send logging via virtlogd.
I'm sorry I missed the email. And the commit is in master now.
And, I missed the fact that
qemu_create(common->logfile, flags, 0666, errp);
Can actually chose fd from "fdset". All named fds in monitor are set blocking
explicitly except for the case when QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING
flag
take place.. But fds coming through add-fd commandline options are not handled
this way, as I can see.
Looking through:
1. util/oslib-posix.c: qemu_write_pidfile()
Uses qemu_write_full(), *unprepared* to non-blocking.
2. ui/ui-qmp-cmds.c: qmp_screendump()
2.1. png_save() wraps the fd to fdopen(), which I assume should work well
with blocking and non-blocking fds
2.2. ppm_save() wraps it into qio_channel_file_new_fd()
qio_channel_write_all() -> qio_channel_writev_full_all() do handle
QIO_CHANNEL_ERR_BLOCK
3. qio_channel_file_new_path()
In general qio-channels are prepared to work with non-blocking fds.
qio_channel_file_new_path() is used (except for 2.2 and tests) from migration
code, which
I think should be prepared to non-blocking fds. At least, qemu_fill_buffer()
handles QIO_CHANNEL_ERR_BLOCK.
4. hw/usb/bus.c: usb_qdev_realize()
Wraps by fdopen()
5. hw/uefi/var-service-pcap.c: uefi_vars_pcap_init()
Wraps by fdopen()
6. hw/uefi/var-service-json.c: uefi_vars_json_init()
Use write() and don't handle EAGAIN: *unprepared* for non-blocking.
7. dump/dump.c: qmp_dump_guest_memory()
Use qemu_write_full() - *unprepared*
8. chardev/char.c: logfd - *unprepared* after my commit
9. block/file-posix.c: raw_co_create()
May use write() and not handle EAGAIN during raw_regular_truncate() -
unprepared.
What to do with it? I see several possibilities:
1. simply support EAGAIN in qemu_write_full()? Like it was in
qemu_chr_write_log()
- with g_usleep(100)?
2. switch to qio_channel_file_new_path() + qio_channel_write_all()
3. call qemu_set_blocking() on fd for *unprepared* cases
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
---
chardev/char.c | 22 +++++-----------------
1 file changed, 5 insertions(+), 17 deletions(-)
diff --git a/chardev/char.c b/chardev/char.c
index 3e432195a5..64006a3119 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -82,29 +82,17 @@ void qemu_chr_be_event(Chardev *s, QEMUChrEvent event)
CHARDEV_GET_CLASS(s)->chr_be_event(s, event);
}
-/* Not reporting errors from writing to logfile, as logs are
- * defined to be "best effort" only */
static void qemu_chr_write_log(Chardev *s, const uint8_t *buf, size_t len)
{
- size_t done = 0;
- ssize_t ret;
-
if (s->logfd < 0) {
return;
}
- while (done < len) {
- retry:
- ret = write(s->logfd, buf + done, len - done);
- if (ret == -1 && errno == EAGAIN) {
- g_usleep(100);
- goto retry;
- }
-
- if (ret <= 0) {
- return;
- }
- done += ret;
+ if (qemu_write_full(s->logfd, buf, len) < len) {
+ /*
+ * qemu_write_full() is defined with G_GNUC_WARN_UNUSED_RESULT,
+ * but logging is best‑effort, we do ignore errors.
+ */
}
}
--
2.52.0
With regards,
Daniel
--
Best regards,
Vladimir
