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

[Daniel P . Berrangé]

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