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

[Cyrill Gorcunov]

On Wed, Mar 12, 2014 at 02:57:27PM +0400, Andrew Vagin wrote:
> > +
> > +/*
> > + * 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?

16 cells for further use if ever needed.

> > +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?

By version + max_hooks tuple. If we ever change API so that
new format won;t be compatible with new one -- we always
can detect it by comparing @version and wrap data with
some new descriptor generated in memory.

> 
> > +} __attribute__((__packed__)) cr_plugin_desc_t;
> 
> Why do we need __packed__ here?

To make sure that there is no holes or anything in the
structure. I must admit it's not that required but
decided to put it here just to be safe.

> > +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.

Without inline I've got compile warnings in earlier development stages
of this code. Not a big deal can drop it out.