Re: [PATCH 0/1] Enable plugin support for windows

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

(Add Paolo to cc for meson questions)

Greg Manning <gmanning@rapitasystems.com> writes:

> This patch enables plugin support on windows.
>
> qemu plugins on windows now correctly load/link the qemu_plugin_*
> symbols when they are loaded[1]. This works by creating a delay-loaded
> .lib file with dlltool[2], then linking the plugins against that to get
> them compiled. No messing around with function pointers required.
>
> The .lib file is created with "qemu.exe" as the name of the DLL.
> The only input to the creation of the .lib file is a list of
> qemu_plugin_* api symbol names.
>
> When the DLL is being loaded, it will fail to find qemu.exe[3], but
> the windows linker has a 'dll loading failed' hook you can put in the
> plugin.
>
> I've implemented this hook to check if the file that's failed to load is
> 'qemu.exe', and if so, substitute in a handle to the top-level
> executable. Symbol linking then continues in the normal way.
>
> As such, windows plugins work for me, and with minimal change to actual
> qemu code.

Excellent work - I'm glad you can make it work without having to hack
the rest of the plugins too much. I'm afraid I can't do much to test
this but overall I'm much happier to merge this.

However can we split the patches into slightly smaller chunks. Maybe
something like:

  - add the QEMU_PLUGIN_API annotations
  - introduce the windows linker bits
  - filter the plugins that can't be built for windows
  - flip the switch in configure to allow plugins to be built on Windows

Also what are we going to do for testing? Most qemu developers are using
POSIX systems so I'm worried about support bit rotting. We do have some
windows coverage in CI but currently only running qtest. There are some
"make check-tcg" tests which are softmmu but I suspect getting the
cross-compilers on windows would be a blocker. Do we know the state of
the avocado tests? We have some minimal testing there which could be
extended as all you need is avocado working and some binary images to
run. I've just posted a patch to update them:

  Message-Id: <20231101135004.1572916-1-alex.bennee@linaro.org>
  Date: Wed,  1 Nov 2023 13:50:03 +0000
  Subject: [RFC PATCH] tests/avocado: update the tcg_plugins test
  From: =?UTF-8?q?Alex=20Benn=C3=A9e?= <alex.bennee@linaro.org>

> [1]: Except lockstep, which uses unix sockets. It could be changed to
>      use another communication mechanism, but that felt outside what I'm
>      trying to achieve here.
>
> [2]: This would be the first use of dlltool in qemu. Is that OK? do we
>      need anything in meson to check it exists?

Paolo any comments on the meson bits?

>
> [3]: If qemu moved to a single executable model, and we got the name
>      right when creating the .lib, then the dll hook would not be
>      needed at all.
>
> Limitations/things I'm worried about
>
> * There is some amount of meson/Makefile tweaking going on here, and
>   while what I've got works for me, I'm not very familiar with meson,
>   and might not be doing things The Right Way.
>
> * plugin/FFI bindings authors would have to copy this hook themselves,
>   or implement something equivalent. Also they need the
>   qemu_plugin_api.lib file, which I think I've added to the set of
>   installed files.
>
> * License for win32_linker.c - I'm happy to put this under whatever
>   license seems most convenient. Given qemu-plugin.h is already GPL,
>   all plugins will be GPL anyway.

While there may be private downstream plugins being used behind closed
doors we very much don't want to encourage binary only plugins so GPL is
fine.

> Greg Manning (1):
>   plugins: enable plugins for windows
>
>  configure                      |  9 ++----
>  contrib/plugins/Makefile       | 27 ++++++++++++++++--
>  contrib/plugins/win32_linker.c | 34 +++++++++++++++++++++++
>  include/qemu/qemu-plugin.h     | 50 ++++++++++++++++++++++++++++++++--
>  meson.build                    |  5 ++++
>  plugins/meson.build            | 17 ++++++++++++
>  tests/plugin/meson.build       | 14 ++++++++--
>  7 files changed, 141 insertions(+), 15 deletions(-)
>  create mode 100644 contrib/plugins/win32_linker.c
>
> --
> 2.42.0

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro