Re: [PATCH] contrib/plugins: ensure build does not pick up a system copy of plugin header

["Alex Bennée" <alex.bennee@linaro.org>]

Brad Smith <brad@comstyle.com> writes:

> On 2024-09-21 8:55 a.m., Alex Bennée wrote:
>> Brad Smith <brad@comstyle.com> writes:
>>
>>> contrib/plugins: ensure build does not pick up a system copy of plugin
>>> header
>> I'm confused because this changes the ordering of the GLIB inclusion. We
>> shouldn't be including the whole QEMU include path.
>
> That's intentional. The GLIB header paths cannot come before the header path
> for the plugin header otherwise it pulls in the older plugin header from the
> installed copy of QEMU and breaks. The QEMU include path is necessary
> for the plugin header.
>
>
> cc  -O2 -g  -I/usr/local/include/glib-2.0
> -I/usr/local/lib/glib-2.0/include -I/usr/local/include -fPIC -Wall
> -I/home/brad/tmp/qemu/contrib/plugins/../../include/qemu -c -o
> execlog.o /home/brad/tmp/qemu/contrib/plugins/execlog.c
> /home/brad/tmp/qemu/contrib/plugins/execlog.c:262:41: error: too many
> arguments to function call, expected single argument 'insn', have 3
> arguments
>             qemu_plugin_insn_data(insn, &insn_opcode, sizeof(insn_opcode));
>             ~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /usr/local/include/qemu-plugin.h:407:13: note: 'qemu_plugin_insn_data'
> declared here
> const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn);
>             ^
> 1 error generated.

Ahh gotcha, I confused myself thinking this was QEMU's internal API header.

Queued to plugins/next, thanks.

>
>> How does this fail?
>>
>>> With the ordering of the header path if a copy of QEMU is installed it
>>> will pickup the system copy of the header before the build paths copy
>>> and the build will fail.
>>>
>>> Signed-off-by: Brad Smith <brad@comstyle.com>
>>> ---
>>>   contrib/plugins/Makefile | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
>>> index 05a2a45c5c..52fc390376 100644
>>> --- a/contrib/plugins/Makefile
>>> +++ b/contrib/plugins/Makefile
>>> @@ -41,9 +41,10 @@ SONAMES := $(addsuffix $(SO_SUFFIX),$(addprefix lib,$(NAMES)))
>>>     # The main QEMU uses Glib extensively so it is perfectly fine
>>> to use it
>>>   # in plugins (which many example do).
>>> -PLUGIN_CFLAGS := $(shell $(PKG_CONFIG) --cflags glib-2.0)
>>> -PLUGIN_CFLAGS += -fPIC -Wall
>>> +GLIB_CFLAGS := $(shell $(PKG_CONFIG) --cflags glib-2.0)
>>>   PLUGIN_CFLAGS += -I$(TOP_SRC_PATH)/include/qemu
>> Not withstanding the fact I've just borrowed bswap.h for a test plugin
>> maybe we should actually copy qemu-plugin.h to an entirely new location
>> during the build and then include from there to avoid any other
>> potential pollutions?
>
> I don't see how that would make any difference, but either way as long
> as the header
> path ordering is corrected so this new path is not passed last on the
> command line
> getting the ordering wrong.
>
>>
>>> +PLUGIN_CFLAGS += $(GLIB_CFLAGS)
>>> +PLUGIN_CFLAGS += -fPIC -Wall
>>>     # Helper that honours V=1 so we get some output when compiling
>>>   quiet-@ = $(if $(V),,@$(if $1,printf "  %-7s %s\n" "$(strip $1)" "$(strip $2)" && ))

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro