diff mbox series

[v5,06/11] 9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX

Message ID 20220207224024.87745-7-wwcohen@gmail.com (mailing list archive)
State New, archived
Headers show
Series 9p: Add support for darwin | expand

Commit Message

Will Cohen Feb. 7, 2022, 10:40 p.m. UTC
From: Keno Fischer <keno@juliacomputing.com>

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>

Because XATTR_SIZE_MAX is not defined on Darwin,
create a cross-platform P9_XATTR_SIZE_MAX instead.

[Will Cohen: - Adjust coding style
             - Lower XATTR_SIZE_MAX to 64k
             - Add explanatory context related to XATTR_SIZE_MAX]
[Fabian Franz: - Move XATTR_SIZE_MAX reference from 9p.c to
                 P9_XATTR_SIZE_MAX in 9p.h]
Signed-off-by: Will Cohen <wwcohen@gmail.com>
Signed-off-by: Fabian Franz <fabianfranz.oss@gmail.com>
---
 hw/9pfs/9p.c |  2 +-
 hw/9pfs/9p.h | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

Christian Schoenebeck Feb. 8, 2022, 12:20 p.m. UTC | #1
On Montag, 7. Februar 2022 23:40:19 CET Will Cohen wrote:
> From: Keno Fischer <keno@juliacomputing.com>
> 
> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
> 
> Because XATTR_SIZE_MAX is not defined on Darwin,
> create a cross-platform P9_XATTR_SIZE_MAX instead.
> 
> [Will Cohen: - Adjust coding style
>              - Lower XATTR_SIZE_MAX to 64k
>              - Add explanatory context related to XATTR_SIZE_MAX]
> [Fabian Franz: - Move XATTR_SIZE_MAX reference from 9p.c to
>                  P9_XATTR_SIZE_MAX in 9p.h]
> Signed-off-by: Will Cohen <wwcohen@gmail.com>
> Signed-off-by: Fabian Franz <fabianfranz.oss@gmail.com>
> ---
>  hw/9pfs/9p.c |  2 +-
>  hw/9pfs/9p.h | 11 +++++++++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 14e84c3bcf..7405352c37 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -3949,7 +3949,7 @@ static void coroutine_fn v9fs_xattrcreate(void
> *opaque) rflags |= XATTR_REPLACE;
>      }
> 
> -    if (size > XATTR_SIZE_MAX) {
> +    if (size > P9_XATTR_SIZE_MAX) {
>          err = -E2BIG;
>          goto out_nofid;
>      }
> diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> index 1567b67841..6a1856b4dc 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -479,4 +479,15 @@ struct V9fsTransport {
>      void        (*push_and_notify)(V9fsPDU *pdu);
>  };
> 
> +/*
> + * Darwin doesn't seem to define a maximum xattr size in its user
> + * space header, so manually configure it across platforms as 64k.
> + *
> + * Having no limit at all can lead to QEMU crashing during large g_malloc()
> + * calls. Because QEMU does not currently support macOS guests, the below
> + * preliminary solution only works due to its being a reflection of the limit of
> + * Linux guests.
> + */
> +#define P9_XATTR_SIZE_MAX 65536

It would be cleaner in a way like this:

#if defined(XATTR_SIZE_MAX)
/* Linux */
#define P9_XATTR_SIZE_MAX XATTR_SIZE_MAX
#elif defined(CONFIG_DARWIN)
/* darwin comment goes here */
#define P9_XATTR_SIZE_MAX 65536
#else
#error Missing definition for P9_XATTR_SIZE_MAX for this host system
#endif

Sorry, I haven't noticed that before. You actually had that wrapped into some
ifdefs in v2 before:

#if defined(CONFIG_DARWIN) && !defined(XATTR_SIZE_MAX)
...
#endif

> +
>  #endif
Will Cohen Feb. 8, 2022, 1:45 p.m. UTC | #2
On Tue, Feb 8, 2022 at 7:20 AM Christian Schoenebeck <qemu_oss@crudebyte.com>
wrote:

> On Montag, 7. Februar 2022 23:40:19 CET Will Cohen wrote:
> > From: Keno Fischer <keno@juliacomputing.com>
> >
> > Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> > Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
> >
> > Because XATTR_SIZE_MAX is not defined on Darwin,
> > create a cross-platform P9_XATTR_SIZE_MAX instead.
> >
> > [Will Cohen: - Adjust coding style
> >              - Lower XATTR_SIZE_MAX to 64k
> >              - Add explanatory context related to XATTR_SIZE_MAX]
> > [Fabian Franz: - Move XATTR_SIZE_MAX reference from 9p.c to
> >                  P9_XATTR_SIZE_MAX in 9p.h]
> > Signed-off-by: Will Cohen <wwcohen@gmail.com>
> > Signed-off-by: Fabian Franz <fabianfranz.oss@gmail.com>
> > ---
> >  hw/9pfs/9p.c |  2 +-
> >  hw/9pfs/9p.h | 11 +++++++++++
> >  2 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 14e84c3bcf..7405352c37 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -3949,7 +3949,7 @@ static void coroutine_fn v9fs_xattrcreate(void
> > *opaque) rflags |= XATTR_REPLACE;
> >      }
> >
> > -    if (size > XATTR_SIZE_MAX) {
> > +    if (size > P9_XATTR_SIZE_MAX) {
> >          err = -E2BIG;
> >          goto out_nofid;
> >      }
> > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> > index 1567b67841..6a1856b4dc 100644
> > --- a/hw/9pfs/9p.h
> > +++ b/hw/9pfs/9p.h
> > @@ -479,4 +479,15 @@ struct V9fsTransport {
> >      void        (*push_and_notify)(V9fsPDU *pdu);
> >  };
> >
> > +/*
> > + * Darwin doesn't seem to define a maximum xattr size in its user
> > + * space header, so manually configure it across platforms as 64k.
> > + *
> > + * Having no limit at all can lead to QEMU crashing during large
> g_malloc()
> > + * calls. Because QEMU does not currently support macOS guests, the
> below
> > + * preliminary solution only works due to its being a reflection of the
> limit of
> > + * Linux guests.
> > + */
> > +#define P9_XATTR_SIZE_MAX 65536
>
> It would be cleaner in a way like this:
>
> #if defined(XATTR_SIZE_MAX)
> /* Linux */
> #define P9_XATTR_SIZE_MAX XATTR_SIZE_MAX
> #elif defined(CONFIG_DARWIN)
> /* darwin comment goes here */
> #define P9_XATTR_SIZE_MAX 65536
> #else
> #error Missing definition for P9_XATTR_SIZE_MAX for this host system
> #endif
>
> Sorry, I haven't noticed that before. You actually had that wrapped into
> some
> ifdefs in v2 before:
>
> #if defined(CONFIG_DARWIN) && !defined(XATTR_SIZE_MAX)
> ...
> #endif
>
> > +
> >  #endif
>
> Agreed, that is cleaner. Adjusting for the next round.
diff mbox series

Patch

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 14e84c3bcf..7405352c37 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3949,7 +3949,7 @@  static void coroutine_fn v9fs_xattrcreate(void *opaque)
         rflags |= XATTR_REPLACE;
     }
 
-    if (size > XATTR_SIZE_MAX) {
+    if (size > P9_XATTR_SIZE_MAX) {
         err = -E2BIG;
         goto out_nofid;
     }
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 1567b67841..6a1856b4dc 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -479,4 +479,15 @@  struct V9fsTransport {
     void        (*push_and_notify)(V9fsPDU *pdu);
 };
 
+/*
+ * Darwin doesn't seem to define a maximum xattr size in its user
+ * space header, so manually configure it across platforms as 64k.
+ *
+ * Having no limit at all can lead to QEMU crashing during large g_malloc()
+ * calls. Because QEMU does not currently support macOS guests, the below
+ * preliminary solution only works due to its being a reflection of the limit of
+ * Linux guests.
+ */
+#define P9_XATTR_SIZE_MAX 65536
+
 #endif