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
