qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Christian Schoenebeck <qemu_oss@crudebyte.com>
To: qemu-devel@nongnu.org
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Will Cohen" <wwcohen@gmail.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Fabian Franz" <fabianfranz.oss@gmail.com>,
	"Greg Kurz" <groug@kaod.org>,
	"Keno Fischer" <keno@juliacomputing.com>,
	"Michael Roitzsch" <reactorcontrol@icloud.com>,
	"Philippe Mathieu-Daudé" <philippe.mathieu.daude@gmail.com>,
	hi@alyssa.is
Subject: Re: [PATCH] 9p: move limits.h include from 9p.c to 9p.h
Date: Thu, 31 Mar 2022 13:07:20 +0200	[thread overview]
Message-ID: <2350345.UNtK3Xum7i@silver> (raw)
In-Reply-To: <CAFEAcA8fdebk3Z9bUbHuYKL5VCFqtu9gqbHdB4i_Umuaabqr7g@mail.gmail.com>

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




  reply	other threads:[~2022-03-31 11:15 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-30 18:19 [PATCH] 9p: move limits.h include from 9p.c to 9p.h Will Cohen
2022-03-30 20:23 ` Philippe Mathieu-Daudé
2022-03-30 21:17   ` Will Cohen
2022-03-31 10:20     ` Christian Schoenebeck
2022-03-30 21:31 ` Peter Maydell
2022-03-30 21:55   ` Will Cohen
2022-03-31  8:03     ` Peter Maydell
2022-03-31 11:07       ` Christian Schoenebeck [this message]
2022-03-31 13:19         ` Will Cohen
2022-03-31 15:34           ` Christian Schoenebeck
2022-03-31 17:57             ` Will Cohen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2350345.UNtK3Xum7i@silver \
    --to=qemu_oss@crudebyte.com \
    --cc=fabianfranz.oss@gmail.com \
    --cc=groug@kaod.org \
    --cc=hi@alyssa.is \
    --cc=keno@juliacomputing.com \
    --cc=peter.maydell@linaro.org \
    --cc=philippe.mathieu.daude@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=reactorcontrol@icloud.com \
    --cc=thuth@redhat.com \
    --cc=wwcohen@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).