qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3
@ 2021-04-13 16:08 Paolo Bonzini
  2021-04-13 16:08 ` [PULL v2 1/3] osdep: include glib-compat.h before other QEMU headers Paolo Bonzini
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-04-13 16:08 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit c1e90def01bdb8fcbdbebd9d1eaa8e4827ece620:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210412' into staging (2021-04-12 12:12:09 +0100)

are available in the Git repository at:

  https://gitlab.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 1a0b186eaf3d1ce63dc7bf608d618b9ca62b6241:

  qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code (2021-04-13 18:04:23 +0200)

----------------------------------------------------------------
* Fix C++ compilation of qemu/osdep.h.
* Fix -object cryptodev-vhost-user

----------------------------------------------------------------
Paolo Bonzini (2):
      osdep: include glib-compat.h before other QEMU headers
      osdep: protect qemu/osdep.h with extern "C"

Thomas Huth (1):
      qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code

 disas/nanomips.cpp      |  2 +-
 include/qemu/compiler.h |  6 ++++++
 include/qemu/osdep.h    | 13 +++++++++++--
 qapi/qom.json           |  4 ++--
 4 files changed, 20 insertions(+), 5 deletions(-)
-- 
2.30.1



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PULL v2 1/3] osdep: include glib-compat.h before other QEMU headers
  2021-04-13 16:08 [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3 Paolo Bonzini
@ 2021-04-13 16:08 ` Paolo Bonzini
  2021-04-14 16:51   ` Philippe Mathieu-Daudé
  2021-04-14 17:00   ` Daniel P. Berrangé
  2021-04-13 16:08 ` [PULL v2 2/3] osdep: protect qemu/osdep.h with extern "C" Paolo Bonzini
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-04-13 16:08 UTC (permalink / raw)
  To: qemu-devel

glib-compat.h is sort of like a system header, and it needs to include
system headers (glib.h) that may dislike being included under
'extern "C"'.  Move it right after all system headers and before
all other QEMU headers.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/osdep.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index ba15be9c56..b67b0a1e8c 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -111,6 +111,8 @@ extern int daemon(int, int);
 #define WEXITSTATUS(x) (x)
 #endif
 
+#include "glib-compat.h"
+
 #ifdef _WIN32
 #include "sysemu/os-win32.h"
 #endif
@@ -123,7 +125,6 @@ extern int daemon(int, int);
 #include <AvailabilityMacros.h>
 #endif
 
-#include "glib-compat.h"
 #include "qemu/typedefs.h"
 
 /*
-- 
2.30.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL v2 2/3] osdep: protect qemu/osdep.h with extern "C"
  2021-04-13 16:08 [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3 Paolo Bonzini
  2021-04-13 16:08 ` [PULL v2 1/3] osdep: include glib-compat.h before other QEMU headers Paolo Bonzini
