qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Thomas Huth" <thuth@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Michael Roth" <michael.roth@amd.com>
Subject: Re: [PATCH 5/6] qapi: apply schema prefix to QAPI feature enum constants
Date: Mon, 5 Aug 2024 14:24:57 +0100	[thread overview]
Message-ID: <ZrDSqXtiT2U7zS-p@redhat.com> (raw)
In-Reply-To: <87a5hrm7wv.fsf@pond.sub.org>

On Mon, Aug 05, 2024 at 03:11:12PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Mon, Aug 05, 2024 at 02:22:47PM +0200, Markus Armbruster wrote:
> >> Daniel P. Berrangé <berrange@redhat.com> writes:
> >> 
> >> > This allows us to include multiple QAPI schemas in the same file.
> >> >
> >> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> >> 
> >> I figure you had reason to simultaneously include headers generated for
> >> multiple schemas.  Do tell :)
> >
> > I didn't want to have this patch, but the unit tests do this :-(
> >
> > [2/37] Compiling C object tests/libtestqapi.a.p/meson-generated_.._test-qapi-commands-sub-sub-module.c.o
> > FAILED: tests/libtestqapi.a.p/meson-generated_.._test-qapi-commands-sub-sub-module.c.o 
> > cc -m64 -Itests/libtestqapi.a.p -Itests -I../tests -I. -Iqapi -Itrace -Iui -Iui/shader -Itests/include -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-6 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -Wempty-body -Wendif-labels -Wexpansion-to-defined -Wformat-security -Wformat-y2k -Wignored-qualifiers -Wimplicit-fallthrough=2 -Winit-self -Wmissing-format-attribute -Wmissing-prototypes -Wnested-externs -Wold-style-declaration -Wold-style-definition -Wredundant-decls -Wshadow=local -Wstrict-prototypes -Wtype-limits -Wundef -Wvla -Wwrite-strings -Wno-missing-include-dirs -Wno-psabi -Wno-shift-negative-value -isystem /var/home/berrange/src/virt/qemu/linux-headers -isystem linux-headers -iquote . -iquote /var/home/berrange/src/virt/qemu -iquote /var/home/berrange/src/virt/qemu/include -iquote /var/home/berrange/src/virt/qemu/host/include/x86_64 -iquote /var/home/berrange/src/virt/qemu/host/include/generic -iquote /var/home/berrange/src/virt/qemu/tcg/i386 -pthread -msse2 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -ftrivial-auto-var-init=zero -fzero-call-used-regs=used-gpr -fPIE -MD -MQ tests/libtestqapi.a.p/meson-generated_.._test-qapi-commands-sub-sub-module.c.o -MF tests/libtestqapi.a.p/meson-generated_.._test-qapi-commands-sub-sub-module.c.o.d -o tests/libtestqapi.a.p/meson-generated_.._test-qapi-commands-sub-sub-module.c.o -c tests/test-qapi-commands-sub-sub-module.c
> > In file included from tests/test-qapi-types-sub-sub-module.h:17,
> >                  from tests/test-qapi-visit-sub-sub-module.h:17,
> >                  from tests/test-qapi-commands-sub-sub-module.c:19:
> > tests/test-qapi-features.h:16:5: error: redeclaration of enumerator ‘QAPI_FEATURE_DEPRECATED’
> >    16 |     QAPI_FEATURE_DEPRECATED,
> >       |     ^~~~~~~~~~~~~~~~~~~~~~~
> > In file included from ./qapi/qapi-types-error.h:17,
> >                  from /var/home/berrange/src/virt/qemu/include/qapi/error.h:275,
> >                  from /var/home/berrange/src/virt/qemu/include/qapi/compat-policy.h:16,
> >                  from tests/test-qapi-commands-sub-sub-module.c:14:
> > ./qapi/qapi-features.h:16:5: note: previous definition of ‘QAPI_FEATURE_DEPRECATED’ with type ‘enum <anonymous>’
> >    16 |     QAPI_FEATURE_DEPRECATED,
> >       |     ^~~~~~~~~~~~~~~~~~~~~~~
> > ninja: build stopped: subcommand failed.
> > make[1]: *** [Makefile:167: run-ninja] Error 1
> > make[1]: Leaving directory '/var/home/berrange/src/virt/qemu/build'
> > make: *** [GNUmakefile:6: build] Error 2
> 
> Compiles for me with PATCH 5/6 taken out.  What am I doing wrong?

The bit in patch 6 which generates the enum still has the prefix:

+        self._genh.add("typedef enum {\n")
+        for name in features:
+            self._genh.add(f"    {c_enum_const(self._prefix + 'QAPI_FEATURE', name)},\n")
+
+        self._genh.add("} " + c_name(self._prefix + 'QapiFeature') + ";\n")


