Hi Greg,
On Thu, Jun 17, 2021 at 10:20 AM Jing Zhang jingzhangos@google.com wrote:
Hi Greg,
On Thu, Jun 17, 2021 at 12:56 AM Greg KH gregkh@linuxfoundation.org wrote:
On Thu, Jun 17, 2021 at 04:41:44AM +0000, Jing Zhang wrote:
struct kvm_stats_desc {__u32 flags;__s16 exponent;__u16 size;__u32 offset;__u32 unused;char name[0];};<snip>
+The ``unused`` fields are reserved for future support for other types of +statistics data, like log/linear histogram.
you HAVE to set unused to 0 for now, otherwise userspace does not know it is unused, right? And then, really it is "used", so why not just say that now? It's tricky, but you have to get this right now otherwise you can never use it in the future.
Sure, will do that.
+The ``name`` field points to the name string of the statistics data. The name
It is not a pointer, it is the data itself.
Will fix it.
+string starts at the end of ``struct kvm_stats_desc``. +The maximum length (including trailing '\0') is indicated by ``name_size`` +in ``struct kvm_stats_header``.
I thought we were replacing [0] arrays with [], are you sure you should be declaring this as [0]? Same for all structures in this document (and code).
The reason to declare it as [0] is to have the flexibility to change the maximum length of KVM stats name. For now, the max len is defined as 48, which can be read from the header. Then the userspace can get the length of descriptor by adding sizeof(struct_kvm_stats_desc) + 48. Whenever the max len is changed in KVM, the userspace would not have to update code to reflect that. However, if we are OK to restrict the maximum KVM stats' length to 48 (or any other number), we can just declear it with [] instead of [0].
Please ignore my above comments. You are right. We can just replace all zero-length arrays [0] with a flexible array member []. Thanks.
thanks,
greg k-h
Thanks, Jing