[RFC,4/4] tcg/plugins: Add example pair of QPP plugins

The first plugin, qpp_srv exposes two functions and one callback that other
plugins can leverage. These functions are described in the corresponding
header file.

The second plugin, qpp_client, imports this header file, registers its
own function to run on a qpp_srv-provided callback, and directly calls
into the two exposed functions in qpp_srv.

Signed-off-by: Andrew Fasano <fasano@mit.edu>
---
 contrib/plugins/Makefile     |  2 ++
 contrib/plugins/qpp_client.c | 42 ++++++++++++++++++++++++++++++++++++
 contrib/plugins/qpp_client.h |  1 +
 contrib/plugins/qpp_srv.c    | 33 ++++++++++++++++++++++++++++
 contrib/plugins/qpp_srv.h    | 17 +++++++++++++++
 5 files changed, 95 insertions(+)
 create mode 100644 contrib/plugins/qpp_client.c
 create mode 100644 contrib/plugins/qpp_client.h
 create mode 100644 contrib/plugins/qpp_srv.c
 create mode 100644 contrib/plugins/qpp_srv.h

Andrew Fasano <fasano@mit.edu> writes:

> The first plugin, qpp_srv exposes two functions and one callback that other
> plugins can leverage. These functions are described in the corresponding
> header file.
>
> The second plugin, qpp_client, imports this header file, registers its
> own function to run on a qpp_srv-provided callback, and directly calls
> into the two exposed functions in qpp_srv.

I'll just sketch out how I would change the API in this example plugin:

>
> Signed-off-by: Andrew Fasano <fasano@mit.edu>
> ---
>  contrib/plugins/Makefile     |  2 ++
>  contrib/plugins/qpp_client.c | 42 ++++++++++++++++++++++++++++++++++++
>  contrib/plugins/qpp_client.h |  1 +
>  contrib/plugins/qpp_srv.c    | 33 ++++++++++++++++++++++++++++
>  contrib/plugins/qpp_srv.h    | 17 +++++++++++++++
>  5 files changed, 95 insertions(+)
>  create mode 100644 contrib/plugins/qpp_client.c
>  create mode 100644 contrib/plugins/qpp_client.h
>  create mode 100644 contrib/plugins/qpp_srv.c
>  create mode 100644 contrib/plugins/qpp_srv.h
>
> diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
> index b7720fea0f..b7510de89c 100644
> --- a/contrib/plugins/Makefile
> +++ b/contrib/plugins/Makefile
> @@ -21,6 +21,8 @@ NAMES += lockstep
>  NAMES += hwprofile
>  NAMES += cache
>  NAMES += drcov
> +NAMES += qpp_srv
> +NAMES += qpp_client
>  
>  SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
>  
> diff --git a/contrib/plugins/qpp_client.c b/contrib/plugins/qpp_client.c
> new file mode 100644
> index 0000000000..de3335e167
> --- /dev/null
> +++ b/contrib/plugins/qpp_client.c
> @@ -0,0 +1,42 @@
> +#include <stdio.h>
> +#include <qemu-plugin.h>
> +#include <plugin-qpp.h>
> +#include <glib.h>
> +#include "qpp_srv.h"
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;

QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "qpp_client";
QEMU_PLUGIN_EXPORT const char *qemu_plugin_uses = "qpp_server";

> +
> +void my_on_exit(int x, bool b)

void my_on_exit(gpointer evdata, gpointer udata)
{
  struct qpp_exit_event *info = (struct qpp_exit_event *) evdata;
  x = info->x;
  b = info->b;

> +{
> +  g_autoptr(GString) report = g_string_new("Client: on_exit runs with args: ");
> +  g_string_append_printf(report, "%d, %d\n", x, b);
> +  qemu_plugin_outs(report->str);
> +
> +  g_string_printf(report, "Client: calls qpp_srv's do_add(1): %d\n",
> +                          qpp_srv_do_add(1));
> +  qemu_plugin_outs(report->str);
> +
> +  g_string_printf(report, "Client: calls qpp_srv's do_sub(1): %d\n",
> +                           qpp_srv_do_sub(1));
> +  qemu_plugin_outs(report->str);
> +}
> +
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> +                   const qemu_info_t *info, int argc, char **argv) {
> +
> +    /*
> +     * Register our "my_on_exit" function to run on the on_exit QPP-callback
> +     * exported by qpp_srv
> +     */
> +    QPP_REG_CB("qpp_srv", on_exit, my_on_exit);

   qemu_plugin_register_event_listener("qpp_server", "exit", my_on_exit);

> +
> +    g_autoptr(GString) report = g_string_new(CURRENT_PLUGIN ": Call "
> +                                             "qpp_srv's do_add(0) => ");
> +    g_string_append_printf(report, "%d\n", qpp_srv_do_add(0));
> +    qemu_plugin_outs(report->str);
> +
> +    g_string_printf(report, "Client: registered on_exit callback\n");
> +    return 0;
> +}
> +
> diff --git a/contrib/plugins/qpp_client.h b/contrib/plugins/qpp_client.h
> new file mode 100644
> index 0000000000..573923f580
> --- /dev/null
> +++ b/contrib/plugins/qpp_client.h
> @@ -0,0 +1 @@
> +void my_on_exit(int, bool);
> diff --git a/contrib/plugins/qpp_srv.c b/contrib/plugins/qpp_srv.c
> new file mode 100644
> index 0000000000..61a6ab38ed
> --- /dev/null
> +++ b/contrib/plugins/qpp_srv.c
> @@ -0,0 +1,33 @@
> +#include <stdio.h>
> +#include <qemu-plugin.h>
> +#include <plugin-qpp.h>
> +#include <gmodule.h>
> +#include "qpp_srv.h"
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;

QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "qpp_server";

> +
> +QPP_CREATE_CB(on_exit);

void *on_exit;

> +
> +static void plugin_exit(qemu_plugin_id_t id, void *p)
> +{
> +  qemu_plugin_outs(CURRENT_PLUGIN "exit triggered, running all registered"
> +                  " QPP callbacks\n");
> +  QPP_RUN_CB(on_exit, 0, true);

     struct qpp_exit_event *info = g_new0(qpp_exit_event, 1);
     info->x = 0;
     info->b = true;
     qemu_plugin_trigger_event(on_exit, info);

> +}
> +
> +QEMU_PLUGIN_EXPORT int do_add(int x)

QEMU_PLUGIN_EXPORT int qpp_srv_do_add(int x)

> +{
> +  return x + 1;
> +}
> +
> +QEMU_PLUGIN_EXPORT int do_sub(int x)

QEMU_PLUGIN_EXPORT int qpp_srv_do_sub(int x)

> +{
> +  return x - 1;
> +}
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> +                   const qemu_info_t *info, int argc, char **argv) {
> +    qemu_plugin_outs("qpp_srv loaded\n");
> +    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
> +    return 0;
> +}
> diff --git a/contrib/plugins/qpp_srv.h b/contrib/plugins/qpp_srv.h
> new file mode 100644
> index 0000000000..ceb26e3d2c
> --- /dev/null
> +++ b/contrib/plugins/qpp_srv.h
> @@ -0,0 +1,17 @@
> +#ifndef QPP_SRV_H
> +#define QPP_SRV_H
> +
> +/*
> + * Prototype for the on_exit callback: callback functions should be
> + * of type `void f(int, bool)`
> + */
> +QPP_CB_PROTOTYPE(void, on_exit, int, bool);

can be dropped.

> +
> +/*
> + * Prototypes for the do_add and do_sub functions. Both return an int and
> + * take an int as an argument.
> + */
> +QPP_FUN_PROTOTYPE(qpp_srv, int, do_add, int);
> +QPP_FUN_PROTOTYPE(qpp_srv, int, do_sub, int);

int qpp_srv_do_add(int);
int qpp_srv_do_sub(int);

the linking is dealt with by loader.

> +
> +#endif /* QPP_SRV_H */

Andrew Fasano <fasano@mit.edu> writes:

> The first plugin, qpp_srv exposes two functions and one callback that other
> plugins can leverage. These functions are described in the corresponding
> header file.
>
> The second plugin, qpp_client, imports this header file, registers its
> own function to run on a qpp_srv-provided callback, and directly calls
> into the two exposed functions in qpp_srv.
>
> Signed-off-by: Andrew Fasano <fasano@mit.edu>
> ---
>  contrib/plugins/Makefile     |  2 ++
>  contrib/plugins/qpp_client.c | 42 ++++++++++++++++++++++++++++++++++++
>  contrib/plugins/qpp_client.h |  1 +
>  contrib/plugins/qpp_srv.c    | 33 ++++++++++++++++++++++++++++
>  contrib/plugins/qpp_srv.h    | 17 +++++++++++++++

Oh and I forgot this toy case should probably be in test/plugins/qpp with an
explicit test in tests/tcg/multiarch/Makefile to exercise it during
"make check-tcg". This should hopefully avoid having to mess with
PLUGINS in tests/tcg/Makefile.target.

<snip>

From: Alex Bennée <alex.bennee@linaro.org>
> Andrew Fasano <fasano@mit.edu> writes:
> 
> > The first plugin, qpp_srv exposes two functions and one callback that other
> > plugins can leverage. These functions are described in the corresponding
> > header file.
> >
> > The second plugin, qpp_client, imports this header file, registers its
> > own function to run on a qpp_srv-provided callback, and directly calls
> > into the two exposed functions in qpp_srv.
> 
> I'll just sketch out how I would change the API in this example plugin:
[snip]
> QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "qpp_client";

This works. Perhaps plugins could (should?) also specify a version number
that other plugins could check before interacting with:

	QEMU_PLUGIN_EXPORT const int qemu_plugin_version = 1;

> QEMU_PLUGIN_EXPORT const char *qemu_plugin_uses = "qpp_server";

I anticipate plugins could depend on multiple other plugins and they might
want to check the version of these plugins, so perhaps a function call would
be better? Something like:

	int qpp_srv_version = load_qemu_plugin("qpp_server");

Perhaps returning negative values on error, otherwise the plugin version. Or
the version compatability checks could be standardized into the plugin core,
but that seems less important to me for now.
[snip]

> QEMU_PLUGIN_EXPORT int qpp_srv_do_add(int x)

I like the change to keep these with the plugin name as a prefix everywhere.

Thanks,
Andrew

From: Alex Bennée <alex.bennee@linaro.org>
> Andrew Fasano <fasano@mit.edu> writes:
> 
> > The first plugin, qpp_srv exposes two functions and one callback that other
> > plugins can leverage. These functions are described in the corresponding
> > header file.
> >
> > The second plugin, qpp_client, imports this header file, registers its
> > own function to run on a qpp_srv-provided callback, and directly calls
> > into the two exposed functions in qpp_srv.
> >
> > Signed-off-by: Andrew Fasano <fasano@mit.edu>
> > ---
> >  contrib/plugins/Makefile     |  2 ++
> >  contrib/plugins/qpp_client.c | 42 ++++++++++++++++++++++++++++++++++++
> >  contrib/plugins/qpp_client.h |  1 +
> >  contrib/plugins/qpp_srv.c    | 33 ++++++++++++++++++++++++++++
> >  contrib/plugins/qpp_srv.h    | 17 +++++++++++++++
> 
> Oh and I forgot this toy case should probably be in test/plugins/qpp with an
> explicit test in tests/tcg/multiarch/Makefile to exercise it during
> "make check-tcg". This should hopefully avoid having to mess with
> PLUGINS in tests/tcg/Makefile.target.

Okay, that makes sense. Thanks so much for all the feedback!

> 
> <snip>
> 
> -- 
> Alex Bennée

Thanks,
Andrew