Merge pull request #739 from giuseppe/criu-use-json-representation

criu: store external descriptors as JSON string

.github/workflows/test.yaml

@@ -99,14 +99,14 @@ jobs:
                 make
                 make syntax-check
                 echo run tests as root
-                # check that the working dir is clean
-                git describe --broken --dirty --all | grep -qv dirty
-
                 sudo make check ASAN_OPTIONS=detect_leaks=false || cat test-suite.log
                 echo run tests as rootless
                 make check ASAN_OPTIONS=detect_leaks=false || (cat test-suite.log; exit 1)
                 echo run tests as rootless in a user namespace
                 unshare -r make check ASAN_OPTIONS=detect_leaks=false || (cat test-suite.log; exit 1)
+
+                # check that the working dir is clean
+                git describe --broken --dirty --all | grep -qv dirty
             ;;
             podman)
                 sudo docker build -t crun-podman tests/podman

src/libcrun/linux.c

@@ -2700,57 +2700,75 @@ 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)
 {
-  cleanup_free char *buf = NULL;
-  size_t buf_allocated = 0;
-  size_t buf_size = 0;
+  const unsigned char *buf = NULL;
+  yajl_gen gen = NULL;
+  size_t buf_len;
+  int ret;
   int i;
 
-  /* Just a guess.  */
-  buf_allocated = 64;
-  buf = xmalloc (buf_allocated);
+  gen = yajl_gen_alloc (NULL);
+  if (gen == NULL)
+    return crun_make_error (err, errno, "yajl_gen_alloc");
+
+  ret = yajl_gen_array_open (gen);
+  if (UNLIKELY (ret != yajl_gen_status_ok))
+    goto yajl_error;
 
   /* Remember original stdin, stdout, stderr for container restore.  */
   for (i = 0; i < 3; i++)
     {
-      char link_path[PATH_MAX];
       char fd_path[64];
-      ssize_t ret;
-
+      char link_path[PATH_MAX];
       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)
-            return crun_make_error (err, errno, "readlink");
-
-          strcpy (link_path, "/dev/null");
-          ret = 9;
+          if (errno == ENOENT)
+            {
+              strcpy (link_path, "/dev/null");
+              ret = 9; /* strlen ("/dev/null").  */
+            }
+          else
+            {
+              yajl_gen_free (gen);
+              return crun_make_error (err, errno, "readlink");
+            }
         }
+      link_path[ret] = 0;
 
-      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';
+      ret = yajl_gen_string (gen, YAJL_STR (link_path), ret);
+      if (UNLIKELY (ret != yajl_gen_status_ok))
+        goto yajl_error;
     }
 
-  /* Move ownership.  */
-  get_private_data (container)->external_descriptors = buf;
-  buf = NULL;
+  ret = yajl_gen_array_close (gen);
+  if (UNLIKELY (ret != yajl_gen_status_ok))
+    goto yajl_error;
+
+  ret = yajl_gen_get_buf (gen, &buf, &buf_len);
+  if (UNLIKELY (ret != yajl_gen_status_ok))
+    goto yajl_error;
+
+  if (buf)
+    {
+      char *b = xmalloc (buf_len + 1);
+      memcpy (b, buf, buf_len);
+      b[buf_len] = '\0';
+      get_private_data (container)->external_descriptors = b;
+    }
+
+  yajl_gen_free (gen);
 
   return 0;
+
+yajl_error:
+  if (gen)
+    yajl_gen_free (gen);
+  return yajl_error_to_crun_error (ret, err);
 }
 
 int

src/libcrun/status.c

@@ -292,28 +292,9 @@ libcrun_write_container_status (const char *state_root, const char *id, libcrun_
   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_string (gen, YAJL_STR (status->external_descriptors), strlen (status->external_descriptors));
+  if (UNLIKELY (r != yajl_gen_status_ok))
+    goto yajl_error;
 
   r = yajl_gen_map_close (gen);
   if (UNLIKELY (r != yajl_gen_status_ok))
@@ -430,36 +411,9 @@ libcrun_read_container_status (libcrun_container_status_t *status, const char *s
     status->detached = YAJL_IS_TRUE (yajl_tree_get (tree, detached, yajl_t_true));
   }
   {
-    const char *external[] = { "external_descriptors", NULL };
-    const unsigned char *buf = NULL;
-    yajl_gen gen = NULL;
-    size_t buf_len;
-
-    gen = yajl_gen_alloc (NULL);
-    if (gen == NULL)
-      return crun_make_error (err, errno, "yajl_gen_alloc");
-    yajl_gen_array_open (gen);
-
-    tmp = yajl_tree_get (tree, external, yajl_t_array);
-    if (tmp && YAJL_IS_ARRAY (tmp))
-      {
-        size_t len = tmp->u.array.len;
-        size_t i;
-        for (i = 0; i < len; ++i)
-          {
-            yajl_val s = tmp->u.array.values[i];
-            if (s && YAJL_IS_STRING (s))
-              {
-                char *str = YAJL_GET_STRING (s);
-                yajl_gen_string (gen, YAJL_STR (str), strlen (str));
-              }
-          }
-      }
-    yajl_gen_array_close (gen);
-    yajl_gen_get_buf (gen, &buf, &buf_len);
-    if (buf)
-      status->external_descriptors = xstrdup ((const char *) buf);
-    yajl_gen_free (gen);
+    const char *external_descriptors[] = { "external_descriptors", NULL };
+    tmp = yajl_tree_get (tree, external_descriptors, yajl_t_string);
+    status->external_descriptors = tmp ? xstrdup (YAJL_GET_STRING (tmp)) : NULL;
   }
   yajl_tree_free (tree);
   return 0;

tests/test_start.py

@@ -87,8 +87,8 @@ def test_start():
         with open(path) as f:
             status = json.load(f)
             descriptors = status["external_descriptors"]
-            if len(descriptors) <= 1:
-                print("invalid length for external_descriptors")
+            if not isinstance(descriptors, str):
+                print("external_descriptors is not a string")
                 return -1
     finally:
         if cid is not None: