* Yury Kotov (yury-ko...@yandex-team.ru) wrote: > Hi! > > 20.12.2019, 19:09, "Markus Armbruster" <arm...@redhat.com>: > > Yury Kotov <yury-ko...@yandex-team.ru> writes: > > > >> Hi, > >> > >> This series is continuation of another one: > >> [PATCH] monitor: Fix slow reading > >> https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg03722.html > >> > >> Which also tried to read more than one byte from a stream at a time, > >> but had some problems with OOB and HMP: > >> https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg05018.html > >> > >> This series is an attempt to fix problems described. > > > > Two problems: (1) breaks HMP migrate -d, and (2) need to think through > > how this affects reading of QMP input, in particular OOB. > > > > This series refrains from changing HMP, thus avoids (1). Good. > > > > What about (2)? I'm feeling denser than usual today... Can you explain > > real slow how QMP input works? PATCH 2 appears to splice in a ring > > buffer. Why is that needed? > > Yes, the second patch introduced the input ring buffer to store remaining > bytes while monitor is suspended. > > QMP input scheme: > 1. monitor_qmp_can_read returns a number of bytes, which it's ready to > receive. > Currently it returns 0 (if suspended) or 1 otherwise. > In my patch: monitor_qmp_can_read returns a free size of the introduced > ring buffer. > > 2. monitor_qmp_read receives and handles input bytes > Currently it just puts received bytes into a json lexer. > If monitor is suspended this function won't be called and thus it won't > process new command until monitor resume. > In my patch: monitor_qmp_read stores input bytes into the buffer and then > handles bytes in the buffer one by one while monitor is not suspended. > So, it allows to be sure that the original logic is preserved and > we won't handle new commands while monitor is suspended. > > 3. monitor_resume schedules monitor_accept_input which calls > monitor_qmp_handle_inbuf which tries to handle remaining bytes > in the buffer. monitor_accept_input is a BH scheduled by monitor_resume > on monitor's aio context. It is needed to be sure, that we access > the input buffer only in monitor's context. > > Example: > 1. QMP read 100 bytes > 2. Handle some command in the first 60 bytes > 3. For some reason, monitor becomes suspended after the first command > 4. 40 bytes are remaining > 5. After a while, something calls monitor_resume which handles > the remaining bytes in the buffer (implicitly: resume -> sched bh -> buf) > > Actually, QMP continues to receive data even though the monitor is suspended > until the buffer is full. But it doesn't process received data.
I *think* that's OK for OOB; my reading is that prior to this set of patches, if you filled the queue (even with oob enabled) you could suspend the monitor and block - but you're just not supposed to be throwing commands quickly at an OOB monitor; but I'm cc'ing in Peter. Dave > Regards, > Yury > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
