On Thu, Jan 26, 2017 at 05:07:16PM -0800, Ed Swierk wrote: > Currently qemu_chr_fe_write() calls qemu_chr_fe_write_log() only for > data consumed by the backend chr_write function. With the pty backend, > pty_chr_write() returns 0 indicating that the data was not consumed > when the pty is disconnected. Simply changing it to return len instead > of 0 tricks the caller into logging the data even when the pty is > disconnected. I don't know what problems this might cause, but one > data point is that tcp_chr_write() already happens to work this way. > > Alternatively, qemu_chr_fe_write() could be modified to log everything > passed to it, regardless of how much data chr_write claims to have > consumed. The trouble is that the serial device retries writing > unconsumed data, so when the pty is disconnected you'd see every > character duplicated 4 times in the log file. > > Any opinions on either approach, or other suggestions? If there are no > objections to the first one, I'll prepare a patch. If the pty backend intends to just drop data into a blackhole when no client is connected, then its chr_write() impl should return the length of the data discarded, not zero.

On Mon, Jan 30, 2017 at 08:58:25PM -0800, Ed Swierk wrote: > On Fri, Jan 27, 2017 at 1:40 AM, Daniel P. Berrange <berrange(a)redhat.com&gt; wrote: >> On Thu, Jan 26, 2017 at 05:07:16PM -0800, Ed Swierk wrote: >>> Currently qemu_chr_fe_write() calls qemu_chr_fe_write_log() only for >>> data consumed by the backend chr_write function. With the pty backend, >>> pty_chr_write() returns 0 indicating that the data was not consumed >>> when the pty is disconnected. Simply changing it to return len instead >>> of 0 tricks the caller into logging the data even when the pty is >>> disconnected. I don't know what problems this might cause, but one >>> data point is that tcp_chr_write() already happens to work this way. >>> >>> Alternatively, qemu_chr_fe_write() could be modified to log everything >>> passed to it, regardless of how much data chr_write claims to have >>> consumed. The trouble is that the serial device retries writing >>> unconsumed data, so when the pty is disconnected you'd see every >>> character duplicated 4 times in the log file. >>> >>> Any opinions on either approach, or other suggestions? If there are no >>> objections to the first one, I'll prepare a patch. >> >> If the pty backend intends to just drop data into a blackhole when >> no client is connected, then its chr_write() impl should return >> the length of the data discarded, not zero. > > That's exactly the question: when no client is connected, should the > pty backend just drop the data into a black hole, returning the length > of the data discarded? Or should it return 0, letting the frontend > device decide what to do with it? It should return len of data discarded. > > I can't discern a consistent pattern across all the char backends. The > closest analog is the tcp backend, which does discard the data and > return len. In contrast, several backends call > io_channel_send{,_full}(), which returns -1 if the write would block > or fails for any other reason. > > It's not clear there's much the frontend can do to recover from an > error, but there's no consistent pattern across serial devices either. > Most just ignore the return value. But the 16550A serial device > retries 4 times after an error. Changing the pty backend to discard > the data on the first attempt would bypass this retry mechanism. Is > that a problem? I don't think so - retrying in this way is pointless IMHO - it is just going to get the same result on every retry on 99% of occassions.