Re: [PATCH v3 2/4] python/qmp: increase read buffer size

[Daniel P . Berrangé]

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-max@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-max@yandex-team.ru>
> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@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.

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....

Create a 200 deep chain of qcow2 files

   qemu-img create -f qcow2 demo0.img 1G
   j=0
   for i in `seq 1 200`
   do
     qemu-img create -f qcow2 -o backing_file=demo$j.qcow2 \
                              -o backing_fmt=qcow2 demo$i.qcow2
     j=$i
   done

Launch qemu with that, and then run

  "query-named-block-nodes"

and (many minutes later) you'll get an 86 MB  QMP reply.

Now, a bare no-arg "query-named-block-nodes" is known to be a bad
command as it returns data which is a combinatorial expansion of
the number of block nodes.  Essentially it is broken by design,
and no one should use it, without setting flat=true.


If we repeat it with flat=true, then we can get a 2.7 MB QMP
reply.  This is large, but a valid reply. Basically 13 KB of
data per qcow2 file.

Libvirt caps the backing chain depth it is willing to accept at 200
qcow2 files, but allows multiple disks. So with 4 disks, each with
200 deep chain, you'll get 10.8 MB of json back. Opps....

...Libvirt's QMP reply limit is 10 MB, so even libvirt will break
quite quickly. Libvirt can cope with around 787 qcow2 files in
at 13 kb per file. NB I'm assuming paths under the dir
/var/lib/libvirt/images/, shorter paths will allow for more.


The 64 KB limit will only cope with 4 qcow2 files before breaking,
while a 512 KB limit will only cope with 39 files before breaking.
Neither is anywhere near sufficient.

I'd suggest following libvirt here and setting the read limits to
10 MB.

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 :|