Re: [PATCH v9 23/23] plugins: Support C++

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Oct 12, 2023 at 01:42:04AM +0900, Akihiko Odaki wrote:
> On 2023/10/12 1:21, Thomas Huth wrote:
> > On 11/10/2023 17.48, Akihiko Odaki wrote:
> > > On 2023/10/11 17:51, Daniel P. Berrangé wrote:
> > > > On Wed, Oct 11, 2023 at 04:03:09PM +0900, Akihiko Odaki wrote:
> > > > > Make qemu-plugin.h consumable for C++ platform.
> > > > > 
> > > > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > > > > ---
> > > > >   docs/devel/tcg-plugins.rst |  4 ++++
> > > > >   meson.build                |  2 +-
> > > > >   include/qemu/qemu-plugin.h |  4 ++++
> > > > >   tests/plugin/cc.cc         | 16 ++++++++++++++++
> > > > >   tests/plugin/meson.build   |  5 +++++
> > > > >   tests/tcg/Makefile.target  |  3 +--
> > > > >   6 files changed, 31 insertions(+), 3 deletions(-)
> > > > >   create mode 100644 tests/plugin/cc.cc
> > > > > 
> > > > > diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
> > > > > index c9f8b27590..984d8012e9 100644
> > > > > --- a/docs/devel/tcg-plugins.rst
> > > > > +++ b/docs/devel/tcg-plugins.rst
> > > > > @@ -283,6 +283,10 @@ run::
> > > > >     160          1      0
> > > > >     135          1      0
> > > > > +- contrib/plugins/cc.cc
> > > > > +
> > > > > +A pure test plugin to ensure that the plugin API is
> > > > > compatible with C++.
> > > > > +
> > > > 
> > > > IMHO we don't need to be adding a test just to prove the
> > > > existance of the G_BEGIN_DECLS/G_END_DECLS macros in the
> > > > plugin header.
> > > 
> > > Strictly speaking, if you include this header file from C++, the
> > > code will be interpreted as C++ instead of C but with C linkage.
> > > That worries me that the header file may get something not allowed
> > > in C++ in the future.
> > 
> > I think it should be enough if you add the G_BEGIN_DECLS/G_END_DECLS
> > macros here. QEMU is a C project, and it was quite difficult to get rid
[> > of the C++ code again, so please don't soften the check in meson.build
> > and don't introduce new .cc files.
> > If you have some code somewhere that uses C++ for plugins, I think it
> > would be better to add a regression test there instead.
> 
> It doesn't sound right to test the upstream header file in a downstream
> project. It will take time until the fix becomes readily usable for the
> downstream even if the downstream finds a bug.

Essentially QEMU is explicitly saying that C++ support is not a core
deliverable of the project. QEMU will accept patches to fix problems
but won't put any effort into C++ beyond that.

In such a scenario it is appropriate for a downstream to do testing
itself. The delay between finding a problem and sending a fix does
not need to be big - it could easily be less than a day if there is
a nightly CI job that tests against latest QEMU git master.

Distributing the testing burden is more scalable as QEMU also does
not have the resources to test every scenario it wants to. This kind
of situation already exists with the Xen project, which does CI against
QEMU on an ongoing basis to identify and report bugs that affect Xen
in scenarios which QEMU does not test. Libvirt also runs CI against
QEMU to detect regressions in QEMU which impact libvirt's usage of
QEMU, that QEMU's own CI won't catch.

All that not withstanding, while you are right that someone might
accidentally introduce something in the header that is not compatible
with C++, the actual chances of this are low. This plugin header file
is small, self contained, and is not undergoing a high volume of
change.

With 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 :|