Re: [PATCH] 9p: move limits.h include from 9p.c to 9p.h

[Christian Schoenebeck]

On Donnerstag, 31. März 2022 10:03:35 CEST Peter Maydell wrote:
> On Wed, 30 Mar 2022 at 22:55, Will Cohen <wwcohen@gmail.com> wrote:
> > On Wed, Mar 30, 2022 at 5:31 PM Peter Maydell <peter.maydell@linaro.org> 
wrote:
> >> Is it possible to do this with a meson.build check for whatever
> >> host property we're relying on here rather than with a
> >> "which OS is this?" ifdef ?
> > 
> > To confirm -- the game plan in this case would be to do a check for
> > something along the lines of
> > config_host_data.set('CONFIG_XATTR_SIZE_MAX',
> > cc.has_header_symbol('linux/limits.h', 'XATTR_SIZE_MAX')) and using that
> > in the corresponding ifs, right?
> > 
> > That makes sense -- if there's no objections, I'll go this route for v2,
> > which I can submit tomorrow.
> Yeah, something like that.
> 
> Looking a bit closer at the code it looks like the handling of
> XATTR_SIZE_MAX is kind of odd: on Linux we use this kernel-provided
> value, whatever it is, on macos we use a hardcoded 64K, and on
> any other host we fail to compile. The comment claims we only
> need to impose a limit to avoid doing an overly large malloc,
> but if that's the case this shouldn't be OS-specific. I suspect
> the problem here is we're trying to impose a non-existent fixed
> maximum size for something where the API on the host just doesn't
> guarantee one.
> 
> But that would be a 7.1 thing to look at improving.

It's like this: macOS does not officially have a limit for xattr size in 
general. HPFS has a xattr size limit on filesystem level it seems up to 
INT32_MAX, whereas today's APFS's xattr size AFAIK is only limited by the max. 
APFS file size (8 EB).

As 9p is only used for Linux guests so far, and Linux having a much smaller 
xattr size limit of 64k, and 9p server still using a very simple RAM only 
xattr implementation, the idea was to cap the xattr size for macOS hosts to 
hard coded 64k for that reason for now, at least until there are e.g. macOS 9p 
guests one day that would then actually start to profit from a streaming xattr 
implementation in 9p server.

However right now 9p in QEMU only supports Linux hosts and macOS hosts, and 
the idea of

#else
#error Missing definition for P9_XATTR_SIZE_MAX for this host system
#endif

was to ensure that whoever adds support for another 9p host system in future, 
to check what's the limit on that host system, i.e. it might even be <64k. So 
I wouldn't just blindly use a default value here for all systems.

Best regards,
Christian Schoenebeck