On Wed, Nov 9, 2022, 6:00 AM Daniel P. Berrangé <berra...@redhat.com> wrote:
> On Wed, Nov 09, 2022 at 09:39:14AM +0000, Daniel P. Berrangé wrote: > > On Tue, Nov 08, 2022 at 03:38:21PM -0500, John Snow wrote: > > > On Thu, Nov 3, 2022 at 6:29 AM Maksim Davydov > > > <davydov-...@yandex-team.ru> wrote: > > > > > > > > After modification of "query-machines" command the buffer size > should be > > > > more than 452kB to contain output with compat-props. > > > > > > > > Signed-off-by: Maksim Davydov <davydov-...@yandex-team.ru> > > > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru > > > > > > --- > > > > python/qemu/qmp/qmp_client.py | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/python/qemu/qmp/qmp_client.py > b/python/qemu/qmp/qmp_client.py > > > > index 5dcda04a75..659fe4d98c 100644 > > > > --- a/python/qemu/qmp/qmp_client.py > > > > +++ b/python/qemu/qmp/qmp_client.py > > > > @@ -197,8 +197,8 @@ async def run(self, address='/tmp/qemu.socket'): > > > > #: Logger object used for debugging messages. > > > > logger = logging.getLogger(__name__) > > > > > > > > - # Read buffer limit; large enough to accept query-qmp-schema > > > > - _limit = (256 * 1024) > > > > + # Read buffer limit; large enough to accept query-machines > > > > + _limit = (512 * 1024) > > > > > > wow :) > > > > Meanwhile over in python/qemu/qmp/protocol.py the read buffer limit is > > set to just 64 kb. > This one will override the other - the protocol limit is for any arbitrary full-duplex message-based protocol. It can stay at the lower limit. (I used protocol.py to implement a qtest client as well, though I didn't upstream that piece. If there's interest, I will.) > > > If the current output of a particular command is known to 450 kb, then > > setting this limit to 512 kb is waaaaaaay to conservative, and we'll > > inevitably have to change it again when someone finds the next command > > that overflows. > > > > Recall this thread > > > > https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg01060.html > > > > In fact, let me be the someone who demonstrates a real case where 512kb > > is not enough.... > > Another example... > > Create a guest with 255 vCPUs (current RHEL downstream vCPU limit), > and run > > {"execute":"query-stats","arguments":{"target": "vcpu"}} > > it'll get back a 0.38 MB QMP reply. RHEL raised the limit to 710 > vCPUs, giving a little over 1 MB QMP reply. There is a strong desire > to go even higher. With 4096 vCPUs it'd get an ~6 MB QMP reply. > > With 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 :| > You're right, of course. I recalled the thread but I was being lazy about it. (Sorry.) I thought, naively, that it was better to speed Maksim along for now. I recall you (Daniel) stating that libvirt used a default of 10MB (iirc). I'd be happy to adopt that default as well, if only for parity. Maksim, can I trouble you to send a revised patch as an MR to gitlab.com/qemu-project/python-qemu-qmp ? If not, a revised patch to the mailing list here is fine and with your permission I'll forward-port it over to the standalone repo myself. (Or I can just handle it entirely myself, if you'd prefer - just let me know.) Sorry for the fuss, and thanks for helping to improve QMP testing and tooling --js >
