* Daniel P. Berrange (berra...@redhat.com) wrote: > On Tue, May 02, 2017 at 05:36:30PM +0100, Dr. David Alan Gilbert wrote: > > * Markus Armbruster (arm...@redhat.com) wrote: > > > "Denis V. Lunev" <d...@openvz.org> writes: > > > > > > > On 05/02/2017 05:43 PM, Markus Armbruster wrote: > > > >> "Denis V. Lunev" <d...@openvz.org> writes: > > > >> > > > >>> Right now QMP and HMP monitors read 1 byte at a time from the socket, > > > >>> which > > > >>> is very inefficient. With 100+ VMs on the host this easily reasults in > > > >>> a lot of unnecessary system calls and CPU usage in the system. > > > >>> > > > >>> This patch changes the amount of data to read to 4096 bytes, which > > > >>> matches > > > >>> buffer size on the channel level. Fortunately, monitor protocol is > > > >>> synchronous right now thus we should not face side effects in reality. > > > >> Can you explain briefly why this relies on "synchronous"? I've spent > > > >> all of two seconds on the question myself... > > > > Each command is processed in sequence as it appears in the > > > > channel. The answer to the command is sent and only after that > > > > next command is processed. > > > > > > Yes, that's how QMP works. > > > > > > > Theoretically tith asynchronous processing we can have some side > > > > effects due to changed buffer size. > > > > > > What kind of side effects do you have in mind? > > > > > > It's quite possible that this obviously inefficient way to read had some > > > deep reason back when it was created. Hmm, git-blame is our friend: > > > > > > commit c62313bbdc48f72e93fa8196f2fff96ba35e4e9d > > > Author: Jan Kiszka <jan.kis...@siemens.com> > > > Date: Fri Dec 4 14:05:29 2009 +0100 > > > > > > monitor: Accept input only byte-wise > > > > > > This allows to suspend command interpretation and execution > > > synchronously, e.g. during migration. > > > > > > Signed-off-by: Jan Kiszka <jan.kis...@siemens.com> > > > Signed-off-by: Anthony Liguori <aligu...@us.ibm.com> > > > > I don't think I understand why that's a problem; if we read more bytes, > > we're not going to interpret them and execute them until after the previous > > command returns are we? > > Actually it sees we might do, due to the way the "migrate" command works > in HMP when you don't give the '-d' flag. > > Most monitors commands will block the caller until they are finished, > but "migrate" is different. The hmp_migrate() method will return > immediately, but we call monitor_suspend() to block processing of > further commands. If another command has already been read off > the wire though (due to "monitor_read" having a buffer that contains > multiple commands), we would in fact start processing this command > despite having suspended the monitor.
Ah OK; yes that's painful. > This is only a problem, however, if the client app has issued "migrate" > followed by another command, at the same time without waiting for the > respond to "migrate". So in practice the only way you'd hit the bug > is probably if you just cut+paste a big chunk of commands into the > monitor at once without waiting for completion and one of the commands > was "migrate" without "-d". That's probably not unusual for scripts; doing something like: migrate .... info migrate isn't that unusual. > Still, I think we would need to figure out a proper fix for this before > we could increase the buffer size. Nod, it's a bit grim. Dave > > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