> 
> > I would be nice to eliminate that, but some parts of the test appear
> > to pull in more of the system emulator code which forces in the main
> > qapi schema
> >
> > n file included from ./qapi/qapi-types-block-core.h:17,
> >                  from /var/home/berrange/src/virt/qemu/include/block/block-common.h:27,
> >                  from /var/home/berrange/src/virt/qemu/include/block/block-global-state.h:27,
> >                  from /var/home/berrange/src/virt/qemu/include/block/block.h:27,
> >                  from /var/home/berrange/src/virt/qemu/include/monitor/monitor.h:4,
> >                  from /var/home/berrange/src/virt/qemu/include/qapi/qmp/dispatch.h:17,
> >                  from tests/test-qapi-init-commands.h:16,
> >                  from tests/test-qapi-init-commands.c:15:
> > ./qapi/qapi-features.h:16:5: error: redeclaration of enumerator ‘QAPI_FEATURE_DEPRECATED’
> >    16 |     QAPI_FEATURE_DEPRECATED,
> >       |     ^~~~~~~~~~~~~~~~~~~~~~~
> > In file included from tests/include/../test-qapi-types-sub-sub-module.h:17,
> >                  from tests/include/../test-qapi-commands-sub-sub-module.h:16,
> >                  from tests/include/test-qapi-commands-sub-module.h:16,
> >                  from tests/test-qapi-commands.h:16,
> >                  from tests/test-qapi-init-commands.c:14:
> > tests/include/../test-qapi-features.h:16:5: note: previous definition of ‘QAPI_FEATURE_DEPRECATED’ with type ‘enum <anonymous>’
> >    16 |     QAPI_FEATURE_DEPRECATED,
> >       |     ^~~~~~~~~~~~~~~~~~~~~~~
> > tests/test-qapi-init-commands.c: In function ‘test_qmp_init_marshal’:
> > tests/test-qapi-init-commands.c:64:71: error: ‘TEST_QAPI_FEATURE_DEPRECATED’ undeclared (first use in this function); did you mean ‘QAPI_FEATURE_DEPRECATED’?
> >    64 |                          qmp_marshal_test_command_features1, 0, 1u << TEST_QAPI_FEATURE_DEPRECATED);
> >       |                                                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >       |                                                                       QAPI_FEATURE_DEPRECATED
> > tests/test-qapi-init-commands.c:64:71: note: each undeclared identifier is reported only once for each function it appears in
> >
> >
> > Maybe the test needs to be split into two ?  One test exclusively testing
> > with the tests/qapi-schema/qapi-schema-test.json, and one test exclusively
> > with the main QMP QAPI schema ?
> 
> Which test is it?

I'm unclear which is using this - its just failing with plain 'make' for
me.

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



  reply	other threads:[~2024-08-05 13:25 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-01 17:59 [PATCH 0/6] qapi: generalize special features Daniel P. Berrangé
2024-08-01 17:59 ` [PATCH 1/6] qapi: change 'unsigned special_features' to 'uint64_t features' Daniel P. Berrangé
2024-08-05 11:53   ` Markus Armbruster
2024-08-01 17:59 ` [PATCH 2/6] scripts/qapi: rename 'special_features' to 'features' Daniel P. Berrangé
2024-08-05 11:59   ` Markus Armbruster
2024-08-01 17:59 ` [PATCH 3/6] qapi: use "QAPI_FEATURE" as namespace for features Daniel P. Berrangé
2024-08-05 12:01   ` Markus Armbruster
2024-08-01 17:59 ` [PATCH 4/6] qapi: cope with feature names containing a '-' Daniel P. Berrangé
2024-08-05 12:10   ` Markus Armbruster
2024-08-01 17:59 ` [PATCH 5/6] qapi: apply schema prefix to QAPI feature enum constants Daniel P. Berrangé
2024-08-05 12:22   ` Markus Armbruster
2024-08-05 12:33     ` Daniel P. Berrangé
2024-08-05 13:11       ` Markus Armbruster
2024-08-05 13:24         ` Daniel P. Berrangé [this message]
2024-08-05 13:54           ` Markus Armbruster
2024-08-05 14:59             ` Markus Armbruster
2024-08-06 17:49               ` Complications due to having multiple QAPI schemas (was: [PATCH 5/6] qapi: apply schema prefix to QAPI feature enum constants) Markus Armbruster
2024-08-08 11:48   ` [PATCH 5/6] qapi: apply schema prefix to QAPI feature enum constants Markus Armbruster
2024-08-01 17:59 ` [PATCH 6/6] qapi: expose all schema features to code Daniel P. Berrangé
2024-08-02 13:50   ` Markus Armbruster
2024-08-02 15:43     ` Daniel P. Berrangé
2024-08-08 12:11   ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZrDSqXtiT2U7zS-p@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=michael.roth@amd.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).