[v5,18/21] unix: Rework ghost name generation

Submitted by Cyrill Gorcunov on May 10, 2018, 2:57 p.m.

Details

Message ID 20180510145801.12322-19-gorcunov@gmail.com
State New
Series "Add support of deleted unix sockets"
Headers show

Commit Message

Cyrill Gorcunov May 10, 2018, 2:57 p.m.
We need unique names of ghost unix sockets so that they won't
inetrsect with another simultaneously running criu instance,
since while using containers with own root fs is a preferred
there is no mandatory requirement to do so, and people are
free to c/r individual process trees on the host with
share root.

For this sake when we create a unix socket to be treated
as a ghost (ie with furthre delete once connected) we
need it to be a unique amont others. For this sake we
use uuid library which fits our needs. In particular
ghost names are converted into the form of

 | /criu-e7845e6a-28f8-41bf-adda-e61efe81b3f4

The rationale of dropping the former path and make
such transition is that any directory inside the
path may absence and they recreation and removing
while we are creating the socket may race with other
processes.

We've been talking to Andrew if there a way to keep
at least part of the former path for potential debugging
sake but it looks like even so there is no guarantee
that some of path won't be unconverted (say complete
path up to the root is dropped so we could try to
replace slashes with dashes or something but then
there is a chance two or more deleted sockets are
pointing to same name and we will have to resolve
this conflict by hangs again). Thus as to me the
unified uuid based scheme a way more preferred.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 criu/sk-unix.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/sk-unix.c b/criu/sk-unix.c
index 03aca89eee18..1ee4d6da075a 100644
--- a/criu/sk-unix.c
+++ b/criu/sk-unix.c
@@ -2,6 +2,7 @@ 
 #include <linux/netlink.h>
 #include <linux/rtnetlink.h>
 #include <libnl3/netlink/msg.h>
+#include <uuid/uuid.h>
 #include <unistd.h>
 #include <netinet/tcp.h>
 #include <sys/stat.h>
@@ -1845,35 +1846,23 @@  static int init_unix_sk_info(struct unix_sk_info *ui, UnixSkEntry *ue)
 	return 0;
 }
 
-#define GHOST_NAME_FMT		"~criu-%u"
-#define GHOST_NAME_FMT_PREFIX	6 /* num of chars before counter */
+#define GHOST_NAME_FMT		"/criu-%s"
 
 static int ghost_new_name(char *name, size_t namelen,
 			  char **name_new, size_t *namelen_new)
 {
-	char sname[64], *pos, *oldname = name;
-	static unsigned int unix_name_cnt = 0;
+	char uuid_str[64], sname[64], *oldname = name;
+	uuid_t uuid;
 	size_t k;
 
 	pr_debug("\tghost: handling name %s namelen %zu\n", name, namelen);
 
-	for (pos = &name[namelen - 1]; pos > name; pos--) {
-		if (*pos == GHOST_NAME_FMT[0])
-			break;
-	}
-
-	if (strncmp(pos, GHOST_NAME_FMT, GHOST_NAME_FMT_PREFIX) == 0) {
-		unsigned int __cnt;
-
-		if (sscanf(pos, GHOST_NAME_FMT, &__cnt) == 1) {
-			pr_debug("\tghost: unix_name_cnt %d detected\n", __cnt);
-			if (__cnt != unix_name_cnt)
-				unix_name_cnt += __cnt;
-		}
-	}
-
 	memzero(sname, sizeof(sname));
-	k = snprintf(sname, sizeof(sname), GHOST_NAME_FMT, unix_name_cnt++) + 1;
+	memzero(uuid_str, sizeof(uuid_str));
+	uuid_generate(uuid);
+	uuid_unparse_lower(uuid, uuid_str);
+
+	k = snprintf(sname, sizeof(sname), GHOST_NAME_FMT, uuid_str) + 1;
 	if (k > sizeof(sname) || k > UNIX_PATH_MAX) {
 		pr_err("\tghost: New name for socket is too long\n");
 		return -1;

Comments

Andrei Vagin May 12, 2018, 5:04 p.m.
On Thu, May 10, 2018 at 05:57:58PM +0300, Cyrill Gorcunov wrote:
> We need unique names of ghost unix sockets so that they won't
> inetrsect with another simultaneously running criu instance,
> since while using containers with own root fs is a preferred
> there is no mandatory requirement to do so, and people are
> free to c/r individual process trees on the host with
> share root.
> 
> For this sake when we create a unix socket to be treated
> as a ghost (ie with furthre delete once connected) we
> need it to be a unique amont others. For this sake we
> use uuid library which fits our needs. In particular
> ghost names are converted into the form of
> 
>  | /criu-e7845e6a-28f8-41bf-adda-e61efe81b3f4

* We can't create all ghost sockets in /, because it can be read-only.
* We have to create a ghost file on the same mountpoint where is
was before dump.

> 
> The rationale of dropping the former path and make
> such transition is that any directory inside the
> path may absence and they recreation and removing
> while we are creating the socket may race with other
> processes.
> 
> We've been talking to Andrew if there a way to keep
> at least part of the former path for potential debugging
> sake but it looks like even so there is no guarantee
> that some of path won't be unconverted (say complete
> path up to the root is dropped so we could try to
> replace slashes with dashes or something but then
> there is a chance two or more deleted sockets are
> pointing to same name and we will have to resolve
> this conflict by hangs again). Thus as to me the
> unified uuid based scheme a way more preferred.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
>  criu/sk-unix.c | 29 +++++++++--------------------
>  1 file changed, 9 insertions(+), 20 deletions(-)
> 
> diff --git a/criu/sk-unix.c b/criu/sk-unix.c
> index 03aca89eee18..1ee4d6da075a 100644
> --- a/criu/sk-unix.c
> +++ b/criu/sk-unix.c
> @@ -2,6 +2,7 @@
>  #include <linux/netlink.h>
>  #include <linux/rtnetlink.h>
>  #include <libnl3/netlink/msg.h>
> +#include <uuid/uuid.h>
>  #include <unistd.h>
>  #include <netinet/tcp.h>
>  #include <sys/stat.h>
> @@ -1845,35 +1846,23 @@ static int init_unix_sk_info(struct unix_sk_info *ui, UnixSkEntry *ue)
>  	return 0;
>  }
>  
> -#define GHOST_NAME_FMT		"~criu-%u"
> -#define GHOST_NAME_FMT_PREFIX	6 /* num of chars before counter */
> +#define GHOST_NAME_FMT		"/criu-%s"
>  
>  static int ghost_new_name(char *name, size_t namelen,
>  			  char **name_new, size_t *namelen_new)
>  {
> -	char sname[64], *pos, *oldname = name;
> -	static unsigned int unix_name_cnt = 0;
> +	char uuid_str[64], sname[64], *oldname = name;
> +	uuid_t uuid;
>  	size_t k;
>  
>  	pr_debug("\tghost: handling name %s namelen %zu\n", name, namelen);
>  
> -	for (pos = &name[namelen - 1]; pos > name; pos--) {
> -		if (*pos == GHOST_NAME_FMT[0])
> -			break;
> -	}
> -
> -	if (strncmp(pos, GHOST_NAME_FMT, GHOST_NAME_FMT_PREFIX) == 0) {
> -		unsigned int __cnt;
> -
> -		if (sscanf(pos, GHOST_NAME_FMT, &__cnt) == 1) {
> -			pr_debug("\tghost: unix_name_cnt %d detected\n", __cnt);
> -			if (__cnt != unix_name_cnt)
> -				unix_name_cnt += __cnt;
> -		}
> -	}
> -
>  	memzero(sname, sizeof(sname));
> -	k = snprintf(sname, sizeof(sname), GHOST_NAME_FMT, unix_name_cnt++) + 1;
> +	memzero(uuid_str, sizeof(uuid_str));
> +	uuid_generate(uuid);
> +	uuid_unparse_lower(uuid, uuid_str);
> +
> +	k = snprintf(sname, sizeof(sname), GHOST_NAME_FMT, uuid_str) + 1;
>  	if (k > sizeof(sname) || k > UNIX_PATH_MAX) {
>  		pr_err("\tghost: New name for socket is too long\n");
>  		return -1;
> -- 
> 2.14.3
>
Cyrill Gorcunov May 12, 2018, 5:11 p.m.
On Sat, May 12, 2018 at 10:04:55AM -0700, Andrey Vagin wrote:
> On Thu, May 10, 2018 at 05:57:58PM +0300, Cyrill Gorcunov wrote:
> > We need unique names of ghost unix sockets so that they won't
> > inetrsect with another simultaneously running criu instance,
> > since while using containers with own root fs is a preferred
> > there is no mandatory requirement to do so, and people are
> > free to c/r individual process trees on the host with
> > share root.
> > 
> > For this sake when we create a unix socket to be treated
> > as a ghost (ie with furthre delete once connected) we
> > need it to be a unique amont others. For this sake we
> > use uuid library which fits our needs. In particular
> > ghost names are converted into the form of
> > 
> >  | /criu-e7845e6a-28f8-41bf-adda-e61efe81b3f4
> 
> * We can't create all ghost sockets in /, because it can be read-only.
> * We have to create a ghost file on the same mountpoint where is
> was before dump.
> 

As been discussed face to face, I'll rework this moment
to keep former names.