Re: [PATCH v2 2/6] accel: accel_available() function

[Markus Armbruster]

Claudio Fontana <cfontana@suse.de> writes:

> On 11/27/20 3:45 PM, Markus Armbruster wrote:
>> Claudio Fontana <cfontana@suse.de> writes:
>> 
>>> On 11/26/20 10:48 PM, Eduardo Habkost wrote:
>>>> On Thu, Nov 26, 2020 at 10:06:03PM +0100, Claudio Fontana wrote:
>>>>> On 11/26/20 3:25 PM, Paolo Bonzini wrote:
>>>>>> On 26/11/20 15:13, Claudio Fontana wrote:
>>>>>>> One option I see is simply to document the behavior where
>>>>>>> accel_available() is declared in accel.h (ie do not use in fast
>>>>>>> path), as well as in accel_find() actually, so that both accel_find()
>>>>>>> and accel_available() are avoided in fast path and avoid being called
>>>>>>> frequently at runtime.
>>>>>>>
>>>>>>> Another option could be to remove the allocation completely, and use
>>>>>>> for example accel_find(ACCEL_CLASS_NAME("tcg")), or another option
>>>>>>> again would be to remove the allocation and use either a fixed buffer
>>>>>>> + snprintf, or alloca -like builtin code to use the stack, ...
>>>>>>>
>>>>>>> Not a big deal, but with a general utility and short name like
>>>>>>> accel_available(name) it might be tempting to use this more in the
>>>>>>> future?
>>>>>>
>>>>>> I think it's just that the usecase is not that common.  "Is this 
>>>>>> accelerator compiled in the binary" is not something you need after 
>>>>>> startup (or if querying the monitor).
>>>>>>
>>>>>> Paolo
>>>>>>
>>>>>>
>>>>>
>>>>> A script that repeatedly uses the QMP interface to query for
>>>>> the status could generate fragmentation this way I think.
>>>>
>>>> Is this a problem?  Today, execution of a "query-kvm" command
>>>> calls g_malloc() 37 times.
>>>>
>>>
>>> Not ideal in my view, but not the end of the world either.
>> 
>> QMP's appetite for malloc is roughly comparable to a pig's for truffles.
>> 
>
> :-)
>
> Btw, do we have limits on the maximum size of these objects? I mean, a single 
> QMP command,
> a single QEMU object type name, etc?
>
> In this case we could do some overall improvement there, and might even avoid 
> some problems down the road..

We have limits, but they are not comprehensive.

The QMP client is trusted.  We don't try to guard against a malicious
QMP client.  We do try to guard against mistakes.

The JSON parser limits token size (in characters), expression size (in
tokens), and expression nesting depth.  This protects against a
malfunctioning QMP client.  The limits are ridiculously generous.

The QMP core limits the number of commands in flight per monitor to a
somewhat parsimonious 8-9 in-band commands, plus one out-of-band
command.  This protects against a QMP client sending commands faster
than we can execute them.

QMP output is buffered without limit.  When a (malfunctioning) QMP
client keeps sending commands without reading their output, QEMU keeps
buffering until it runs out of memory and crashes.