launchpad-reviewers team mailing list archive

[Colin Watson <mp+443813@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad-buildd:dm-mknod-check-existence into launchpad-buildd:master.

Commit message:
Only create /dev/dm-* in LXD containers if they don't already exist

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/+git/launchpad-buildd/+merge/443813

This fixes devtmpfs-related failures on riscv64, since apparently at least `/dev/dm-0` ends up existing there in a freshly-mounted devtmpfs - probably something to do with those being non-virtualized builders.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-buildd:dm-mknod-check-existence into launchpad-buildd:master.

diff --git a/debian/changelog b/debian/changelog
index 18cfa5e..aa10235 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+launchpad-buildd (233) UNRELEASED; urgency=medium
+
+  * Only create /dev/dm-* in LXD containers if they don't already exist
+    (fixes devtmpfs-related failures on riscv64).
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx>  Tue, 30 May 2023 16:44:15 +0100
+
 launchpad-buildd (232) focal; urgency=medium
 
   [ Colin Watson ]
diff --git a/lpbuildd/target/lxd.py b/lpbuildd/target/lxd.py
index 7b71a88..832537a 100644
--- a/lpbuildd/target/lxd.py
+++ b/lpbuildd/target/lxd.py
@@ -595,17 +595,18 @@ class LXD(Backend):
         # in their absence.
         major = get_device_mapper_major()
         for minor in range(8):
-            self.run(
-                [
-                    "mknod",
-                    "-m",
-                    "0660",
-                    "/dev/dm-%d" % minor,
-                    "b",
-                    str(major),
-                    str(minor),
-                ]
-            )
+            if not self.path_exists(f"/dev/dm-{minor}"):
+                self.run(
+                    [
+                        "mknod",
+                        "-m",
+                        "0660",
+                        f"/dev/dm-{minor}",
+                        "b",
+                        str(major),
+                        str(minor),
+                    ]
+                )
 
         if "gpu-nvidia" in self.constraints:
             # Create nvidia* devices.  We have to do this here rather than
diff --git a/lpbuildd/target/tests/test_lxd.py b/lpbuildd/target/tests/test_lxd.py
index 2cbe972..a5caba0 100644
--- a/lpbuildd/target/tests/test_lxd.py
+++ b/lpbuildd/target/tests/test_lxd.py
@@ -452,6 +452,7 @@ class TestLXD(TestCase):
         self,
         arch="amd64",
         unmounts_cpuinfo=False,
+        dm_device_nodes_exist=False,
         gpu_nvidia=False,
         gpu_nvidia_device_nodes_exist=False,
     ):
@@ -481,6 +482,9 @@ class TestLXD(TestCase):
         processes_fixture.add(
             FakeHostname("example", "example.buildd"), name="hostname"
         )
+        if dm_device_nodes_exist:
+            for minor in range(8):
+                existing_files[f"/dev/dm-{minor}"] = []
         if gpu_nvidia:
             os.mknod("/dev/nvidia0", stat.S_IFCHR | 0o666, os.makedev(195, 0))
             os.mknod(
@@ -603,21 +607,22 @@ class TestLXD(TestCase):
                 Equals(["hostname", "--fqdn"]),
             ]
         )
-        for minor in range(8):
-            expected_args.append(
-                Equals(
-                    lxc
-                    + [
-                        "mknod",
-                        "-m",
-                        "0660",
-                        "/dev/dm-%d" % minor,
-                        "b",
-                        str(DM_BLOCK_MAJOR),
-                        str(minor),
-                    ]
+        if not dm_device_nodes_exist:
+            for minor in range(8):
+                expected_args.append(
+                    Equals(
+                        lxc
+                        + [
+                            "mknod",
+                            "-m",
+                            "0660",
+                            f"/dev/dm-{minor}",
+                            "b",
+                            str(DM_BLOCK_MAJOR),
+                            str(minor),
+                        ]
+                    )
                 )
-            )
         if gpu_nvidia:
             if not gpu_nvidia_device_nodes_exist:
                 expected_args.extend(
@@ -814,6 +819,11 @@ class TestLXD(TestCase):
     def test_start_armhf_unmounts_cpuinfo(self):
         self.test_start(arch="armhf", unmounts_cpuinfo=True)
 
+    def test_start_dm_device_nodes_exist(self):
+        # Starting a container works even if mounting devtmpfs inside the
+        # container causes dm-* device nodes to exist.
+        self.test_start(dm_device_nodes_exist=True)
+
     def test_start_gpu_nvidia(self):
         self.test_start(gpu_nvidia=True)