@ 2021-04-13 16:08 ` Paolo Bonzini
  2021-04-14 17:07   ` Daniel P. Berrangé
  2021-04-13 16:08 ` [PULL v2 3/3] qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code Paolo Bonzini
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2021-04-13 16:08 UTC (permalink / raw)
  To: qemu-devel

System headers may include templates if compiled with a C++ compiler,
which cause the compiler to complain if qemu/osdep.h is included
within a C++ source file's 'extern "C"' block.  Add
an 'extern "C"' block directly to qemu/osdep.h, so that
system headers can be kept out of it.

There is a stray declaration early in qemu/osdep.h, which needs
to be special cased.  Add a definition in qemu/compiler.h to
make it look nice.

config-host.h, CONFIG_TARGET, exec/poison.h and qemu/compiler.h
are included outside the 'extern "C"' block; that is not
an issue because they consist entirely of preprocessor directives.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 disas/nanomips.cpp      |  2 +-
 include/qemu/compiler.h |  6 ++++++
 include/qemu/osdep.h    | 10 +++++++++-
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
index 2b09655271..8ddef897f0 100644
--- a/disas/nanomips.cpp
+++ b/disas/nanomips.cpp
@@ -27,8 +27,8 @@
  *      Reference Manual", Revision 01.01, April 27, 2018
  */
 
-extern "C" {
 #include "qemu/osdep.h"
+extern "C" {
 #include "disas/dis-asm.h"
 }
 
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index cf28bb2bcd..091c45248b 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -11,6 +11,12 @@
 #define QEMU_STATIC_ANALYSIS 1
 #endif
 
+#ifdef __cplusplus
+#define QEMU_EXTERN_C extern "C"
+#else
+#define QEMU_EXTERN_C extern
+#endif
+
 #define QEMU_NORETURN __attribute__ ((__noreturn__))
 
 #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index b67b0a1e8c..3f8785a471 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -57,7 +57,7 @@
 #define daemon qemu_fake_daemon_function
 #include <stdlib.h>
 #undef daemon
-extern int daemon(int, int);
+QEMU_EXTERN_C int daemon(int, int);
 #endif
 
 #ifdef _WIN32
@@ -113,6 +113,10 @@ extern int daemon(int, int);
 
 #include "glib-compat.h"
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 #ifdef _WIN32
 #include "sysemu/os-win32.h"
 #endif
@@ -723,4 +727,8 @@ static inline int platform_does_not_support_system(const char *command)
 }
 #endif /* !HAVE_SYSTEM_FUNCTION */
 
+#ifdef __cplusplus
+}
+#endif
+
 #endif
-- 
2.30.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL v2 3/3] qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code
  2021-04-13 16:08 [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3 Paolo Bonzini
  2021-04-13 16:08 ` [PULL v2 1/3] osdep: include glib-compat.h before other QEMU headers Paolo Bonzini
  2021-04-13 16:08 ` [PULL v2 2/3] osdep: protect qemu/osdep.h with extern "C" Paolo Bonzini
@ 2021-04-13 16:08 ` Paolo Bonzini
  2021-04-13 17:05 ` [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3 no-reply
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2021-04-13 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth

From: Thomas Huth <thuth@redhat.com>

The ObjectType enum and ObjectOptions are included from qapi-types-qom.h
into common code. We should not use target-specific config switches like
CONFIG_VIRTIO_CRYPTO here, since this is not defined in common code and
thus the enum will look differently between common and target specific
code. For this case, it's hopefully enough to check for CONFIG_VHOST_CRYPTO
only (which is a host specific config switch, i.e. it's the same on all
targets).

Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20210412160710.639800-1-thuth@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qapi/qom.json | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index db5ac419b1..cd0e76d564 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -752,7 +752,7 @@
     'cryptodev-backend',
     'cryptodev-backend-builtin',
     { 'name': 'cryptodev-vhost-user',
-      'if': 'defined(CONFIG_VIRTIO_CRYPTO) && defined(CONFIG_VHOST_CRYPTO)' },
+      'if': 'defined(CONFIG_VHOST_CRYPTO)' },
     'dbus-vmstate',
     'filter-buffer',
     'filter-dump',
@@ -809,7 +809,7 @@
       'cryptodev-backend':          'CryptodevBackendProperties',
       'cryptodev-backend-builtin':  'CryptodevBackendProperties',
       'cryptodev-vhost-user':       { 'type': 'CryptodevVhostUserProperties',
-                                      'if': 'defined(CONFIG_VIRTIO_CRYPTO) && defined(CONFIG_VHOST_CRYPTO)' },
+                                      'if': 'defined(CONFIG_VHOST_CRYPTO)' },
       'dbus-vmstate':               'DBusVMStateProperties',
       'filter-buffer':              'FilterBufferProperties',
       'filter-dump':                'FilterDumpProperties',
-- 
2.30.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3
  2021-04-13 16:08 [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3 Paolo Bonzini
                   ` (2 preceding siblings ...)
  2021-04-13 16:08 ` [PULL v2 3/3] qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code Paolo Bonzini
@ 2021-04-13 17:05 ` no-reply
  2021-04-13 20:15 ` Peter Maydell
  2021-04-14 18:22 ` Peter Maydell
  5 siblings, 0 replies; 16+ messages in thread
