Re: [PATCH kvmtool 0/2] Fixes for virtio_balloon stats printing

[Keir Fraser <keirf@google.com>]

On Mon, May 23, 2022 at 03:42:49PM +0100, Andre Przywara wrote:
> On Fri, 20 May 2022 21:51:07 +0100
> Will Deacon <will@kernel.org> wrote:
> 
> Hi,
> 
> > On Fri, 20 May 2022 14:37:04 +0000, Keir Fraser wrote:
> > > While playing with kvmtool's virtio_balloon device I found a couple of
> > > niggling issues with the printing of memory stats. Please consider
> > > these fairly trivial fixes.
> 
> Unfortunately patch 2/2 breaks compilation on userland with older kernel
> headers, like Ubuntu 18.04:
> ...
>   CC       builtin-stat.o
> builtin-stat.c: In function 'do_memstat':
> builtin-stat.c:86:8: error: 'VIRTIO_BALLOON_S_HTLB_PGALLOC' undeclared (first use in this function); did you mean 'VIRTIO_BALLOON_S_AVAIL'?
>    case VIRTIO_BALLOON_S_HTLB_PGALLOC:
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>         VIRTIO_BALLOON_S_AVAIL
> (repeated for VIRTIO_BALLOON_S_HTLB_PGFAIL and VIRTIO_BALLOON_S_CACHES).
> 
> I don't quite remember what we did here in the past in those cases,
> conditionally redefine the symbols in a local header, or protect the
> new code with an #ifdef?

For what it's worth, my opinion is that the sensible options are to:
1. Build against the latest stable, or a specified version of, kernel
headers; or 2. Protect with ifdef'ery until new definitions are
considered "common enough".

Supporting older headers by grafting or even modifying required newer
definitions on top seems a horrid middle ground, albeit I can
appreciate the pragmatism of it.

 Regards,
 Keir


> I would lean towards the former (and hacking this in works), but then we
> would need to redefine VIRTIO_BALLOON_S_NR, to encompass the new symbols,
> which sounds fragile.
> 
> Happy to send a patch if we agree on an approach.
> 
> Cheers,
> Andre
> 
> > > 
> > > Keir Fraser (2):
> > >   virtio/balloon: Fix a crash when collecting stats
> > >   stat: Add descriptions for new virtio_balloon stat types
> > > 
> > > [...]  
> > 
> > Applied to kvmtool (master), thanks!
> > 
> > [1/2] virtio/balloon: Fix a crash when collecting stats
> >       https://git.kernel.org/will/kvmtool/c/3a13530ae99a
> > [2/2] stat: Add descriptions for new virtio_balloon stat types
> >       https://git.kernel.org/will/kvmtool/c/bc77bf49df6e
> > 
> > Cheers,
>