Re: [Qemu-devel] [PATCH v3 1/4] QEMU_PACKED: Remove gcc_struct attribute in Windows non x86 targets
On 5/3/19 5:56 PM, Marc-André Lureau wrote: > Hi > > Le ven. 3 mai 2019 à 17:23, Peter Maydell a > écrit : > >> On Fri, 3 May 2019 at 06:07, Thomas Huth wrote: >>> >>> On 03/05/2019 02.36, Cao Jiaxi wrote: gcc_struct is for x86 only, and it generates an warning on ARM64 >> Clang/MinGW targets. Signed-off-by: Cao Jiaxi --- contrib/libvhost-user/libvhost-user.h | 2 +- include/qemu/compiler.h | 2 +- scripts/cocci-macro-file.h| 7 ++- slirp/src/util.h | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) >> diff --git a/slirp/src/util.h b/slirp/src/util.h index 01f1e0e068..278828fe3f 100644 --- a/slirp/src/util.h +++ b/slirp/src/util.h @@ -43,7 +43,7 @@ #include #endif -#if defined(_WIN32) +#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) # define SLIRP_PACKED __attribute__((gcc_struct, packed)) #else # define SLIRP_PACKED __attribute__((packed)) >>> >>> The slirp code is currently on its way into a separate module, so you >>> might need to provide that hunk to the libslirp folks again... I'm >>> putting the slirp maintainers on CC:, maybe they can pick it up from >> here. >> >> Yes, the slirp module has now landed in master, so this patch >> definitely needs to be split into two. I've kept in my >> target-arm.next tree the parts which are applicable to >> the QEMU repo itself (ie everything except the slirp/ change), >> so we just need a new patch for the slirp submodule part. >> >> Marc-André, Samuel -- what's the process for submitting and >> getting reviewed changes to the slirp submodule now it's a >> separate project ? >> > > It's hosted on gitlab.freedesktop.org, with some CI. It's fine to send > patches on qemu devel, as long as Samuel or I are in CC. But in the long > term, I think gitlab MR will be favoured (after a while using it, I think > gitlab is better than ML for patches & bug tracking tbh) Except when working offline (or with poor intermittent link).
Re: [Qemu-devel] [PATCH v3 1/4] QEMU_PACKED: Remove gcc_struct attribute in Windows non x86 targets
Hi Le ven. 3 mai 2019 à 17:23, Peter Maydell a écrit : > On Fri, 3 May 2019 at 06:07, Thomas Huth wrote: > > > > On 03/05/2019 02.36, Cao Jiaxi wrote: > > > gcc_struct is for x86 only, and it generates an warning on ARM64 > Clang/MinGW targets. > > > > > > Signed-off-by: Cao Jiaxi > > > --- > > > contrib/libvhost-user/libvhost-user.h | 2 +- > > > include/qemu/compiler.h | 2 +- > > > scripts/cocci-macro-file.h| 7 ++- > > > slirp/src/util.h | 2 +- > > > 4 files changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/slirp/src/util.h b/slirp/src/util.h > > > index 01f1e0e068..278828fe3f 100644 > > > --- a/slirp/src/util.h > > > +++ b/slirp/src/util.h > > > @@ -43,7 +43,7 @@ > > > #include > > > #endif > > > > > > -#if defined(_WIN32) > > > +#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) > > > # define SLIRP_PACKED __attribute__((gcc_struct, packed)) > > > #else > > > # define SLIRP_PACKED __attribute__((packed)) > > > > > > > The slirp code is currently on its way into a separate module, so you > > might need to provide that hunk to the libslirp folks again... I'm > > putting the slirp maintainers on CC:, maybe they can pick it up from > here. > > Yes, the slirp module has now landed in master, so this patch > definitely needs to be split into two. I've kept in my > target-arm.next tree the parts which are applicable to > the QEMU repo itself (ie everything except the slirp/ change), > so we just need a new patch for the slirp submodule part. > > Marc-André, Samuel -- what's the process for submitting and > getting reviewed changes to the slirp submodule now it's a > separate project ? > It's hosted on gitlab.freedesktop.org, with some CI. It's fine to send patches on qemu devel, as long as Samuel or I are in CC. But in the long term, I think gitlab MR will be favoured (after a while using it, I think gitlab is better than ML for patches & bug tracking tbh) >
Re: [Qemu-devel] [PATCH v3 1/4] QEMU_PACKED: Remove gcc_struct attribute in Windows non x86 targets
On Fri, 3 May 2019 at 06:07, Thomas Huth wrote: > > On 03/05/2019 02.36, Cao Jiaxi wrote: > > gcc_struct is for x86 only, and it generates an warning on ARM64 > > Clang/MinGW targets. > > > > Signed-off-by: Cao Jiaxi > > --- > > contrib/libvhost-user/libvhost-user.h | 2 +- > > include/qemu/compiler.h | 2 +- > > scripts/cocci-macro-file.h| 7 ++- > > slirp/src/util.h | 2 +- > > 4 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/slirp/src/util.h b/slirp/src/util.h > > index 01f1e0e068..278828fe3f 100644 > > --- a/slirp/src/util.h > > +++ b/slirp/src/util.h > > @@ -43,7 +43,7 @@ > > #include > > #endif > > > > -#if defined(_WIN32) > > +#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) > > # define SLIRP_PACKED __attribute__((gcc_struct, packed)) > > #else > > # define SLIRP_PACKED __attribute__((packed)) > > > > The slirp code is currently on its way into a separate module, so you > might need to provide that hunk to the libslirp folks again... I'm > putting the slirp maintainers on CC:, maybe they can pick it up from here. Yes, the slirp module has now landed in master, so this patch definitely needs to be split into two. I've kept in my target-arm.next tree the parts which are applicable to the QEMU repo itself (ie everything except the slirp/ change), so we just need a new patch for the slirp submodule part. Marc-André, Samuel -- what's the process for submitting and getting reviewed changes to the slirp submodule now it's a separate project ? thanks -- PMM
Re: [Qemu-devel] [PATCH v3 1/4] QEMU_PACKED: Remove gcc_struct attribute in Windows non x86 targets
On 03/05/2019 02.36, Cao Jiaxi wrote:
> gcc_struct is for x86 only, and it generates an warning on ARM64 Clang/MinGW
> targets.
>
> Signed-off-by: Cao Jiaxi
> ---
> contrib/libvhost-user/libvhost-user.h | 2 +-
> include/qemu/compiler.h | 2 +-
> scripts/cocci-macro-file.h| 7 ++-
> slirp/src/util.h | 2 +-
> 4 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/libvhost-user/libvhost-user.h
> b/contrib/libvhost-user/libvhost-user.h
> index 414ceb0a2f..78b33306e8 100644
> --- a/contrib/libvhost-user/libvhost-user.h
> +++ b/contrib/libvhost-user/libvhost-user.h
> @@ -148,7 +148,7 @@ typedef struct VhostUserInflight {
> uint16_t queue_size;
> } VhostUserInflight;
>
> -#if defined(_WIN32)
> +#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
> # define VU_PACKED __attribute__((gcc_struct, packed))
> #else
> # define VU_PACKED __attribute__((packed))
> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> index 296b2fd572..09fc44cca4 100644
> --- a/include/qemu/compiler.h
> +++ b/include/qemu/compiler.h
> @@ -28,7 +28,7 @@
>
> #define QEMU_SENTINEL __attribute__((sentinel))
>
> -#if defined(_WIN32)
> +#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
> # define QEMU_PACKED __attribute__((gcc_struct, packed))
> #else
> # define QEMU_PACKED __attribute__((packed))
> diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
> index e485cdccae..c6bbc05ba3 100644
> --- a/scripts/cocci-macro-file.h
> +++ b/scripts/cocci-macro-file.h
> @@ -23,7 +23,12 @@
> #define QEMU_NORETURN __attribute__ ((__noreturn__))
> #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
> #define QEMU_SENTINEL __attribute__((sentinel))
> -#define QEMU_PACKED __attribute__((gcc_struct, packed))
> +
> +#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
> +# define QEMU_PACKED __attribute__((gcc_struct, packed))
> +#else
> +# define QEMU_PACKED __attribute__((packed))
> +#endif
>
> #define cat(x,y) x ## y
> #define cat2(x,y) cat(x,y)
> diff --git a/slirp/src/util.h b/slirp/src/util.h
> index 01f1e0e068..278828fe3f 100644
> --- a/slirp/src/util.h
> +++ b/slirp/src/util.h
> @@ -43,7 +43,7 @@
> #include
> #endif
>
> -#if defined(_WIN32)
> +#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
> # define SLIRP_PACKED __attribute__((gcc_struct, packed))
> #else
> # define SLIRP_PACKED __attribute__((packed))
>
The slirp code is currently on its way into a separate module, so you
might need to provide that hunk to the libslirp folks again... I'm
putting the slirp maintainers on CC:, maybe they can pick it up from here.
Anyway:
Reviewed-by: Thomas Huth