From: no-reply @ 2021-04-13 17:05 UTC (permalink / raw)
  To: pbonzini; +Cc: qemu-devel

Patchew URL: https://patchew.org/QEMU/20210413160850.240064-1-pbonzini@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210413160850.240064-1-pbonzini@redhat.com
Subject: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
487722a qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code
588f61f osdep: protect qemu/osdep.h with extern "C"
5316327 osdep: include glib-compat.h before other QEMU headers

=== OUTPUT BEGIN ===
1/3 Checking commit 5316327519a7 (osdep: include glib-compat.h before other QEMU headers)
2/3 Checking commit 588f61fea9ae (osdep: protect qemu/osdep.h with extern "C")
WARNING: architecture specific defines should be avoided
#51: FILE: include/qemu/compiler.h:14:
+#ifdef __cplusplus

ERROR: storage class should be at the beginning of the declaration
#52: FILE: include/qemu/compiler.h:15:
+#define QEMU_EXTERN_C extern "C"

ERROR: storage class should be at the beginning of the declaration
#54: FILE: include/qemu/compiler.h:17:
+#define QEMU_EXTERN_C extern

WARNING: architecture specific defines should be avoided
#77: FILE: include/qemu/osdep.h:116:
+#ifdef __cplusplus

WARNING: architecture specific defines should be avoided
#88: FILE: include/qemu/osdep.h:730:
+#ifdef __cplusplus

total: 2 errors, 3 warnings, 47 lines checked

