Re: API/ABIs: Re: [RFC PATCH v2 0/2] Add a new acquire resource to query vcpu statistics

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.03.2023 12:54, Matias Ezequiel Vara Larsen wrote:
> On Tue, Mar 07, 2023 at 11:12:00AM +0100, Jan Beulich wrote:
>> On 06.03.2023 15:23, Matias Ezequiel Vara Larsen wrote:
>>> - Xen shall use the "stats_active" field to indicate what it is producing. 
>>> In
>>>   this field, reserved bits shall be 0. This shall allow us to agree on the
>>> layout even when producer and consumer are compiled with different headers.
>>
>> I wonder how well such a bitfield is going to scale. It provides for
>> only 32 (maybe 64) counters. Of course this may seem a lot right now,
>> but you never know how quickly something like this can grow. Plus
>> with ...
>>
> 
> Would it make sense to define it like this?:
> 
> struct vcpu_shmem_stats {
> #define STATS_A (1u << 0)
> ...
> #define VCPU_STATS_MAGIC 0xaabbccdd
>      uint32_t magic;
>      uint32_t offset;  // roundup(sizeof(vcpu_shmem_stats) + sizeof(uint32_t) 
> * nr_stats, cacheline_size)
>      uint32_t size;    // sizeof(vcpu_stats)
>      uint32_t stride;  // roundup(sizeof(vcpu_stats), cacheline_size)
>      uint32_t nr_stats; // size of stats_active in uint32_t
>      uint32_t stats_active[XEN_FLEX_ARRAY_DIM];
>      ...
> };

Possibly, but this would make it harder to use the interface. An alternative
might be to specify that an actual stats value set to ~0 marks an inactive
element. Since these are 64-bit counters, with today's and tomorrow's
computers we won't be at risk of a counter reaching a value of 2^^64-1, I
guess. And even if there was one where such a risk could not be excluded
(e.g. because of using pretty large increments), one would merely need to
make sure to saturate at 2^^64-2. Plus at such a point one would need to
consider anyway to switch to 128-bit fields, as neither saturated nor
wrapped values are of much use to consumers.

>>> - In the vcpu_stats structure, new fields can only ever be appended.
>>
>> ... this rule the only ambiguity that could arise to consumers when
>> no active flags existed would be that they can't tell "event never
>> occurred" from "hypervisor doesn't know about this anymore".
> 
> Do you mean how the consumer can figure out if either 1) Xen does not know yet
> about some flag or 2) the flag has been deprecated? I think 2) is the case 
> that
> Andrew mention in which the magic number could be used as an ABI version to
> break backwards compatibility.

No, an inactive field wouldn't break the ABI. An ABI change would be if
such an inactive field was actually removed from the array. Or if e.g.,
as per above, we needed to switch to 128-bit counters.

Jan