On 10.08.20 12:14, Denis V. Lunev wrote: > Right now BlockAcctStats is always reside on BlockBackend. This structure > is not used in any other place. Thus we are able to create a converter > from one pointer to another. > > Signed-off-by: Denis V. Lunev > Reviewed-by: Vladimir Sementsov-Ogievskiy > CC: Stefan Hajnoczi > CC: Kevin Wolf > CC: Max Reitz > --- > block/block-backend.c | 5 +++++ > include/sysemu/block-backend.h | 1 + > 2 files changed, 6 insertions(+) (Note: I’m just writing this mail because I already responded to patch 2. I wouldn’t have if I didn’t have anything to say regarding the other patches in this series, so nothing I say here is backed by a strong opinion from me.) > diff --git a/block/block-backend.c b/block/block-backend.c > index 3a13cb5f0b..88e531df98 100644 > --- a/block/block-backend.c > +++ b/block/block-backend.c > @@ -2153,6 +2153,11 @@ BlockAcctStats *blk_get_stats(BlockBackend *blk) > return &blk->stats; > } > > +BlockBackend *blk_stats2blk(BlockAcctStats *s) (Extreme bikeshedding: I’d prefer something like blk_from_stats().) > +{ > + return container_of(s, BlockBackend, stats); > +} Works, but I get all itchy, especially given the reasoning in the commit message, which is basically “Right now this works”. That sounds to me like maybe in the future someone could get the idea of moving BlockAcctStats somewhere else and then this is something that we absolutely must not forget about, lest horrible accidents occur. Would a back pointer from BlockAcctStats to the BlockBackend work or do you find that just too ugly and unnecessary? (I think it would help at least so that we do not forget this place here.) Or maybe just a comment above BlockAcctStats would help quench my itch, too, like “Note: blk_stats2blk() expects objects of this type only to occur as part of a BlockBackend”. Max