Peter Xu <pet...@redhat.com> writes: > Previously we cleanup the queues when we got CLOSED event. It was used
we clean up > to make sure we won't leftover replies/events of a old client to a new we won't send leftover replies/events of an old client > client. Now this patch postpones that until OPENED. What if OPENED never comes? > In most cases, it does not really matter much since either way will make > sure that the new client won't receive unexpected old events/responses. > However there can be a very rare race condition with the old way when we > use QMP with stdio and pipelines (which is quite common in iotests). > The problem is that, we can easily have something like this in scripts: > > cat $QMP_COMMANDS | qemu -qmp stdio ... | filter_commands > > In most cases, a QMP session will be based on a bidirectional > channel (read/write to a TCP port, for example), so IN and OUT are > sharing some fundamentally same thing. However that's untrue for pipes Suggest "are fundamentally the same thing". > above - the IN is the "cat" program, while the "OUT" is directed to the > "filter_commands" which is actually another process. Regarding 'IN is the "cat" program': a pipe is not a program. Suggest 'IN is connected to "cat", while OUT is connected to "filter_commands", which are separate processes. > Now when we received the CLOSED event, we cleanup the queues (including we clean up > the QMP response queue). That means, if we kill/stop the "cat" process > faster than the filter_commands process, the filter_commands process is > possible to miss some responses/events that should belong to it. I'm not sure I understand the error scenario. Can you explain it in more concrete terms? Like "cat terminates, QEMU reads EOF, QEMU does this, QEMU does that, oops, it shouldn't have done that". > In practise, I encountered a very strange SHUTDOWN event missing when practice May I suggest use of a spell checker? ;) > running test with iotest 087. Without this patch, iotest 078 will have 087 or 078? > ~10% chance to miss the SHUTDOWN event and fail when with Out-Of-Band > enabled: > > 087 8s ... - output mismatch (see 087.out.bad)