On Mon, 2021-09-27 at 09:51 +0200, Jan Beulich wrote:
> On 24.09.2021 16:51, Dario Faggioli wrote:
> > On Mon, 2021-09-20 at 18:25 +0100, Andrew Cooper wrote:
> > 
> > > There is one buggy race record, TRC_RTDS_BUDGET_BURN.  As it must
> > > remain
> > > __packed (as cur_budget is misaligned), change bool has_extratime
> > > to
> > > uint32_t
> > > to compensate.
> > > 
> > Mmm... maybe my understanding of data alignment inside structs is a
> > bit
> > lacking, but what the actual issue here, and what would we need to
> > do
> > to fix it (where, by fix, I mean us being able to get rid of the
> > `__packed`)?
> > 
> > If rearranging fields is not enough, we can think about making
> > priority_level and has_extratime smaller, or even combining them in
> > just one field and decode the information in xentrace.
> 
> I guess Andrew has tried to avoid re-arranging field order so that
> the consumer side doesn't need to also change.
> 
Right, but is it really worth it, in this case?

Aren't we (very very likely) in control, by having them here in the
tree, of all the consumers? And is is this a stable ABI?

Also, both xentrace_format and xenalyze are being modified in this
series anyway...

Maybe there's still something I'm missing, but if we've getting rid of
those ugly bitfields and __packed attributes, it seems to me that doing
it completely --i.e., including in this case-- is worth the small
change in the tools.

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)