Re: [PATCH v10 2/5] KVM: stats: Add fd-based API to read binary stats data

2021-06-17 Thread Jing Zhang
On Thu, Jun 17, 2021 at 10:29 AM Paolo Bonzini wrote: > > On 17/06/21 16:56, Jing Zhang wrote: > > Actually, it is really not easy to separate this change into two patches > > even by > > following Paolo's suggestion. And it would be a surprise to userspace to see > > only VM stats, no VCPU stats

Re: [PATCH v10 2/5] KVM: stats: Add fd-based API to read binary stats data

2021-06-17 Thread Jing Zhang
Hi Greg, On Thu, Jun 17, 2021 at 2:24 AM Greg KH wrote: > > On Thu, Jun 17, 2021 at 04:41:43AM +, Jing Zhang wrote: > > Provides a file descriptor per VM to read VM stats info/data. > > Provides a file descriptor per vCPU to read vCPU stats info/data. > > Shouldn't this be two separate patche

Re: [PATCH v10 2/5] KVM: stats: Add fd-based API to read binary stats data

2021-06-17 Thread Jing Zhang
Hi Greg, On Thu, Jun 17, 2021 at 2:03 AM Greg KH wrote: > > On Thu, Jun 17, 2021 at 04:41:43AM +, Jing Zhang wrote: > > Provides a file descriptor per VM to read VM stats info/data. > > Provides a file descriptor per vCPU to read vCPU stats info/data. > > > > The KVM stats now is only accessi

Re: [PATCH v10 2/5] KVM: stats: Add fd-based API to read binary stats data

2021-06-17 Thread Paolo Bonzini
On 17/06/21 16:56, Jing Zhang wrote: Actually, it is really not easy to separate this change into two patches even by following Paolo's suggestion. And it would be a surprise to userspace to see only VM stats, no VCPU stats. That does not matter. Having two or three patches is useful because i

Re: [PATCH v10 2/5] KVM: stats: Add fd-based API to read binary stats data

2021-06-17 Thread Paolo Bonzini
On 17/06/21 09:03, Greg KH wrote: 3. Fd-based solution provides the possibility that a telemetry can read KVM stats in a less privileged situation. "possiblity"? Does this work or not? Have you tested it? I think this is essentially s/that/for/. But more precisely: 3. Compared for exa

Re: [PATCH v10 2/5] KVM: stats: Add fd-based API to read binary stats data

2021-06-17 Thread Paolo Bonzini
On 17/06/21 09:24, Greg KH wrote: Provides a file descriptor per VM to read VM stats info/data. Provides a file descriptor per vCPU to read vCPU stats info/data. Shouldn't this be two separate patches, one for each thing as these are two different features being added? They share a lot of code

[PATCH v10 2/5] KVM: stats: Add fd-based API to read binary stats data

2021-06-17 Thread Jing Zhang
Provides a file descriptor per VM to read VM stats info/data. Provides a file descriptor per vCPU to read vCPU stats info/data. The KVM stats now is only accessible by debugfs, which has some shortcomings this change are supposed to fix: 1. Debugfs is not a stable interface for production and it i

Re: [PATCH v10 2/5] KVM: stats: Add fd-based API to read binary stats data

2021-06-17 Thread Greg KH
On Thu, Jun 17, 2021 at 04:41:43AM +, Jing Zhang wrote: > Provides a file descriptor per VM to read VM stats info/data. > Provides a file descriptor per vCPU to read vCPU stats info/data. Shouldn't this be two separate patches, one for each thing as these are two different features being added

Re: [PATCH v10 2/5] KVM: stats: Add fd-based API to read binary stats data

2021-06-17 Thread Greg KH
On Thu, Jun 17, 2021 at 04:41:43AM +, Jing Zhang wrote: > Provides a file descriptor per VM to read VM stats info/data. > Provides a file descriptor per vCPU to read vCPU stats info/data. > > The KVM stats now is only accessible by debugfs, which has some > shortcomings this change are suppose