[CRIU] [PATCH 2/3] plugin: Rework plugins API

[Andrew Vagin]

On Thu, Mar 06, 2014 at 08:53:45PM +0400, Cyrill Gorcunov wrote:
> 
> Here we define new api to be used in plugins.
> 
>  - Plugin should provide a descriptor with help of
>    CR_PLUGIN_REGISTER macro, or in case if plugin require
>    no init/exit functions -- with CR_PLUGIN_REGISTER_DUMMY.
> 
>  - Plugin should define a plugin hook with help of
>    CR_PLUGIN_REGISTER_HOOK macro.
> 
> The idea behind is to not limit plugins authors with names
> of functions they might need to use for particular hook.
> Also plugin descriptor allows us to not call for additional
> malloc()'s during plugin handling -- all information needed
> will be stored in plugin descriptor statically allocated
> in plugin library itself.
> 
> Such new API deprecates olds plugins structure but to keep
> backward compatibility we will provide a tiny layer of
> additional code to support old plugins for at least a couple
> of release cycles.
> 
> For example a trivial plugin might look like
> 
>  | #include <sys/types.h>
>  | #include <sys/stat.h>
>  | #include <fcntl.h>
>  | #include <libgen.h>
>  | #include <errno.h>
>  |
>  | #include <sys/socket.h>
>  | #include <linux/un.h>
>  |
>  | #include <stdio.h>
>  | #include <stdlib.h>
>  | #include <string.h>
>  | #include <unistd.h>
>  |
>  | #include "criu-plugin.h"
>  | #include "criu-log.h"
>  |
>  | static int dump_ext_file(int fd, int id)
>  | {
>  |	pr_info("dump_ext_file: fd %d id %d\n", fd, id);
>  |	return 0;
>  | }
>  |
>  | CR_PLUGIN_REGISTER_DUMMY("trivial")
>  | CR_PLUGIN_REGISTER_HOOK(CR_PLUGIN_HOOK__DUMP_EXT_FILE, dump_ext_file)
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  include/criu-plugin.h | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/plugin.h      |  35 +++++++++++++++++
>  2 files changed, 138 insertions(+)
> 

> diff --git a/include/criu-plugin.h b/include/criu-plugin.h
> index cda8c8eb98e6..fb2558d50deb 100644
> --- a/include/criu-plugin.h
> +++ b/include/criu-plugin.h
> @@ -23,6 +23,109 @@
>  #include <limits.h>
>  #include <stdbool.h>
>  
> +#define CRIU_PLUGIN_GEN_VERSION(a,b,c)	(((a) << 16) + ((b) << 8) + (c))
> +#define CRIU_PLUGIN_VERSION_MAJOR	0
> +#define CRIU_PLUGIN_VERSION_MINOR	2
> +#define CRIU_PLUGIN_VERSION_SUBLEVEL	0
> +
> +#define CRIU_PLUGIN_VERSION_OLD		CRIU_PLUGIN_GEN_VERSION(0,1,0)
> +
> +#define CRIU_PLUGIN_VERSION					\
> +	CRIU_PLUGIN_GEN_VERSION(CRIU_PLUGIN_VERSION_MAJOR,	\
> +				CRIU_PLUGIN_VERSION_MINOR,	\
> +				CRIU_PLUGIN_VERSION_SUBLEVEL)
> +
> +/*
> + * Plugin hook points and their arguments in hooks.
> + */
> +
> +#define DECLARE_PLUGIN_HOOK_ARGS(__hook, ...)	\
> +	typedef int (__hook ##_t)(__VA_ARGS__)
> +
> +enum {
> +	CR_PLUGIN_HOOK__DUMP_UNIX_SK,
> +	CR_PLUGIN_HOOK__RESTORE_UNIX_SK,
> +
> +	CR_PLUGIN_HOOK__DUMP_EXT_FILE,
> +	CR_PLUGIN_HOOK__RESTORE_EXT_FILE,
> +
> +	CR_PLUGIN_HOOK__DUMP_EXT_MOUNT,
> +	CR_PLUGIN_HOOK__RESTORE_EXT_MOUNT,
> +
> +	CR_PLUGIN_HOOK__DUMP_EXT_LINK,
> +
> +	CR_PLUGIN_HOOK__MAX
> +};
> +
> +DECLARE_PLUGIN_HOOK_ARGS(CR_PLUGIN_HOOK__DUMP_UNIX_SK, int fd, int id);
> +DECLARE_PLUGIN_HOOK_ARGS(CR_PLUGIN_HOOK__RESTORE_UNIX_SK, int id);
> +DECLARE_PLUGIN_HOOK_ARGS(CR_PLUGIN_HOOK__DUMP_EXT_FILE, int fd, int id);
> +DECLARE_PLUGIN_HOOK_ARGS(CR_PLUGIN_HOOK__RESTORE_EXT_FILE, int id);
> +DECLARE_PLUGIN_HOOK_ARGS(CR_PLUGIN_HOOK__DUMP_EXT_MOUNT, char *mountpoint, int id);
> +DECLARE_PLUGIN_HOOK_ARGS(CR_PLUGIN_HOOK__RESTORE_EXT_MOUNT, int id, char *mountpoint, char *old_root, int *is_file);
> +DECLARE_PLUGIN_HOOK_ARGS(CR_PLUGIN_HOOK__DUMP_EXT_LINK, int index, int type, char *kind);
> +
> +/*
> + * Strictly speaking this CR_PRIV_CHAIN_ENTRY_SIZE stands for
> + * double linked list entry size.
> + */
> +#define CR_PRIV_CHAIN_ENTRY_SIZE	(2 * sizeof(void *))
> +#define CR_PRIV_SIZE			((CR_PLUGIN_HOOK__MAX) * CR_PRIV_CHAIN_ENTRY_SIZE + \
> +					 CR_PRIV_CHAIN_ENTRY_SIZE + 16 * sizeof(void *))

