Merge pull request #736 from giuseppe/criu-fix-store-external-fds

criu: fix save of external descriptors

src/libcrun/linux.c

@@ -2700,48 +2700,55 @@ libcrun_get_external_descriptors (libcrun_container_t *container)
   return get_private_data (container)->external_descriptors;
 }
 
+/*
+ * Save the external descriptors as a NUL terminated string list
+ * e.g. "/dev/pts/2\0/dev/pts/2\0/dev/pts/2\0\0"
+ */
 static int
 save_external_descriptors (libcrun_container_t *container, pid_t pid, libcrun_error_t *err)
 {
-  const unsigned char *buf = NULL;
-  yajl_gen gen = NULL;
-  size_t buf_len;
-  int ret;
+  cleanup_free char *buf = NULL;
+  size_t buf_allocated = 0;
+  size_t buf_size = 0;
   int i;
 
-  gen = yajl_gen_alloc (NULL);
-  if (gen == NULL)
-    return crun_make_error (err, errno, "yajl_gen_alloc");
-  yajl_gen_array_open (gen);
+  /* Just a guess.  */
+  buf_allocated = 64;
+  buf = xmalloc (buf_allocated);
 
-  /* Remember original stdin, stdout, stderr for container restore. */
+  /* Remember original stdin, stdout, stderr for container restore.  */
   for (i = 0; i < 3; i++)
     {
-      char fd_path[64];
       char link_path[PATH_MAX];
+      char fd_path[64];
+      ssize_t ret;
+
       sprintf (fd_path, "/proc/%d/fd/%d", pid, i);
       ret = readlink (fd_path, link_path, PATH_MAX - 1);
       if (UNLIKELY (ret < 0))
         {
           /* The fd could not exist.  */
-          if (errno == ENOENT)
-            strcpy (link_path, "/dev/null");
-          else
-            {
-              yajl_gen_free (gen);
-              return crun_make_error (err, errno, "readlink");
-            }
+          if (errno != ENOENT)
+            return crun_make_error (err, errno, "readlink");
+
+          strcpy (link_path, "/dev/null");
+          ret = 9;
         }
-      else
-        link_path[ret] = 0;
-      yajl_gen_string (gen, YAJL_STR (link_path), strlen (link_path));
+
+      if (buf_allocated < buf_size + ret + 2)
+        {
+          buf_allocated = buf_size + ret + 2;
+          buf = xrealloc (buf, buf_allocated);
+        }
+      memcpy (buf + buf_size, link_path, ret);
+      buf_size += ret;
+      buf[buf_size++] = '\0';
+      buf[buf_size] = '\0';
     }
 
-  yajl_gen_array_close (gen);
-  yajl_gen_get_buf (gen, &buf, &buf_len);
-  if (buf)
-    get_private_data (container)->external_descriptors = xstrdup ((const char *) buf);
-  yajl_gen_free (gen);
+  /* Move ownership.  */
+  get_private_data (container)->external_descriptors = buf;
+  buf = NULL;
 
   return 0;
 }

src/libcrun/status.c

@@ -292,9 +292,28 @@ libcrun_write_container_status (const char *state_root, const char *id, libcrun_
   if (UNLIKELY (r != yajl_gen_status_ok))
     goto yajl_error;
 
-  r = yajl_gen_string (gen, YAJL_STR (status->external_descriptors), strlen (status->external_descriptors));
-  if (UNLIKELY (r != yajl_gen_status_ok))
-    goto yajl_error;
+  {
+    const char *it = status->external_descriptors;
+
+    r = yajl_gen_array_open (gen);
+    if (UNLIKELY (r != yajl_gen_status_ok))
+      goto yajl_error;
+
+    while (*it)
+      {
+        size_t len = strlen (it);
+
+        r = yajl_gen_string (gen, (unsigned char *) it, len);
+        if (UNLIKELY (r != yajl_gen_status_ok))
+          goto yajl_error;
+
+        it += len + 1;
+      }
+
+    r = yajl_gen_array_close (gen);
+    if (UNLIKELY (r != yajl_gen_status_ok))
+      goto yajl_error;
+  }
 
   r = yajl_gen_map_close (gen);
   if (UNLIKELY (r != yajl_gen_status_ok))

tests/test_start.py

@@ -21,6 +21,7 @@
 import os.path
 import threading
 import socket
+import json
 from tests_utils import *
 
 def test_cwd_relative():
@@ -80,6 +81,15 @@ def test_start():
         out, _ = proc.communicate()
         if "hello" not in str(out):
             return -1
+
+        # verify that the external_descriptors are stored correctly
+        path = os.path.join(get_tests_root_status(), cid, "status")
+        with open(path) as f:
+            status = json.load(f)
+            descriptors = status["external_descriptors"]
+            if len(descriptors) <= 1:
+                print("invalid length for external_descriptors")
+                return -1
     finally:
         if cid is not None:
             run_crun_command(["delete", "-f", cid])

tests/tests_utils.py

@@ -185,6 +185,9 @@ def run_all_tests(all_tests, allowed_tests):
 def get_tests_root():
     return '%s/.testsuite-run-%d' % (os.getcwd(), os.getpid())
 
+def get_tests_root_status():
+    return os.path.join(get_tests_root(), "root")
+
 def get_crun_path():
     cwd = os.getcwd()
     return os.getenv("OCI_RUNTIME") or os.path.join(cwd, "crun")
@@ -243,7 +246,8 @@ def run_and_get_output(config, detach=False, preserve_fds=None, pid_file=None,
     pid_file_arg = ['--pid-file', pid_file] if pid_file else []
     relative_config_path = ['--config', relative_config_path] if relative_config_path else []
 
-    args = [crun, command] + relative_config_path + preserve_fds_arg + detach_arg + pid_file_arg + [id_container]
+    root = get_tests_root_status()
+    args = [crun, "--root", root, command] + relative_config_path + preserve_fds_arg + detach_arg + pid_file_arg + [id_container]
 
     stderr = subprocess.STDOUT
     if hide_stderr:
@@ -266,8 +270,9 @@ def run_and_get_output(config, detach=False, preserve_fds=None, pid_file=None,
         return subprocess.check_output(args, cwd=temp_dir, stderr=stderr, env=env, close_fds=False).decode(), id_container
 
 def run_crun_command(args):
+    root = get_tests_root_status()
     crun = get_crun_path()
-    args = [crun] + args
+    args = [crun, "--root", root] + args
     return subprocess.check_output(args, close_fds=False).decode()
 
 def tests_main(all_tests):