On Tue, 11 Mar 2025 at 10:33, Philippe Mathieu-Daudé <phi...@linaro.org> wrote: > On 10/3/25 18:28, Peter Maydell wrote: > > This seems to be because the pl011 code and the chardev > > code disagree about how "couldn't write anything" is > > reported. pl011 here is looking for "0 means wrote nothing", > > but the chardev code reports it as "-1 and errno is EAGAIN". > > > > I think the chardev code is actually what we need to fix here, > > because it makes basically no effort to guarantee that the > > errno from the underlying write is still in 'errno' by the > > time qemu_chr_fe_write() returns. In particular it may > > call qemu_chr_write_log() or replay_char_write_event_save(), > > both of which will happily trash errno if something fails > > during their execution. > > IIUC when retrying qemu_chr_write_buffer(s, buf, len, ofs) could > write less than @len (but still writing few bytes, that information > is stored in @offset) and return -errno, discarding @offset partial > write len.
I thought that too when I first read the code, because it's been written in a way that didn't match what I was expecting for a "write until you would block, then stop" loop, but on second reading I decided it could not, at least in the case where write_all is false. If we write any data at all on the first cc->chr_write call, we will then break out of the loop without trying to send any more, so there's no code path that will then set res to a negative number. (If write_all is true, then yeah I think we can return an errno rather than a partial-write count if chr_write fails on the second time through the loop, but all the callers of that version of the function ignore errors of any kind anyway, and certainly don't have any means to handle "only wrote half the data", otherwise they'd be using the non-blocking version.) -- PMM