* [PATCH v2 0/2] buildsys: Fix building with SASL on Windows
@ 2020-03-09 12:24 Philippe Mathieu-Daudé
2020-03-09 12:24 ` [PATCH v2 1/2] configure: " Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-09 12:24 UTC (permalink / raw)
To: qemu-devel
Cc: Fam Zheng, Daniel P . Berrangé, Philippe Mathieu-Daudé,
Алексей
Павлов,
Метлицкий
Юрий
Викторович,
Biswapriyo Nath, Paolo Bonzini, Alex Bennée
Fix a bug reported by Youry few months ago:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg625606.html
Since v1:
- addressed Daniel review (always define STRUCT_IOVEC_DEFINED)
The Fedora docker image already uses the libsasl since commit
8ea5962f286. Add the similar package to the Debian (host) image.
Philippe Mathieu-Daudé (2):
configure: Fix building with SASL on Windows
tests/docker: Install SASL library to extend code coverage on amd64
configure | 4 +++-
tests/docker/dockerfiles/debian-amd64.docker | 1 +
2 files changed, 4 insertions(+), 1 deletion(-)
--
2.21.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] configure: Fix building with SASL on Windows
2020-03-09 12:24 [PATCH v2 0/2] buildsys: Fix building with SASL on Windows Philippe Mathieu-Daudé
@ 2020-03-09 12:24 ` Philippe Mathieu-Daudé
2020-03-09 12:30 ` Daniel P. Berrangé
2020-03-09 12:24 ` [PATCH v2 2/2] tests/docker: Install SASL library to extend code coverage on amd64 Philippe Mathieu-Daudé
2020-03-11 18:38 ` [PATCH v2 0/2] buildsys: Fix building with SASL on Windows Paolo Bonzini
2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-09 12:24 UTC (permalink / raw)
To: qemu-devel
Cc: Fam Zheng, Alexey Pavlov, Daniel P . Berrangé,
Philippe Mathieu-Daudé,
Алексей
Павлов,
Метлицкий
Юрий
Викторович,
Biswapriyo Nath, Paolo Bonzini, Alex Bennée,
Youry Metlitsky
The Simple Authentication and Security Layer (SASL) library
re-defines the struct iovec on Win32 [*]. QEMU also re-defines
it in "qemu/osdep.h". The two definitions then clash on a MinGW
build.
We can avoid the SASL definition by defining STRUCT_IOVEC_DEFINED.
Since QEMU already defines 'struct iovec' if it is missing, add
the definition to vnc_sasl_cflags to avoid SASL re-defining it.
[*] https://github.com/cyrusimap/cyrus-sasl/blob/cyrus-sasl-2.1.27/include/sasl.h#L187
Cc: Alexey Pavlov <alexpux@gmail.com>
Cc: Biswapriyo Nath <nathbappai@gmail.com>
Reported-by: Youry Metlitsky <winaes@yandex.ru>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2:
Since QEMU provides 'struct iovec' if missing, always define
STRUCT_IOVEC_DEFINED (danpb review).
---
configure | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index cbf864bff1..a4cd4bccfb 100755
--- a/configure
+++ b/configure
@@ -3349,7 +3349,9 @@ if test "$vnc" = "yes" && test "$vnc_sasl" != "no" ; then
int main(void) { sasl_server_init(NULL, "qemu"); return 0; }
EOF
# Assuming Cyrus-SASL installed in /usr prefix
- vnc_sasl_cflags=""
+ # QEMU defines struct iovec in "qemu/osdep.h",
+ # we don't want libsasl to redefine it in <sasl/sasl.h>.
+ vnc_sasl_cflags="-DSTRUCT_IOVEC_DEFINED"
vnc_sasl_libs="-lsasl2"
if compile_prog "$vnc_sasl_cflags" "$vnc_sasl_libs" ; then
vnc_sasl=yes
--
2.21.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] tests/docker: Install SASL library to extend code coverage on amd64
2020-03-09 12:24 [PATCH v2 0/2] buildsys: Fix building with SASL on Windows Philippe Mathieu-Daudé
2020-03-09 12:24 ` [PATCH v2 1/2] configure: " Philippe Mathieu-Daudé
@ 2020-03-09 12:24 ` Philippe Mathieu-Daudé
2020-03-11 18:38 ` [PATCH v2 0/2] buildsys: Fix building with SASL on Windows Paolo Bonzini
2 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-09 12:24 UTC (permalink / raw)
To: qemu-devel
Cc: Fam Zheng, Daniel P . Berrangé, Philippe Mathieu-Daudé,
Алексей
Павлов,
Метлицкий
Юрий
Викторович,
Biswapriyo Nath, Paolo Bonzini, Alex Bennée
Install the SASL library to build the VNC SASL auth protocol code.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/docker/dockerfiles/debian-amd64.docker | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/docker/dockerfiles/debian-amd64.docker b/tests/docker/dockerfiles/debian-amd64.docker
index 3b860af106..0456fc7a0c 100644
--- a/tests/docker/dockerfiles/debian-amd64.docker
+++ b/tests/docker/dockerfiles/debian-amd64.docker
@@ -17,6 +17,7 @@ RUN apt update && \
libbz2-dev \
liblzo2-dev \
librdmacm-dev \
+ libsasl2-dev \
libsnappy-dev \
libvte-dev
--
2.21.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] configure: Fix building with SASL on Windows
2020-03-09 12:24 ` [PATCH v2 1/2] configure: " Philippe Mathieu-Daudé
@ 2020-03-09 12:30 ` Daniel P. Berrangé
2020-03-09 12:43 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Daniel P. Berrangé @ 2020-03-09 12:30 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Fam Zheng, Alexey Pavlov,
Алексей
Павлов,
qemu-devel,
Метлицкий
Юрий
Викторович,
Biswapriyo Nath, Paolo Bonzini, Alex Bennée,
Youry Metlitsky
On Mon, Mar 09, 2020 at 01:24:53PM +0100, Philippe Mathieu-Daudé wrote:
> The Simple Authentication and Security Layer (SASL) library
> re-defines the struct iovec on Win32 [*]. QEMU also re-defines
> it in "qemu/osdep.h". The two definitions then clash on a MinGW
> build.
> We can avoid the SASL definition by defining STRUCT_IOVEC_DEFINED.
> Since QEMU already defines 'struct iovec' if it is missing, add
> the definition to vnc_sasl_cflags to avoid SASL re-defining it.
>
> [*] https://github.com/cyrusimap/cyrus-sasl/blob/cyrus-sasl-2.1.27/include/sasl.h#L187
>
> Cc: Alexey Pavlov <alexpux@gmail.com>
> Cc: Biswapriyo Nath <nathbappai@gmail.com>
> Reported-by: Youry Metlitsky <winaes@yandex.ru>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2:
> Since QEMU provides 'struct iovec' if missing, always define
> STRUCT_IOVEC_DEFINED (danpb review).
> ---
> configure | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index cbf864bff1..a4cd4bccfb 100755
> --- a/configure
> +++ b/configure
> @@ -3349,7 +3349,9 @@ if test "$vnc" = "yes" && test "$vnc_sasl" != "no" ; then
> int main(void) { sasl_server_init(NULL, "qemu"); return 0; }
> EOF
> # Assuming Cyrus-SASL installed in /usr prefix
> - vnc_sasl_cflags=""
> + # QEMU defines struct iovec in "qemu/osdep.h",
> + # we don't want libsasl to redefine it in <sasl/sasl.h>.
> + vnc_sasl_cflags="-DSTRUCT_IOVEC_DEFINED"
> vnc_sasl_libs="-lsasl2"
> if compile_prog "$vnc_sasl_cflags" "$vnc_sasl_libs" ; then
> vnc_sasl=yes
This works so:
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
however, I'm wondering if we'd be better doing this in a more
localized place. This applies to everything we compile, but
only one place imports sasl.h, so should we instead do
#define STRUCT_IOVEC_DEFINED
#include <sasl/saslh.>
in vnc-auth-sasl.h, so we localize the namespace pollution.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] configure: Fix building with SASL on Windows
2020-03-09 12:30 ` Daniel P. Berrangé
@ 2020-03-09 12:43 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-09 12:43 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Fam Zheng, Alexey Pavlov,
Алексей
Павлов,
qemu-devel,
Метлицкий
Юрий
Викторович,
Biswapriyo Nath, Paolo Bonzini, Alex Bennée,
Youry Metlitsky
On 3/9/20 1:30 PM, Daniel P. Berrangé wrote:
> On Mon, Mar 09, 2020 at 01:24:53PM +0100, Philippe Mathieu-Daudé wrote:
>> The Simple Authentication and Security Layer (SASL) library
>> re-defines the struct iovec on Win32 [*]. QEMU also re-defines
>> it in "qemu/osdep.h". The two definitions then clash on a MinGW
>> build.
>> We can avoid the SASL definition by defining STRUCT_IOVEC_DEFINED.
>> Since QEMU already defines 'struct iovec' if it is missing, add
>> the definition to vnc_sasl_cflags to avoid SASL re-defining it.
>>
>> [*] https://github.com/cyrusimap/cyrus-sasl/blob/cyrus-sasl-2.1.27/include/sasl.h#L187
>>
>> Cc: Alexey Pavlov <alexpux@gmail.com>
>> Cc: Biswapriyo Nath <nathbappai@gmail.com>
>> Reported-by: Youry Metlitsky <winaes@yandex.ru>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> v2:
>> Since QEMU provides 'struct iovec' if missing, always define
>> STRUCT_IOVEC_DEFINED (danpb review).
>> ---
>> configure | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index cbf864bff1..a4cd4bccfb 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3349,7 +3349,9 @@ if test "$vnc" = "yes" && test "$vnc_sasl" != "no" ; then
>> int main(void) { sasl_server_init(NULL, "qemu"); return 0; }
>> EOF
>> # Assuming Cyrus-SASL installed in /usr prefix
>> - vnc_sasl_cflags=""
>> + # QEMU defines struct iovec in "qemu/osdep.h",
>> + # we don't want libsasl to redefine it in <sasl/sasl.h>.
>> + vnc_sasl_cflags="-DSTRUCT_IOVEC_DEFINED"
>> vnc_sasl_libs="-lsasl2"
>> if compile_prog "$vnc_sasl_cflags" "$vnc_sasl_libs" ; then
>> vnc_sasl=yes
>
> This works so:
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>
> however, I'm wondering if we'd be better doing this in a more
> localized place. This applies to everything we compile, but
> only one place imports sasl.h, so should we instead do
>
> #define STRUCT_IOVEC_DEFINED
> #include <sasl/saslh.>
>
> in vnc-auth-sasl.h, so we localize the namespace pollution.
Good idea, but we'll need to remember this fix if we ever use
<sasl/sasl.h> in another header.
>
> Regards,
> Daniel
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] buildsys: Fix building with SASL on Windows
2020-03-09 12:24 [PATCH v2 0/2] buildsys: Fix building with SASL on Windows Philippe Mathieu-Daudé
2020-03-09 12:24 ` [PATCH v2 1/2] configure: " Philippe Mathieu-Daudé
2020-03-09 12:24 ` [PATCH v2 2/2] tests/docker: Install SASL library to extend code coverage on amd64 Philippe Mathieu-Daudé
@ 2020-03-11 18:38 ` Paolo Bonzini
2 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2020-03-11 18:38 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Fam Zheng, Daniel P . Berrangé,
Алексей
Павлов,
Метлицкий
Юрий
Викторович,
Biswapriyo Nath, Alex Bennée
On 09/03/20 13:24, Philippe Mathieu-Daudé wrote:
> Fix a bug reported by Youry few months ago:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg625606.html
>
> Since v1:
> - addressed Daniel review (always define STRUCT_IOVEC_DEFINED)
>
> The Fedora docker image already uses the libsasl since commit
> 8ea5962f286. Add the similar package to the Debian (host) image.
>
> Philippe Mathieu-Daudé (2):
> configure: Fix building with SASL on Windows
> tests/docker: Install SASL library to extend code coverage on amd64
>
> configure | 4 +++-
> tests/docker/dockerfiles/debian-amd64.docker | 1 +
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-11 18:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-09 12:24 [PATCH v2 0/2] buildsys: Fix building with SASL on Windows Philippe Mathieu-Daudé
2020-03-09 12:24 ` [PATCH v2 1/2] configure: " Philippe Mathieu-Daudé
2020-03-09 12:30 ` Daniel P. Berrangé
2020-03-09 12:43 ` Philippe Mathieu-Daudé
2020-03-09 12:24 ` [PATCH v2 2/2] tests/docker: Install SASL library to extend code coverage on amd64 Philippe Mathieu-Daudé
2020-03-11 18:38 ` [PATCH v2 0/2] buildsys: Fix building with SASL on Windows Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.