include/qemu/qemu-plugin.h | 19 +++++++++++++++++++ plugins/loader.c | 15 +++++++++++++++ plugins/plugin.h | 2 ++ tests/plugin/bb.c | 2 ++ tests/plugin/empty.c | 2 ++ tests/plugin/hotpages.c | 2 ++ tests/plugin/howvec.c | 2 ++ tests/plugin/insn.c | 2 ++ tests/plugin/mem.c | 2 ++ 9 files changed, 48 insertions(+)
This is a very simple versioning API which allows the plugin
infrastructure to check the API a plugin was built against. We also
expose a min/cur API version to the plugin via the info block in case
it wants to avoid using old deprecated APIs in the future.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/qemu/qemu-plugin.h | 19 +++++++++++++++++++
plugins/loader.c | 15 +++++++++++++++
plugins/plugin.h | 2 ++
tests/plugin/bb.c | 2 ++
tests/plugin/empty.c | 2 ++
tests/plugin/hotpages.c | 2 ++
tests/plugin/howvec.c | 2 ++
tests/plugin/insn.c | 2 ++
tests/plugin/mem.c | 2 ++
9 files changed, 48 insertions(+)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index a00a7deb461..5502e112c81 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -38,9 +38,28 @@
typedef uint64_t qemu_plugin_id_t;
+/*
+ * Versioning plugins:
+ *
+ * The plugin API will pass a minimum and current API version that
+ * QEMU currently supports. The minimum API will be incremented if an
+ * API needs to be deprecated.
+ *
+ * The plugins export the API they were built against by exposing the
+ * symbol qemu_plugin_version which can be checked.
+ */
+
+extern QEMU_PLUGIN_EXPORT int qemu_plugin_version;
+
+#define QEMU_PLUGIN_VERSION 0
+
typedef struct {
/* string describing architecture */
const char *target_name;
+ struct {
+ int min;
+ int cur;
+ } version;
/* is this a full system emulation? */
bool system_emulation;
union {
diff --git a/plugins/loader.c b/plugins/loader.c
index ce724ed5839..1bcca909691 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -178,6 +178,19 @@ static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info)
goto err_symbol;
}
+ if (!g_module_symbol(ctx->handle, "qemu_plugin_version", &sym)) {
+ warn_report("%s: missing version %s", __func__, g_module_error());
+ } else {
+ int version = *(int *)sym;
+ if (version < QEMU_PLUGIN_MIN_VERSION ||
+ version > QEMU_PLUGIN_VERSION) {
+ error_report("%s: bad plugin version %d vs %d/%d",
+ __func__, version, QEMU_PLUGIN_MIN_VERSION,
+ QEMU_PLUGIN_VERSION);
+ goto err_symbol;
+ }
+ }
+
qemu_rec_mutex_lock(&plugin.lock);
/* find an unused random id with &ctx as the seed */
@@ -248,6 +261,8 @@ int qemu_plugin_load_list(QemuPluginList *head)
g_autofree qemu_info_t *info = g_new0(qemu_info_t, 1);
info->target_name = TARGET_NAME;
+ info->version.min = QEMU_PLUGIN_MIN_VERSION;
+ info->version.cur = QEMU_PLUGIN_VERSION;
#ifndef CONFIG_USER_ONLY
MachineState *ms = MACHINE(qdev_get_machine());
info->system_emulation = true;
diff --git a/plugins/plugin.h b/plugins/plugin.h
index 5482168d797..1aa29dcaddf 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -14,6 +14,8 @@
#include <gmodule.h>
+#define QEMU_PLUGIN_MIN_VERSION 0
+
/* global state */
struct qemu_plugin_state {
QTAILQ_HEAD(, qemu_plugin_ctx) ctxs;
diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c
index 45e1de5bd68..f30bea08dcc 100644
--- a/tests/plugin/bb.c
+++ b/tests/plugin/bb.c
@@ -14,6 +14,8 @@
#include <qemu-plugin.h>
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
static uint64_t bb_count;
static uint64_t insn_count;
static bool do_inline;
diff --git a/tests/plugin/empty.c b/tests/plugin/empty.c
index 3f60f690278..8fa6bacd93d 100644
--- a/tests/plugin/empty.c
+++ b/tests/plugin/empty.c
@@ -13,6 +13,8 @@
#include <qemu-plugin.h>
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
/*
* Empty TB translation callback.
* This allows us to measure the overhead of injecting and then
diff --git a/tests/plugin/hotpages.c b/tests/plugin/hotpages.c
index 77df07a3ccf..ecd6c187327 100644
--- a/tests/plugin/hotpages.c
+++ b/tests/plugin/hotpages.c
@@ -18,6 +18,8 @@
#include <qemu-plugin.h>
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
static uint64_t page_size = 4096;
diff --git a/tests/plugin/howvec.c b/tests/plugin/howvec.c
index 58fa675e348..4ca555e1239 100644
--- a/tests/plugin/howvec.c
+++ b/tests/plugin/howvec.c
@@ -20,6 +20,8 @@
#include <qemu-plugin.h>
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
typedef enum {
diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c
index e5fd07fb64b..0a8f5a0000e 100644
--- a/tests/plugin/insn.c
+++ b/tests/plugin/insn.c
@@ -14,6 +14,8 @@
#include <qemu-plugin.h>
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
static uint64_t insn_count;
static bool do_inline;
diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c
index d9673889896..878abf09d19 100644
--- a/tests/plugin/mem.c
+++ b/tests/plugin/mem.c
@@ -14,6 +14,8 @@
#include <qemu-plugin.h>
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
static uint64_t mem_count;
static uint64_t io_count;
static bool do_inline;
--
2.20.1
On 11/4/19 2:18 PM, Alex Bennée wrote: > This is a very simple versioning API which allows the plugin > infrastructure to check the API a plugin was built against. We also > expose a min/cur API version to the plugin via the info block in case > it wants to avoid using old deprecated APIs in the future. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > include/qemu/qemu-plugin.h | 19 +++++++++++++++++++ > plugins/loader.c | 15 +++++++++++++++ > plugins/plugin.h | 2 ++ > tests/plugin/bb.c | 2 ++ > tests/plugin/empty.c | 2 ++ > tests/plugin/hotpages.c | 2 ++ > tests/plugin/howvec.c | 2 ++ > tests/plugin/insn.c | 2 ++ > tests/plugin/mem.c | 2 ++ > 9 files changed, 48 insertions(+) > > diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h > index a00a7deb461..5502e112c81 100644 > --- a/include/qemu/qemu-plugin.h > +++ b/include/qemu/qemu-plugin.h > @@ -38,9 +38,28 @@ > > typedef uint64_t qemu_plugin_id_t; > > +/* > + * Versioning plugins: > + * > + * The plugin API will pass a minimum and current API version that > + * QEMU currently supports. The minimum API will be incremented if an > + * API needs to be deprecated. > + * > + * The plugins export the API they were built against by exposing the > + * symbol qemu_plugin_version which can be checked. > + */ > + > +extern QEMU_PLUGIN_EXPORT int qemu_plugin_version; > + > +#define QEMU_PLUGIN_VERSION 0 > + > typedef struct { > /* string describing architecture */ > const char *target_name; > + struct { > + int min; > + int cur; > + } version; > /* is this a full system emulation? */ > bool system_emulation; > union { > diff --git a/plugins/loader.c b/plugins/loader.c > index ce724ed5839..1bcca909691 100644 > --- a/plugins/loader.c > +++ b/plugins/loader.c > @@ -178,6 +178,19 @@ static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info) > goto err_symbol; > } > > + if (!g_module_symbol(ctx->handle, "qemu_plugin_version", &sym)) { > + warn_report("%s: missing version %s", __func__, g_module_error()); > + } else { > + int version = *(int *)sym; > + if (version < QEMU_PLUGIN_MIN_VERSION || > + version > QEMU_PLUGIN_VERSION) { > + error_report("%s: bad plugin version %d vs %d/%d", > + __func__, version, QEMU_PLUGIN_MIN_VERSION, > + QEMU_PLUGIN_VERSION); > + goto err_symbol; > + } > + } > + > qemu_rec_mutex_lock(&plugin.lock); > > /* find an unused random id with &ctx as the seed */ > @@ -248,6 +261,8 @@ int qemu_plugin_load_list(QemuPluginList *head) > g_autofree qemu_info_t *info = g_new0(qemu_info_t, 1); > > info->target_name = TARGET_NAME; > + info->version.min = QEMU_PLUGIN_MIN_VERSION; > + info->version.cur = QEMU_PLUGIN_VERSION; > #ifndef CONFIG_USER_ONLY > MachineState *ms = MACHINE(qdev_get_machine()); > info->system_emulation = true; > diff --git a/plugins/plugin.h b/plugins/plugin.h > index 5482168d797..1aa29dcaddf 100644 > --- a/plugins/plugin.h > +++ b/plugins/plugin.h > @@ -14,6 +14,8 @@ > > #include <gmodule.h> > > +#define QEMU_PLUGIN_MIN_VERSION 0 > + > /* global state */ > struct qemu_plugin_state { > QTAILQ_HEAD(, qemu_plugin_ctx) ctxs; > diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c > index 45e1de5bd68..f30bea08dcc 100644 > --- a/tests/plugin/bb.c > +++ b/tests/plugin/bb.c > @@ -14,6 +14,8 @@ > > #include <qemu-plugin.h> > > +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION; > + > static uint64_t bb_count; > static uint64_t insn_count; > static bool do_inline; > diff --git a/tests/plugin/empty.c b/tests/plugin/empty.c > index 3f60f690278..8fa6bacd93d 100644 > --- a/tests/plugin/empty.c > +++ b/tests/plugin/empty.c > @@ -13,6 +13,8 @@ > > #include <qemu-plugin.h> > > +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION; > + > /* > * Empty TB translation callback. > * This allows us to measure the overhead of injecting and then > diff --git a/tests/plugin/hotpages.c b/tests/plugin/hotpages.c > index 77df07a3ccf..ecd6c187327 100644 > --- a/tests/plugin/hotpages.c > +++ b/tests/plugin/hotpages.c > @@ -18,6 +18,8 @@ > > #include <qemu-plugin.h> > > +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION; > + > #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) > > static uint64_t page_size = 4096; > diff --git a/tests/plugin/howvec.c b/tests/plugin/howvec.c > index 58fa675e348..4ca555e1239 100644 > --- a/tests/plugin/howvec.c > +++ b/tests/plugin/howvec.c > @@ -20,6 +20,8 @@ > > #include <qemu-plugin.h> > > +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION; > + > #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) > > typedef enum { > diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c > index e5fd07fb64b..0a8f5a0000e 100644 > --- a/tests/plugin/insn.c > +++ b/tests/plugin/insn.c > @@ -14,6 +14,8 @@ > > #include <qemu-plugin.h> > > +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION; > + > static uint64_t insn_count; > static bool do_inline; > > diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c > index d9673889896..878abf09d19 100644 > --- a/tests/plugin/mem.c > +++ b/tests/plugin/mem.c > @@ -14,6 +14,8 @@ > > #include <qemu-plugin.h> > > +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION; > + > static uint64_t mem_count; > static uint64_t io_count; > static bool do_inline; > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On Mon, 4 Nov 2019 at 13:18, Alex Bennée <alex.bennee@linaro.org> wrote: > > This is a very simple versioning API which allows the plugin > infrastructure to check the API a plugin was built against. We also > expose a min/cur API version to the plugin via the info block in case > it wants to avoid using old deprecated APIs in the future. I think the general idea here is fine, and I want to see us get the version-check into 4.2, because I don't think we should release a QEMU which doesn't do a basic version sanity check. I have some minor comments below. > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > include/qemu/qemu-plugin.h | 19 +++++++++++++++++++ > plugins/loader.c | 15 +++++++++++++++ > plugins/plugin.h | 2 ++ > tests/plugin/bb.c | 2 ++ > tests/plugin/empty.c | 2 ++ > tests/plugin/hotpages.c | 2 ++ > tests/plugin/howvec.c | 2 ++ > tests/plugin/insn.c | 2 ++ > tests/plugin/mem.c | 2 ++ > 9 files changed, 48 insertions(+) > > diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h > index a00a7deb461..5502e112c81 100644 > --- a/include/qemu/qemu-plugin.h > +++ b/include/qemu/qemu-plugin.h As an aside, is this the header file used for building the plugins? It seems a bit odd to have put it in include/qemu, which is for headers used when building QEMU itself. > @@ -38,9 +38,28 @@ > > typedef uint64_t qemu_plugin_id_t; > > +/* > + * Versioning plugins: > + * > + * The plugin API will pass a minimum and current API version that > + * QEMU currently supports. The minimum API will be incremented if an > + * API needs to be deprecated. > + * > + * The plugins export the API they were built against by exposing the > + * symbol qemu_plugin_version which can be checked. > + */ > + > +extern QEMU_PLUGIN_EXPORT int qemu_plugin_version; > + > +#define QEMU_PLUGIN_VERSION 0 > + > typedef struct { > /* string describing architecture */ > const char *target_name; > + struct { > + int min; > + int cur; > + } version; > /* is this a full system emulation? */ > bool system_emulation; > union { > diff --git a/plugins/loader.c b/plugins/loader.c > index ce724ed5839..1bcca909691 100644 > --- a/plugins/loader.c > +++ b/plugins/loader.c > @@ -178,6 +178,19 @@ static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info) > goto err_symbol; > } > > + if (!g_module_symbol(ctx->handle, "qemu_plugin_version", &sym)) { > + warn_report("%s: missing version %s", __func__, g_module_error()); Failure to declare the required version should be an error, not just a warning. > + } else { > + int version = *(int *)sym; > + if (version < QEMU_PLUGIN_MIN_VERSION || > + version > QEMU_PLUGIN_VERSION) { > + error_report("%s: bad plugin version %d vs %d/%d", > + __func__, version, QEMU_PLUGIN_MIN_VERSION, > + QEMU_PLUGIN_VERSION); I think this message is too cryptic, and would prefer something more like: "TCG plugin %s requires API version %d, but this QEMU supports only versions %d to %d" where the first %s is the name of the plugin binary, not the __func__ of this function (which is not useful to the end user). Better still, special case the MIN_VERSION == VERSION case to avoid saying "versions 1 to 1". > + goto err_symbol; > + } > + } > + > qemu_rec_mutex_lock(&plugin.lock); > > /* find an unused random id with &ctx as the seed */ > @@ -248,6 +261,8 @@ int qemu_plugin_load_list(QemuPluginList *head) > g_autofree qemu_info_t *info = g_new0(qemu_info_t, 1); > > info->target_name = TARGET_NAME; > + info->version.min = QEMU_PLUGIN_MIN_VERSION; > + info->version.cur = QEMU_PLUGIN_VERSION; > #ifndef CONFIG_USER_ONLY > MachineState *ms = MACHINE(qdev_get_machine()); > info->system_emulation = true; thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Mon, 4 Nov 2019 at 13:18, Alex Bennée <alex.bennee@linaro.org> wrote: >> >> This is a very simple versioning API which allows the plugin >> infrastructure to check the API a plugin was built against. We also >> expose a min/cur API version to the plugin via the info block in case >> it wants to avoid using old deprecated APIs in the future. > > I think the general idea here is fine, and I want to see > us get the version-check into 4.2, because I don't think > we should release a QEMU which doesn't do a basic version > sanity check. I have some minor comments below. > > >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> include/qemu/qemu-plugin.h | 19 +++++++++++++++++++ >> plugins/loader.c | 15 +++++++++++++++ >> plugins/plugin.h | 2 ++ >> tests/plugin/bb.c | 2 ++ >> tests/plugin/empty.c | 2 ++ >> tests/plugin/hotpages.c | 2 ++ >> tests/plugin/howvec.c | 2 ++ >> tests/plugin/insn.c | 2 ++ >> tests/plugin/mem.c | 2 ++ >> 9 files changed, 48 insertions(+) >> >> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h >> index a00a7deb461..5502e112c81 100644 >> --- a/include/qemu/qemu-plugin.h >> +++ b/include/qemu/qemu-plugin.h > > As an aside, is this the header file used for building > the plugins? It seems a bit odd to have put it > in include/qemu, which is for headers used when building > QEMU itself. It's also included by the internal plugin header. We could put it somewhere else. include/api/? Where do the QMP specs end up? I'll make the other changes in v2. -- Alex Bennée
© 2016 - 2024 Red Hat, Inc.