Re: Re: [PATCH v4 11/12] cryptodev: Support query-stats QMP command

From: zhenwei pi <pizhenwei@bytedance.com>
To: "Michael S. Tsirkin" <mst@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>
Cc: arei.gonglei@huawei.com, dgilbert@redhat.com,
	pbonzini@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org
Subject: Re: Re: [PATCH v4 11/12] cryptodev: Support query-stats QMP command
Date: Wed, 1 Mar 2023 10:53:18 +0800	[thread overview]
Message-ID: <cdbd5949-7377-356a-a307-0b2314e1dae9@bytedance.com> (raw)
In-Reply-To: <20230228095807-mutt-send-email-mst@kernel.org>

On 2/28/23 22:58, Michael S. Tsirkin wrote:
> On Tue, Feb 28, 2023 at 02:21:17PM +0000, Daniel P. Berrangé wrote:
>> On Tue, Feb 28, 2023 at 09:13:30AM -0500, Michael S. Tsirkin wrote:
>>> On Tue, Feb 28, 2023 at 01:17:52PM +0000, Daniel P. Berrangé wrote:
>>>> On Tue, Feb 28, 2023 at 07:56:14AM -0500, Michael S. Tsirkin wrote:
>>>>> On Sun, Jan 29, 2023 at 10:57:46AM +0800, zhenwei pi wrote:
>>>>>> Now we can use "query-stats" QMP command to query statistics of
>>>>>> crypto devices. (Originally this was designed to show statistics
>>>>>> by '{"execute": "query-cryptodev"}'. Daniel Berrangé suggested that
>>>>>> querying configuration info by "query-cryptodev", and querying
>>>>>> runtime performance info by "query-stats". This makes sense!)
>>>>>>
>>>>>> Example:
>>>>>> ~# virsh qemu-monitor-command vm '{"execute": "query-stats", \
>>>>>>     "arguments": {"target": "cryptodev"} }' | jq
>>>>>> {
>>>>>>    "return": [
>>>>>>      {
>>>>>>        "provider": "cryptodev",
>>>>>>        "stats": [
>>>>>>          {
>>>>>>            "name": "asym-verify-bytes",
>>>>>>            "value": 7680
>>>>>>          },
>>>>>>          ...
>>>>>>          {
>>>>>>            "name": "asym-decrypt-ops",
>>>>>>            "value": 32
>>>>>>          },
>>>>>>          {
>>>>>>            "name": "asym-encrypt-ops",
>>>>>>            "value": 48
>>>>>>          }
>>>>>>        ],
>>>>>>        "qom-path": "/objects/cryptodev0" # support asym only
>>>>>>      },
>>>>>>      {
>>>>>>        "provider": "cryptodev",
>>>>>>        "stats": [
>>>>>>          {
>>>>>>            "name": "asym-verify-bytes",
>>>>>>            "value": 0
>>>>>>          },
>>>>>>          ...
>>>>>>          {
>>>>>>            "name": "sym-decrypt-bytes",
>>>>>>            "value": 5376
>>>>>>          },
>>>>>>          ...
>>>>>>        ],
>>>>>>        "qom-path": "/objects/cryptodev1" # support asym/sym
>>>>>>      }
>>>>>>    ],
>>>>>>    "id": "libvirt-422"
>>>>>> }
>>>>>>
>>>>>> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
>>>>>> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
>>>>>
>>>>> I assume since this has been out a long time and no
>>>>> comments by maintainers it's ok from QAPI POV.
>>>>
>>>> I'm not the QAPI maintainer, but I think this worked out
>>>> pretty nicely compared to the previous versions of the
>>>> series which didn't use query-stats.. just a minor point
>>>> below.
>>>
>>> Hmm applied already ... is this ok to fix with patch on top
>>> or do I have to revert?
>>
>> Not a big deal. It'd be fine as a followup improvement if
>> zhenwei wants to submit a later cleanup.
>>
>> With regards,
>> Daniel
> 
> zhenwei can you confirm pls?
> 

Hi,
I noticed that Markus has already separated HMP commands from monitor/* 
into subsystems, so I need rework this part of this series.
I sent a new version which includes: 1, HMP part; 2, suggested by 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 :|
> 

-- 
zhenwei pi