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

[zhenwei pi]

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