Re: [PATCH v3 1/3] char: qemu_chr_write_log() use qemu_write_full()

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Feb 19, 2026 at 09:07:07PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 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

IMHO, use of FDs in non-blocking mode should always be an explicit
opt-in decision by the code in question.  So my preference is to
ensure FDs coming via -add-fd get forced into blocking mode, the
same as we do for FDs coming via the monitor (or any socket recv
when QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING is NOT set)

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|