On 18/06/21 10:23, Greg KH wrote:
On Fri, Jun 18, 2021 at 10:02:57AM +0200, Paolo Bonzini wrote:
On 18/06/21 09:00, Greg KH wrote:
+struct kvm_stats_header {
- __u32 name_size;
- __u32 count;
- __u32 desc_offset;
- __u32 data_offset;
- char id[];
+};
You mentioned before that the size of this really is the size of the structure + KVM_STATS_ID_MAXLEN, right? Or is it - KVM_STATS_ID_MAXLEN?
If so, why not put that value explicitly in: char id[THE_REST_OF_THE_HEADER_SPACE];
As this is not a variable header size at all, and you can not change it going forward, so the variable length array here feels disingenuous.
It can change; the header goes up to desc_offset. Let's rename desc_offset to header_size.
"Traditionally" the first field of a variable length structure like this has the size. So maybe this needs to be:
struct kvm_stats_header { __u32 header_size;
Thinking more about it, I slightly prefer id_offset so that we can later give a meaning to any bytes after kvm_stats_header and before id_offset.
Adding four unused bytes (for now always zero) is also useful to future proof the struct a bit, thus:
struct kvm_stats_header { __u32 flags; __u32 name_size; __u32 num_desc; __u32 id_offset; __u32 desc_offset; __u32 data_offset; }
(Indeed num_desc is better than count).
Wait, what is "name_size" here for?
So that you know the full size of the descriptors is (name_size + sizeof(kvm_stats_desc) + name_size) * num_desc. That's the memory you allocate and the size that you can then pass to a single pread system call starting from offset desc_offset.
There is certainly room for improvement in that the length of id[] and name[] can be unified to name_size.
+struct kvm_stats_desc {
- __u32 flags;
- __s16 exponent;
- __u16 size;
- __u32 offset;
- __u32 unused;
- char name[];
+};
What is the max length of name?
It's name_size in the header.
So it's specified in the _previous_ header? That feels wrong, shouldn't this descriptor define what is in it?
Compared to e.g. PCI where you can do random-access reads from memory or configuration space, reading from a file has slightly different tradeoffs. So designing a file format is slightly different compared to designing an in-memory format, or a wire protocol.
Paolo