What is a magic 16 here?

> +
> +typedef struct {
> +	const char		*name;
> +	int			(*init)(void);
> +	void			(*exit)(void);
> +	unsigned int		version;
> +	unsigned int		max_hooks;
> +	void			*hooks[CR_PLUGIN_HOOK__MAX];
> +	size_t			priv_size;
> +
> +	/*
> +	 * @private is CRIU plugin engine private field,
> +	 * don't ever modify inside plugin code!
> +	 *
> +	 * Also if this structure need to be extended
> +	 * put new members _AFTER_ the @private field.
> +	 */
> +	char			private[CR_PRIV_SIZE];

How we are going to support its backward compatibility in a future?

> +} __attribute__((__packed__)) cr_plugin_desc_t;

Why do we need __packed__ here?

> +
> +extern cr_plugin_desc_t CR_PLUGIN_DESC;
> +
> +#define CR_PLUGIN_REGISTER(___name, ___init, ___exit)					\
> +	cr_plugin_desc_t CR_PLUGIN_DESC = {						\
> +		.name		= ___name,						\
> +		.init		= ___init,						\
> +		.exit		= ___exit,						\
> +		.version	= CRIU_PLUGIN_VERSION,					\
> +		.max_hooks	= CR_PLUGIN_HOOK__MAX,					\
> +		.priv_size	= CR_PRIV_SIZE,						\
> +	}
> +
> +static inline int cr_plugin_dummy_init(void) { return 0; }
> +static inline void cr_plugin_dummy_exit(void) { }

I think inline is useless here, because you are going to access these
function by pointers.

> +
> +#define CR_PLUGIN_REGISTER_DUMMY(___name)						\
> +	cr_plugin_desc_t CR_PLUGIN_DESC = {						\
> +		.name		= ___name,						\
> +		.init		= cr_plugin_dummy_init,					\
> +		.exit		= cr_plugin_dummy_exit,					\
> +		.version	= CRIU_PLUGIN_VERSION,					\
> +		.max_hooks	= CR_PLUGIN_HOOK__MAX,					\
> +		.priv_size	= CR_PRIV_SIZE,						\
> +	};
> +
> +#define CR_PLUGIN_REGISTER_HOOK(__hook, __func)						\
> +static void __attribute__((constructor)) cr_plugin_register_hook_##__func (void)	\
> +{											\
> +	CR_PLUGIN_DESC.hooks[__hook] = (void *)__func;					\
> +}
> +
> +/*
> + * Obsolete, will be dropped off after a couple of releases.
> + */
>  typedef int (cr_plugin_init_t)(void);
>  typedef void (cr_plugin_fini_t)(void);
>  
> diff --git a/include/plugin.h b/include/plugin.h
> index 3c53e3003de3..ddcfcc99ca0d 100644
> --- a/include/plugin.h
> +++ b/include/plugin.h
> @@ -2,6 +2,8 @@
>  #define __CR_PLUGIN_H__
>  
>  #include "criu-plugin.h"
> +#include "compiler.h"
> +#include "list.h"
>  
>  #define CR_PLUGIN_DEFAULT "/var/lib/criu/"
>  
> @@ -19,4 +21,37 @@ int cr_plugin_restore_ext_mount(int id, char *mountpoint, char *old_root, int *i
>  
>  int cr_plugin_dump_ext_link(int index, int type, char *kind);
>  
> +typedef struct {
> +	struct list_head	head;
> +	struct list_head	hook_chain[CR_PLUGIN_HOOK__MAX];
> +} cr_plugin_ctl_t;
> +
> +extern cr_plugin_ctl_t cr_plugin_ctl;
> +
> +typedef struct {
> +	struct list_head	list;
> +	void			*dlhandle;
> +	struct list_head	link[0];
> +} __packed plugin_desc_t;
> +
> +#define cr_plugin_desc_from(this)								\
> +	((void *)this - (size_t)offsetof(cr_plugin_desc_t, private))
> +
> +#define run_plugins(__hook, ...)								\
> +({												\
> +	cr_plugin_desc_t *d;									\
> +	plugin_desc_t *this;									\
> +	int __ret = -ENOTSUP;									\
> +												\
> +	list_for_each_entry(this, &cr_plugin_ctl.hook_chain[__hook], link[__hook]) {		\
> +		d = cr_plugin_desc_from(this);							\
> +		pr_debug("plugin: `%s' hook %u -> %p\n", d->name, __hook, d->hooks[__hook]);	\
> +		__ret = ((__hook ##_t *)d->hooks[__hook])(__VA_ARGS__);				\
> +		if (__ret == -ENOTSUP)								\
> +			continue;								\
> +		break;										\
> +	}											\
> +	__ret;											\
> +})
> +
>  #endif