| Message ID | 20220331182651.887-1-wwcohen@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] 9p: move P9_XATTR_SIZE_MAX from 9p.h to 9p.c | expand |
On Thu, 31 Mar 2022 at 19:27, Will Cohen <wwcohen@gmail.com> wrote: > > The patch set adding 9p functionality to darwin introduced an issue > where limits.h, which defines XATTR_SIZE_MAX, is included in 9p.c, > though the referenced constant is needed in 9p.h. This commit fixes that > issue by moving the definition of P9_XATTR_SIZE_MAX, which uses > XATTR_SIZE_MAX, to also be in 9p.c. > > Additionally, this commit moves the location of the system headers > include in 9p.c to occur before the project headers. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/950 > Fixes: 38d7fd68b0 ("9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX") > > Signed-off-by: Will Cohen <wwcohen@gmail.com> > --- > hw/9pfs/9p.c | 28 +++++++++++++++++++++++----- > hw/9pfs/9p.h | 18 ------------------ > 2 files changed, 23 insertions(+), 23 deletions(-) > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > index dcaa602d4c..b9152c7882 100644 > --- a/hw/9pfs/9p.c > +++ b/hw/9pfs/9p.c > @@ -16,6 +16,11 @@ > * https://wiki.qemu.org/Documentation/9p > */ > > +#ifdef CONFIG_LINUX > +#include <linux/limits.h> > +#else > +#include <limits.h> > +#endif > #include "qemu/osdep.h" osdep.h must always be the first include line in any .c file. thanks -- PMM
On Thu, Mar 31, 2022 at 4:00 PM Peter Maydell <peter.maydell@linaro.org> wrote: > On Thu, 31 Mar 2022 at 19:27, Will Cohen <wwcohen@gmail.com> wrote: > > > > The patch set adding 9p functionality to darwin introduced an issue > > where limits.h, which defines XATTR_SIZE_MAX, is included in 9p.c, > > though the referenced constant is needed in 9p.h. This commit fixes that > > issue by moving the definition of P9_XATTR_SIZE_MAX, which uses > > XATTR_SIZE_MAX, to also be in 9p.c. > > > > Additionally, this commit moves the location of the system headers > > include in 9p.c to occur before the project headers. > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/950 > > Fixes: 38d7fd68b0 ("9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX") > > > > Signed-off-by: Will Cohen <wwcohen@gmail.com> > > --- > > hw/9pfs/9p.c | 28 +++++++++++++++++++++++----- > > hw/9pfs/9p.h | 18 ------------------ > > 2 files changed, 23 insertions(+), 23 deletions(-) > > > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > > index dcaa602d4c..b9152c7882 100644 > > --- a/hw/9pfs/9p.c > > +++ b/hw/9pfs/9p.c > > @@ -16,6 +16,11 @@ > > * https://wiki.qemu.org/Documentation/9p > > */ > > > > +#ifdef CONFIG_LINUX > > +#include <linux/limits.h> > > +#else > > +#include <limits.h> > > +#endif > > #include "qemu/osdep.h" > > osdep.h must always be the first include line in any .c file. > Understood, apologies -- if there's other changes for a v3 I can resubmit accordingly, but if this otherwise looks okay then I would be fine with a QEMU maintainer adjusting the header placement as needed when preparing for submission to the main tree. Will > > thanks > -- PMM >
On 31/03/2022 22.06, Will Cohen wrote: > On Thu, Mar 31, 2022 at 4:00 PM Peter Maydell <peter.maydell@linaro.org > <mailto:peter.maydell@linaro.org>> wrote: > > On Thu, 31 Mar 2022 at 19:27, Will Cohen <wwcohen@gmail.com > <mailto:wwcohen@gmail.com>> wrote: > > > > The patch set adding 9p functionality to darwin introduced an issue > > where limits.h, which defines XATTR_SIZE_MAX, is included in 9p.c, > > though the referenced constant is needed in 9p.h. This commit fixes that > > issue by moving the definition of P9_XATTR_SIZE_MAX, which uses > > XATTR_SIZE_MAX, to also be in 9p.c. > > > > Additionally, this commit moves the location of the system headers > > include in 9p.c to occur before the project headers. > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/950 > <https://gitlab.com/qemu-project/qemu/-/issues/950> > > Fixes: 38d7fd68b0 ("9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX") > > > > Signed-off-by: Will Cohen <wwcohen@gmail.com <mailto:wwcohen@gmail.com>> > > --- > > hw/9pfs/9p.c | 28 +++++++++++++++++++++++----- > > hw/9pfs/9p.h | 18 ------------------ > > 2 files changed, 23 insertions(+), 23 deletions(-) > > > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > > index dcaa602d4c..b9152c7882 100644 > > --- a/hw/9pfs/9p.c > > +++ b/hw/9pfs/9p.c > > @@ -16,6 +16,11 @@ > > * https://wiki.qemu.org/Documentation/9p > <https://wiki.qemu.org/Documentation/9p> > > */ > > > > +#ifdef CONFIG_LINUX > > +#include <linux/limits.h> > > +#else > > +#include <limits.h> > > +#endif > > #include "qemu/osdep.h" > > osdep.h must always be the first include line in any .c file. > > Understood, apologies -- if there's other changes for a v3 I can resubmit > accordingly, but if this otherwise looks okay then I would be fine with a > QEMU maintainer adjusting the header placement as needed when preparing for > submission to the main tree. Makes sense. I'm currently assembling a pull req with some misc fixes for 7.0 ... if Christian & Greg do not have any other patches pending right now, I could throw this in, with the osdep.h location fixed. Thomas
On Freitag, 1. April 2022 13:01:31 CEST Thomas Huth wrote: > On 31/03/2022 22.06, Will Cohen wrote: > > On Thu, Mar 31, 2022 at 4:00 PM Peter Maydell <peter.maydell@linaro.org > > > > <mailto:peter.maydell@linaro.org>> wrote: > > On Thu, 31 Mar 2022 at 19:27, Will Cohen <wwcohen@gmail.com > > > > <mailto:wwcohen@gmail.com>> wrote: > > > The patch set adding 9p functionality to darwin introduced an issue > > > where limits.h, which defines XATTR_SIZE_MAX, is included in 9p.c, > > > though the referenced constant is needed in 9p.h. This commit fixes > > > that > > > issue by moving the definition of P9_XATTR_SIZE_MAX, which uses > > > XATTR_SIZE_MAX, to also be in 9p.c. > > > > > > Additionally, this commit moves the location of the system headers > > > include in 9p.c to occur before the project headers. > > > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/950 > > > > <https://gitlab.com/qemu-project/qemu/-/issues/950> > > > > > Fixes: 38d7fd68b0 ("9p: darwin: Move > > > XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX") > > > > > > Signed-off-by: Will Cohen <wwcohen@gmail.com > > > <mailto:wwcohen@gmail.com>> > > > --- > > > > > > hw/9pfs/9p.c | 28 +++++++++++++++++++++++----- > > > hw/9pfs/9p.h | 18 ------------------ > > > 2 files changed, 23 insertions(+), 23 deletions(-) > > > > > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > > > index dcaa602d4c..b9152c7882 100644 > > > --- a/hw/9pfs/9p.c > > > +++ b/hw/9pfs/9p.c > > > @@ -16,6 +16,11 @@ > > > > > > * https://wiki.qemu.org/Documentation/9p > > > > <https://wiki.qemu.org/Documentation/9p> > > > > > */ > > > > > > +#ifdef CONFIG_LINUX > > > +#include <linux/limits.h> > > > +#else > > > +#include <limits.h> > > > +#endif > > > > > > #include "qemu/osdep.h" > > > > osdep.h must always be the first include line in any .c file. > > > > Understood, apologies -- if there's other changes for a v3 I can resubmit > > accordingly, but if this otherwise looks okay then I would be fine with a > > QEMU maintainer adjusting the header placement as needed when preparing > > for > > submission to the main tree. > > Makes sense. I'm currently assembling a pull req with some misc fixes for > 7.0 ... if Christian & Greg do not have any other patches pending right now, > I could throw this in, with the osdep.h location fixed. > > Thomas That would be appreciated, nothing else for 7.0 at this point. Thanks Thomas! Best regards, Christian Schoenebeck
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index dcaa602d4c..b9152c7882 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -16,6 +16,11 @@ * https://wiki.qemu.org/Documentation/9p */ +#ifdef CONFIG_LINUX +#include <linux/limits.h> +#else +#include <limits.h> +#endif #include "qemu/osdep.h" #include <glib/gprintf.h> #include "hw/virtio/virtio.h" @@ -33,11 +38,6 @@ #include "migration/blocker.h" #include "qemu/xxhash.h" #include <math.h> -#ifdef CONFIG_LINUX -#include <linux/limits.h> -#else -#include <limits.h> -#endif int open_fd_hw; int total_open_fd; @@ -3925,6 +3925,24 @@ out_nofid: v9fs_string_free(&name); } +#if defined(CONFIG_LINUX) +/* Currently, only Linux has XATTR_SIZE_MAX */ +#define P9_XATTR_SIZE_MAX XATTR_SIZE_MAX +#elif defined(CONFIG_DARWIN) +/* + * 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 +#else +#error Missing definition for P9_XATTR_SIZE_MAX for this host system +#endif + static void coroutine_fn v9fs_xattrcreate(void *opaque) { int flags, rflags = 0; diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h index af2635fae9..994f952600 100644 --- a/hw/9pfs/9p.h +++ b/hw/9pfs/9p.h @@ -479,22 +479,4 @@ struct V9fsTransport { void (*push_and_notify)(V9fsPDU *pdu); }; -#if defined(XATTR_SIZE_MAX) -/* Linux */ -#define P9_XATTR_SIZE_MAX XATTR_SIZE_MAX -#elif defined(CONFIG_DARWIN) -/* - * 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 -#else -#error Missing definition for P9_XATTR_SIZE_MAX for this host system -#endif - #endif
The patch set adding 9p functionality to darwin introduced an issue where limits.h, which defines XATTR_SIZE_MAX, is included in 9p.c, though the referenced constant is needed in 9p.h. This commit fixes that issue by moving the definition of P9_XATTR_SIZE_MAX, which uses XATTR_SIZE_MAX, to also be in 9p.c. Additionally, this commit moves the location of the system headers include in 9p.c to occur before the project headers. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/950 Fixes: 38d7fd68b0 ("9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX") Signed-off-by: Will Cohen <wwcohen@gmail.com> --- hw/9pfs/9p.c | 28 +++++++++++++++++++++++----- hw/9pfs/9p.h | 18 ------------------ 2 files changed, 23 insertions(+), 23 deletions(-)