Patch 2/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/3 Checking commit 487722a3d4ef (qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210413160850.240064-1-pbonzini@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3
  2021-04-13 16:08 [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3 Paolo Bonzini
                   ` (3 preceding siblings ...)
  2021-04-13 17:05 ` [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3 no-reply
@ 2021-04-13 20:15 ` Peter Maydell
  2021-04-14 12:10   ` Peter Maydell
  2021-04-14 18:22 ` Peter Maydell
  5 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2021-04-13 20:15 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On Tue, 13 Apr 2021 at 17:18, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The following changes since commit c1e90def01bdb8fcbdbebd9d1eaa8e4827ece620:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210412' into staging (2021-04-12 12:12:09 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 1a0b186eaf3d1ce63dc7bf608d618b9ca62b6241:
>
>   qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code (2021-04-13 18:04:23 +0200)
>
> ----------------------------------------------------------------
> * Fix C++ compilation of qemu/osdep.h.
> * Fix -object cryptodev-vhost-user
>
> ----------------------------------------------------------------
> Paolo Bonzini (2):
>       osdep: include glib-compat.h before other QEMU headers
>       osdep: protect qemu/osdep.h with extern "C"
>
> Thomas Huth (1):
>       qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code

This still seems to have the same version of patch 2 that I gave
review comments on, and which you haven't replied to ?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3
  2021-04-13 20:15 ` Peter Maydell
@ 2021-04-14 12:10   ` Peter Maydell
  0 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2021-04-14 12:10 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On Tue, 13 Apr 2021 at 21:15, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 13 Apr 2021 at 17:18, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > The following changes since commit c1e90def01bdb8fcbdbebd9d1eaa8e4827ece620:
> >
> >   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210412' into staging (2021-04-12 12:12:09 +0100)
> >
> > are available in the Git repository at:
> >
> >   https://gitlab.com/bonzini/qemu.git tags/for-upstream
> >
> > for you to fetch changes up to 1a0b186eaf3d1ce63dc7bf608d618b9ca62b6241:
> >
> >   qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code (2021-04-13 18:04:23 +0200)
> >
> > ----------------------------------------------------------------
> > * Fix C++ compilation of qemu/osdep.h.
> > * Fix -object cryptodev-vhost-user
> >
> > ----------------------------------------------------------------
> > Paolo Bonzini (2):
> >       osdep: include glib-compat.h before other QEMU headers
> >       osdep: protect qemu/osdep.h with extern "C"
> >
> > Thomas Huth (1):
> >       qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code
>
> This still seems to have the same version of patch 2 that I gave
> review comments on, and which you haven't replied to ?

Ping? I'd like to tag rc3 today... I guess we could just leave out
the extern C related changes, as they're not a regression since 5.2.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL v2 1/3] osdep: include glib-compat.h before other QEMU headers
  2021-04-13 16:08 ` [PULL v2 1/3] osdep: include glib-compat.h before other QEMU headers Paolo Bonzini
@ 2021-04-14 16:51   ` Philippe Mathieu-Daudé
  2021-04-14 17:00   ` Daniel P. Berrangé
  1 sibling, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-14 16:51 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

On 4/13/21 6:08 PM, Paolo Bonzini wrote:
> glib-compat.h is sort of like a system header, and it needs to include
> system headers (glib.h) that may dislike being included under
> 'extern "C"'.  Move it right after all system headers and before
> all other QEMU headers.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/osdep.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index ba15be9c56..b67b0a1e8c 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -111,6 +111,8 @@ extern int daemon(int, int);
>  #define WEXITSTATUS(x) (x)
>  #endif
>  

Maybe worth a comment "must be after system headers and before other
QEMU headers" so we don't move it again by mistake.

> +#include "glib-compat.h"
> +
>  #ifdef _WIN32
>  #include "sysemu/os-win32.h"
>  #endif
> @@ -123,7 +125,6 @@ extern int daemon(int, int);
>  #include <AvailabilityMacros.h>
>  #endif
>  
> -#include "glib-compat.h"
>  #include "qemu/typedefs.h"
>  
>  /*
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL v2 1/3] osdep: include glib-compat.h before other QEMU headers
  2021-04-13 16:08 ` [PULL v2 1/3] osdep: include glib-compat.h before other QEMU headers Paolo Bonzini
  2021-04-14 16:51   ` Philippe Mathieu-Daudé
@ 2021-04-14 17:00   ` Daniel P. Berrangé
  1 sibling, 0 replies; 16+ messages in thread
From: Daniel P. Berrangé @ 2021-04-14 17:00 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On Tue, Apr 13, 2021 at 06:08:48PM +0200, Paolo Bonzini wrote:
> glib-compat.h is sort of like a system header, and it needs to include
> system headers (glib.h) that may dislike being included under
> 'extern "C"'.  Move it right after all system headers and before
> all other QEMU headers.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/osdep.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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] 16+ messages in thread

* Re: [PULL v2 2/3] osdep: protect qemu/osdep.h with extern "C"
  2021-04-13 16:08 ` [PULL v2 2/3] osdep: protect qemu/osdep.h with extern "C" Paolo Bonzini
@ 2021-04-14 17:07   ` Daniel P. Berrangé
  2021-04-14 18:39     ` Peter Maydell
  2021-04-14 18:50     ` Peter Maydell
  0 siblings, 2 replies; 16+ messages in thread
From: Daniel P. Berrangé @ 2021-04-14 17:07 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On Tue, Apr 13, 2021 at 06:08:49PM +0200, Paolo Bonzini wrote:
> System headers may include templates if compiled with a C++ compiler,
> which cause the compiler to complain if qemu/osdep.h is included
> within a C++ source file's 'extern "C"' block.  Add
> an 'extern "C"' block directly to qemu/osdep.h, so that
> system headers can be kept out of it.
> 
> There is a stray declaration early in qemu/osdep.h, which needs
> to be special cased.  Add a definition in qemu/compiler.h to
> make it look nice.
> 
> config-host.h, CONFIG_TARGET, exec/poison.h and qemu/compiler.h
> are included outside the 'extern "C"' block; that is not
> an issue because they consist entirely of preprocessor directives.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  disas/nanomips.cpp      |  2 +-
>  include/qemu/compiler.h |  6 ++++++
>  include/qemu/osdep.h    | 10 +++++++++-
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
> index 2b09655271..8ddef897f0 100644
> --- a/disas/nanomips.cpp
> +++ b/disas/nanomips.cpp
> @@ -27,8 +27,8 @@
>   *      Reference Manual", Revision 01.01, April 27, 2018
>   */
>  
> -extern "C" {
>  #include "qemu/osdep.h"
> +extern "C" {
>  #include "disas/dis-asm.h"
>  }

disas/arm-a64.c  also has an 'extern "C"' block around
an include of qemu/osdep.h.   Do we need a similar
fix for that file, or are we no longer using that
bit of code ?

>  
> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> index cf28bb2bcd..091c45248b 100644
> --- a/include/qemu/compiler.h
> +++ b/include/qemu/compiler.h
> @@ -11,6 +11,12 @@
>  #define QEMU_STATIC_ANALYSIS 1
>  #endif
>  
> +#ifdef __cplusplus
> +#define QEMU_EXTERN_C extern "C"
> +#else
> +#define QEMU_EXTERN_C extern
> +#endif
> +
>  #define QEMU_NORETURN __attribute__ ((__noreturn__))
>  
>  #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index b67b0a1e8c..3f8785a471 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -57,7 +57,7 @@
>  #define daemon qemu_fake_daemon_function
>  #include <stdlib.h>
>  #undef daemon
> -extern int daemon(int, int);
> +QEMU_EXTERN_C int daemon(int, int);
>  #endif
>  
>  #ifdef _WIN32
> @@ -113,6 +113,10 @@ extern int daemon(int, int);
>  
>  #include "glib-compat.h"
>  
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
>  #ifdef _WIN32
>  #include "sysemu/os-win32.h"

This and os-posix.h both include other system headers. We don't currently
have problem, so this is ok as the minimal fix for 6.0, but long term we
need more work on this header to further narrow the extern {} block.

So assuming my question about disas/arm-a64.c is a non-issue, then


Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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] 16+ messages in thread

* Re: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3
  2021-04-13 16:08 [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3 Paolo Bonzini
                   ` (4 preceding siblings ...)
  2021-04-13 20:15 ` Peter Maydell
@ 2021-04-14 18:22 ` Peter Maydell
  2021-04-15  5:57   ` Markus Armbruster
  5 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2021-04-14 18:22 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On Tue, 13 Apr 2021 at 17:18, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The following changes since commit c1e90def01bdb8fcbdbebd9d1eaa8e4827ece620:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210412' into staging (2021-04-12 12:12:09 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 1a0b186eaf3d1ce63dc7bf608d618b9ca62b6241:
>
>   qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code (2021-04-13 18:04:23 +0200)
>
> ----------------------------------------------------------------
> * Fix C++ compilation of qemu/osdep.h.
> * Fix -object cryptodev-vhost-user
>
> ----------------------------------------------------------------
> Paolo Bonzini (2):
>       osdep: include glib-compat.h before other QEMU headers
>       osdep: protect qemu/osdep.h with extern "C"
>
> Thomas Huth (1):
>       qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code

Given Dan's review, I think that the osdep patches need another
revision. So my plan is to cherry-pick the CONFIG_VIRTIO_CRYPTO
patch here and tag rc3 with just that. If we need an rc4 (which
on our current track record is not unlikely) we can put in some
version of the osdep patches; if not, this isn't a regression
since 5.2 so I'm happy releasing 6.0 with it still present.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL v2 2/3] osdep: protect qemu/osdep.h with extern "C"
  2021-04-14 17:07   ` Daniel P. Berrangé
@ 2021-04-14 18:39     ` Peter Maydell
  2021-04-14 18:50     ` Peter Maydell
  1 sibling, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2021-04-14 18:39 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: Paolo Bonzini, QEMU Developers

On Wed, 14 Apr 2021 at 18:26, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Tue, Apr 13, 2021 at 06:08:49PM +0200, Paolo Bonzini wrote:
> > System headers may include templates if compiled with a C++ compiler,
> > which cause the compiler to complain if qemu/osdep.h is included
> > within a C++ source file's 'extern "C"' block.  Add
> > an 'extern "C"' block directly to qemu/osdep.h, so that
> > system headers can be kept out of it.
> >
> > There is a stray declaration early in qemu/osdep.h, which needs
> > to be special cased.  Add a definition in qemu/compiler.h to
> > make it look nice.
> >
> > config-host.h, CONFIG_TARGET, exec/poison.h and qemu/compiler.h
> > are included outside the 'extern "C"' block; that is not
> > an issue because they consist entirely of preprocessor directives.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  disas/nanomips.cpp      |  2 +-
> >  include/qemu/compiler.h |  6 ++++++
> >  include/qemu/osdep.h    | 10 +++++++++-
> >  3 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
> > index 2b09655271..8ddef897f0 100644
> > --- a/disas/nanomips.cpp
> > +++ b/disas/nanomips.cpp
> > @@ -27,8 +27,8 @@
> >   *      Reference Manual", Revision 01.01, April 27, 2018
> >   */
> >
> > -extern "C" {
> >  #include "qemu/osdep.h"
> > +extern "C" {
> >  #include "disas/dis-asm.h"
> >  }

> This and os-posix.h both include other system headers. We don't currently
> have problem, so this is ok as the minimal fix for 6.0, but long term we
> need more work on this header to further narrow the extern {} block.

The other path where we can include system headers inside extern "C"
is that the code above still has dis-asm.h inside the extern C block,
but dis-asm.h includes qemu/bswap.h (midway down the file!) and bswap.h
in turn includes some system headers.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL v2 2/3] osdep: protect qemu/osdep.h with extern "C"
  2021-04-14 17:07   ` Daniel P. Berrangé
  2021-04-14 18:39     ` Peter Maydell
@ 2021-04-14 18:50     ` Peter Maydell
  2021-04-15  9:20       ` Daniel P. Berrangé
  1 sibling, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2021-04-14 18:50 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: Paolo Bonzini, QEMU Developers

On Wed, 14 Apr 2021 at 18:26, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Tue, Apr 13, 2021 at 06:08:49PM +0200, Paolo Bonzini wrote:
> >  #ifdef _WIN32
> >  #include "sysemu/os-win32.h"
>
> This and os-posix.h both include other system headers. We don't currently
> have problem, so this is ok as the minimal fix for 6.0, but long term we
> need more work on this header to further narrow the extern {} block.

Maybe we should just move all the system header includes out of
both os-posix.h and os-win32.h ? We already have one header file
we've treated that way (sys/wait.h).

Alternatively we could leave os-win32.h and os-posix.h outside
osdep.h's extern block, and require that they both use an
extern block themselves for their declarations.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3
  2021-04-14 18:22 ` Peter Maydell
@ 2021-04-15  5:57   ` Markus Armbruster
  2021-04-15 10:48     ` Peter Maydell
  0 siblings, 1 reply; 16+ messages in thread
From: Markus Armbruster @ 2021-04-15  5:57 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Thomas Huth, QEMU Developers

Peter Maydell <peter.maydell@linaro.org> writes:

> On Tue, 13 Apr 2021 at 17:18, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> The following changes since commit c1e90def01bdb8fcbdbebd9d1eaa8e4827ece620:
>>
>>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210412' into staging (2021-04-12 12:12:09 +0100)
>>
>> are available in the Git repository at:
>>
>>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
>>
>> for you to fetch changes up to 1a0b186eaf3d1ce63dc7bf608d618b9ca62b6241:
>>
>>   qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code (2021-04-13 18:04:23 +0200)
>>
>> ----------------------------------------------------------------
>> * Fix C++ compilation of qemu/osdep.h.
>> * Fix -object cryptodev-vhost-user
>>
>> ----------------------------------------------------------------
>> Paolo Bonzini (2):
>>       osdep: include glib-compat.h before other QEMU headers
>>       osdep: protect qemu/osdep.h with extern "C"
>>
>> Thomas Huth (1):
>>       qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code
>
> Given Dan's review, I think that the osdep patches need another
> revision. So my plan is to cherry-pick the CONFIG_VIRTIO_CRYPTO
> patch here and tag rc3 with just that. If we need an rc4 (which

Uh, I had a question on that one:

Message-ID: <87tuo9j7hw.fsf@dusky.pond.sub.org>
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg02341.html

> on our current track record is not unlikely) we can put in some
> version of the osdep patches; if not, this isn't a regression
> since 5.2 so I'm happy releasing 6.0 with it still present.
>
> thanks
> -- PMM



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL v2 2/3] osdep: protect qemu/osdep.h with extern "C"
  2021-04-14 18:50     ` Peter Maydell
@ 2021-04-15  9:20       ` Daniel P. Berrangé
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel P. Berrangé @ 2021-04-15  9:20 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, QEMU Developers

On Wed, Apr 14, 2021 at 07:50:41PM +0100, Peter Maydell wrote:
> On Wed, 14 Apr 2021 at 18:26, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Tue, Apr 13, 2021 at 06:08:49PM +0200, Paolo Bonzini wrote:
> > >  #ifdef _WIN32
> > >  #include "sysemu/os-win32.h"
> >
> > This and os-posix.h both include other system headers. We don't currently
> > have problem, so this is ok as the minimal fix for 6.0, but long term we
> > need more work on this header to further narrow the extern {} block.
> 
> Maybe we should just move all the system header includes out of
> both os-posix.h and os-win32.h ? We already have one header file
> we've treated that way (sys/wait.h).
> 
> Alternatively we could leave os-win32.h and os-posix.h outside
> osdep.h's extern block, and require that they both use an
> extern block themselves for their declarations.

I'd be inclined towards the latter as I tihnk its reasonable for
os-win32/posix.h  to want to include system headers.


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] 16+ messages in thread

* Re: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3
  2021-04-15  5:57   ` Markus Armbruster
@ 2021-04-15 10:48     ` Peter Maydell
  0 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2021-04-15 10:48 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Paolo Bonzini, Thomas Huth, QEMU Developers

On Thu, 15 Apr 2021 at 06:57, Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Tue, 13 Apr 2021 at 17:18, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >> Paolo Bonzini (2):
> >>       osdep: include glib-compat.h before other QEMU headers
> >>       osdep: protect qemu/osdep.h with extern "C"
> >>
> >> Thomas Huth (1):
> >>       qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code
> >
> > Given Dan's review, I think that the osdep patches need another
> > revision. So my plan is to cherry-pick the CONFIG_VIRTIO_CRYPTO
> > patch here and tag rc3 with just that. If we need an rc4 (which
>
> Uh, I had a question on that one:
>
> Message-ID: <87tuo9j7hw.fsf@dusky.pond.sub.org>
> https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg02341.html

Sorry, I missed that. Let me know if the discussion ends up concluding
that we should revert this change for 6.0.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-04-15 10:50 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-13 16:08 [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3 Paolo Bonzini
2021-04-13 16:08 ` [PULL v2 1/3] osdep: include glib-compat.h before other QEMU headers Paolo Bonzini
2021-04-14 16:51   ` Philippe Mathieu-Daudé
2021-04-14 17:00   ` Daniel P. Berrangé
2021-04-13 16:08 ` [PULL v2 2/3] osdep: protect qemu/osdep.h with extern "C" Paolo Bonzini
2021-04-14 17:07   ` Daniel P. Berrangé
2021-04-14 18:39     ` Peter Maydell
2021-04-14 18:50     ` Peter Maydell
2021-04-15  9:20       ` Daniel P. Berrangé
2021-04-13 16:08 ` [PULL v2 3/3] qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code Paolo Bonzini
2021-04-13 17:05 ` [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3 no-reply
2021-04-13 20:15 ` Peter Maydell
2021-04-14 12:10   ` Peter Maydell
2021-04-14 18:22 ` Peter Maydell
2021-04-15  5:57   ` Markus Armbruster
2021-04-15 10:48     ` Peter Maydell

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).