[CRIU] [PATCH 3/4] unix: add ability to set callbacks for external sockets

[Andrew Vagin]

On Mon, Dec 02, 2013 at 02:12:59AM +0400, Cyrill Gorcunov wrote:
> On Sat, Nov 30, 2013 at 09:43:55PM +0400, Andrey Vagin wrote:
> > We don't know a state behind an external socket. It depends on logic
> > of the program, which handles this socket.
> > 
> > This patch adds ability to load a library with callbacks for dumping
> > and restoring external sockets.
> > 
> > This patch introduces two callbacks cr_dump_unix_sk and
> > cr_restore_unix_sk. If a callback can not handle a socket, it
> > must return CR_SK_SKIP.
> > 
> > The main questions, what kind of information should be tranfered in
> > these callbacks. Pls, think a few minutes about that and send me
> > your opinion.
> 
> OK, here are my thoughts.
> 
> Naming convention
> =================
> 
> The library functions better could be named with some greppable prefix,
> say criu_plugin__, so the libraries would have been exporting
> 
>  criu_plugin__init(int action)
>  criu_plugin__fini(int status)
> 
> note the calling criu_plugin__fini() is important as well since external libraries
> may have been doing something on its own and will to cleanup own resources
> (int err in criu_plugin__fini should inform the library if dumping/restore is successed,
>  since we restore processes with sigreturn, the __fini should be called
>  right before it I think).

ok

> 
> Data exchange with libraries
> ============================
> 
> I think we might pass protobuf objects to libraries because it's extendable
> so if one day you need to pass more insformation to library you simply update
> protobuf file.

Pavel has suggested to pass structure with size. It's extendable too.

> 
> Headers
> =======
> 
> If people start providing own libraries to criu, we need some api headers
> for that (which would include error codes and such, your CR_CB_SKIP,
> criu__init/fini prototypes and the rest).

cr-lib.h

It may be renamed into criu-lib.h

> 
> For all image objects which migh need own dump restore handlers I guess we could
> provide special type in protobuf files. Something like
> 
> message criu_plugin_hook {
> 	bool		dump	= 1;
> 	bool		restore	= 2;
> }
> 
> thus socket will be something like
> 
> --- a/protobuf/sk-unix.proto
> +++ b/protobuf/sk-unix.proto
> @@ -39,4 +39,5 @@ message unix_sk_entry {
>         optional sk_shutdown            shutdown        = 12;
> 
>         optional file_perms_entry	file_perms	= 13;
> 	optional criu_plugin_hook	plugin_hook	= 14;

diff --git a/protobuf/sk-unix.proto b/protobuf/sk-unix.proto
index bbe2588..9f87baa 100644
--- a/protobuf/sk-unix.proto
+++ b/protobuf/sk-unix.proto
@@ -39,4 +39,5 @@ message unix_sk_entry {
        optional sk_shutdown            shutdown        = 12;
 
        optional file_perms_entry       file_perms      = 13;
+       optional bool                   need_callback   = 14;
 }

I don't understand why we need dump and restore arguments.

>  }
> 
> When we meet a soket which must be additionaly handled, we pass
> unix_sk_entry into a hook function:
> 
> 	int criu_plugin__dump_unix_socket(unix_sk_entry *e)
> 	{
> 		library doing something with this socket
> 		and resturns
> 
> 			< 0 -- error happened
> 			= 0 -- successfully handled
> 
> 		and it sets plugin_hook.restore = true if it will
> 		need this entry to be handled on restore, or false
> 		if nothing is needed
> 	}
> 
> once function handled this entry in criu code we set plugin_hook.dump
> to true, pointing out that the entry on image were handled by external
> library as well (this will help debuggin).
> 
> Thus in short it should be like
> 
> Dump
> ----
> 	criu					plugin
> 	====					======
> 	try_to_dump_with_callback
> 						criu_plugin__dump_unix_socket()
> 						  plugin_hook.restore = true
> 	plugin_hook.dump = true
> 	write image
> 
> Restore
> -------
> 	criu					plugin
> 	====					======
> 	open_unixsk_standalone
> 	  if (plugin_hook.restore)
> 						criu_plugin__restore_unix_socket()
> 						  = 0 on success
> 						  < 1 on error
> 
> Again, since you're sending socket fd into plugin we might need own
> protobuf entry for that, not unix_sk_desc. But I think using protobuf
> here would be a win.

Thank you for the comments.