On Thu, Mar 31, 2022 at 7:07 AM Christian Schoenebeck <
qemu_...@crudebyte.com> wrote:
> On Donnerstag, 31. März 2022 10:03:35 CEST Peter Maydell wrote:
> > On Wed, 30 Mar 2022 at 22:55, Will Cohen <wwco...@gmail.com> wrote:
> > > On Wed, Mar 30, 2022 at 5:31 PM Peter Maydell <
> peter.mayd...@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.
>
Christian, do you have thoughts on the meson.build check, then? For all the
reasons you state directly above, there's still some macOS-specific logic
inherent to this functionality. If I create a meson check for
CONFIG_XATTR_SIZE_MAX, the code becomes something like the following:
#if defined(CONFIG_XATTR_SIZE_MAX)
/* Currently, only Linux has XATTR_SIZE_MAX */
#define P9_XATTR_SIZE_MAX XATTR_SIZE_MAX
#elif defined(CONFIG_DARWIN)
...
On the one hand, I can see how this makes the intent a little clearer --
there's some kind of conceptual pre-defined header symbol in "most" cases
(currently only one operating system), with some os-specific fallback logic.
On the other hand, this isn't really shortening anything, it's just
replacing CONFIG_LINUX with something which effectively resolves to
CONFIG_LINUX through redirection.
Will
>
> Best regards,
> Christian Schoenebeck
>
>
>
