[CRIU] [PATCH 3/5] unix: add ability to set callbacks for external sockets (v2)

[Andrew Vagin]

On Fri, Dec 06, 2013 at 01:20:30PM +0400, Andrew Vagin wrote:
> On Fri, Dec 06, 2013 at 12:43:35PM +0400, Pavel Emelyanov wrote:
> > On 12/06/2013 07:58 AM, Andrew Vagin wrote:
> > > On Thu, Dec 05, 2013 at 10:29:36PM +0400, Pavel Emelyanov wrote:
> > >>> @@ -469,6 +471,30 @@ int unix_receive_one(struct nlmsghdr *h, void *arg)
> > >>>  	return unix_collect_one(m, tb);
> > >>>  }
> > >>>  
> > >>> +static int try_to_dump_with_callback(struct unix_sk_desc *peer, bool *need_callback)
> > >>> +{
> > >>> +	struct unix_sk_desc *sk;
> > >>> +	int ret = -1;
> > >>> +
> > >>> +	*need_callback = false;
> > >>> +	list_for_each_entry(sk, &peer->peer_list, peer_node) {
> > >>> +		ret = cr_plugin_dump_unix_sk(sk->fd, sk->sd.ino, sk->peer_ino);
> > >>> +		if (ret == CRIU_CB_SKIP) {
> > >>> +			if (!*need_callback)
> > >>> +				return 0;
> > >>
> > >> If the very first library says "don't know this socket" we get out of this fn? Why?
> > > 
> > > If ALL librararies say "don't know this socket" for the first socket we
> > > get out of this fn.
> > > 
> > > We are dumping all peer-s of an external sockets and if one of peer-s
> > > was dumped by a library, all other should be dumped by the same labrary.
> > 
> > But this code tries to dump other peers with _all_ libraries again. Not
> > by the same library. Is it OK? Why?
> 
> It's OK, if libraries will handle only own sockets. We can use the same
> library for all socket, but the code will be a bit more complicated.

Look at the attached patch, it calls the same callback for dumping
sockets. I don't know do we need the same for restoring sockets.
Currently we restore each external socket separately.

> 
> > 
> > > Currently the code doesn't check "the same".
> > > 
> > > 
> > >>
> > >>> +
> > >>> +			pr_err("The callback is not suitable for all peer sockets\n");
> > >>> +			return -1;
> > >>> +		}
> > >>> +		if (ret < 0)
> > >>> +			return -1;
> > >>> +
> > >>> +		*need_callback = true;
> > >>> +	}
> > >>> +
> > >>> +	return ret;
> > >>> +}
> > >>> +
> > > .
> > > 
> > 
> > 
-------------- next part --------------
diff --git a/include/plugin.h b/include/plugin.h
index 8538a1b..6977600 100644
--- a/include/plugin.h
+++ b/include/plugin.h
@@ -20,12 +20,12 @@ struct cr_plugins {
 	struct cr_plugin_entry *cr_plugin_restore_unix_sk;
 };
 
-extern struct cr_plugins cr_plugins;
-
 void cr_lib_fini(void);
 int cr_lib_init(void);
 
-int cr_plugin_dump_unix_sk(int fd, int ino, int peer_ino);
-int cr_plugin_restore_unix_sk(int ino, char *name, int len);
+int cr_plugin_dump_unix_sk(int fd, int ino, int peer_ino,
+				cr_plugin_dump_unix_sk_t **cb);
+int cr_plugin_restore_unix_sk(int ino, char *name, int len,
+				cr_plugin_restore_unix_sk_t **cb);
 
 #endif
diff --git a/lib.c b/lib.c
index 28ab101..b9679ae 100644
--- a/lib.c
+++ b/lib.c
@@ -28,7 +28,7 @@ struct cr_plugins cr_plugins;
 	} while (0);							\
 
 
-#define run_plugin_funcs(name, ...) ({					\
+#define run_plugin_funcs(name, cb, ...) ({				\
 		struct cr_plugin_entry *__ce = cr_plugins.name;		\
 		int __ret = CRIU_CB_SKIP;				\
 									\
@@ -38,20 +38,22 @@ struct cr_plugins cr_plugins;
 				__ce = __ce->next;			\
 				continue;				\
 			}						\
+			if (cb != NULL)					\
+				*cb = __ce->name;			\
 			break;						\
 		}							\
 									\
 		__ret;							\
 	})								\
 
-int cr_plugin_dump_unix_sk(int fd, int ino, int peer_ino)
+int cr_plugin_dump_unix_sk(int fd, int ino, int peer_ino, cr_plugin_dump_unix_sk_t **cb)
 {
-	return run_plugin_funcs(cr_plugin_dump_unix_sk, fd, ino, peer_ino);
+	return run_plugin_funcs(cr_plugin_dump_unix_sk, cb, fd, ino, peer_ino);
 }
 
-int cr_plugin_restore_unix_sk(int ino, char *name, int len)
+int cr_plugin_restore_unix_sk(int ino, char *name, int len, cr_plugin_restore_unix_sk_t **cb)
 {
-	return run_plugin_funcs(cr_plugin_restore_unix_sk, ino, name, len);
+	return run_plugin_funcs(cr_plugin_restore_unix_sk, cb, ino, name, len);
 }
 
 static int cr_lib_load(char *path)
@@ -173,7 +175,8 @@ int cr_lib_init(void)
 err:
 	closedir(d);
 
-	cr_lib_fini();
+	if (exit_code)
+		cr_lib_fini();
 
 	return exit_code;
 }
diff --git a/sk-unix.c b/sk-unix.c
index 9dd8280..3170670 100644
--- a/sk-unix.c
+++ b/sk-unix.c
@@ -473,23 +473,27 @@ int unix_receive_one(struct nlmsghdr *h, void *arg)
 
 static int try_to_dump_with_callback(struct unix_sk_desc *peer, bool *need_callback)
 {
+	cr_plugin_dump_unix_sk_t *cb;
 	struct unix_sk_desc *sk;
 	int ret = -1;
 
 	*need_callback = false;
-	list_for_each_entry(sk, &peer->peer_list, peer_node) {
-		ret = cr_plugin_dump_unix_sk(sk->fd, sk->sd.ino, sk->peer_ino);
-		if (ret == CRIU_CB_SKIP) {
-			if (!*need_callback)
-				return 0;
+	sk = list_first_entry(&peer->peer_list, struct unix_sk_desc, peer_node);
+	ret = cr_plugin_dump_unix_sk(sk->fd, sk->sd.ino, sk->peer_ino, &cb);
+	if (ret == CRIU_CB_SKIP)
+		return 0;
+	if (ret < 0)
+		return -1;
 
+	*need_callback = true;
+	list_for_each_entry_continue(sk, &peer->peer_list, peer_node) {
+		ret = cb(sk->fd, sk->sd.ino, sk->peer_ino);
+		if (ret == CRIU_CB_SKIP) {
 			pr_err("The callback is not suitable for all peer sockets\n");
 			return -1;
 		}
 		if (ret < 0)
 			return -1;
-
-		*need_callback = true;
 	}
 
 	return ret;