NAS-132816 / 24.10.2 / Fix disk.format (by yocalebo) (#15071)

* call disk.wipe in disk.format

(cherry picked from commit 20696c7)

* add explicit disk.format test

(cherry picked from commit ad2b003)

---------

Co-authored-by: Caleb <[email protected]>

src/middlewared/middlewared/plugins/disk_/format.py

@@ -19,35 +19,10 @@ def format(self, disk):
             # 0 == disabled, > 0 enabled
             raise CallError(f'Disk: {disk!r} is incorrectly formatted with Data Integrity Feature (DIF).')
 
-        dev = parted.getDevice(f'/dev/{disk}')
-        # it's important we remove this device from the global cache
-        # so that libparted probes the disk for the latest up-to-date
-        # information. This becomes _very_ important, for example,
-        # when we overprovision disk devices. If the disk is overprovisioned
-        # to a larger/smaller size, then libparted has possibility of
-        # referencing the old disk size. So depending on the direction of
-        # the resize operation, the `clobber()` operation can run off of
-        # the end of the disk and raise an IO failure. We actually saw this
-        # interally during testing
-        dev._Device__device.cache_remove()
-        for i in range(2):
-            if not dev.clobber():
-                # clobber() wipes partition label info from disk but during testing
-                # on an m40 HA system, the disk had to be clobber()'ed twice before
-                # fdisk -l wouldn't show any partitions. Only doing it once showed
-                # the following output
-                # Disk /dev/sda: 10.91 TiB, 12000138625024 bytes, 2929721344 sectors
-                # Disk model: HUH721212AL4200
-                # Units: sectors of 1 * 4096 = 4096 bytes
-                # Sector size (logical/physical): 4096 bytes / 4096 bytes
-                # I/O size (minimum/optimal): 4096 bytes / 4096 bytes
-                # Disklabel type: dos
-                # Disk identifier: 0x00000000
-                #
-                # Device     Boot Start        End    Sectors  Size Id Type
-                # /dev/sda1           1 2929721343 2929721343 10.9T ee GPT
-                raise CallError(f'Failed on attempt #{i} clearing partition labels for {disk!r}')
+        # wipe the disk (quickly) of any existing filesystems
+        self.middleware.call_sync('disk.wipe', disk, 'QUICK').wait_sync(raise_error=True)
 
+        dev = parted.getDevice(f'/dev/{disk}')
         parted_disk = parted.freshDisk(dev, 'gpt')
         regions = sorted(parted_disk.getFreeSpaceRegions(), key=lambda x: x.length)[-1]
         geom = parted.Geometry(start=regions.start, end=regions.end, device=dev)

tests/api2/test_disk_wipe.py

@@ -119,3 +119,29 @@ def test_disk_wipe_abort():
         time.sleep(0.1)
     else:
         assert False, result
+
+def test_disk_format():
+    """Explicitly test the `disk.format` method since
+    this is the endpoint that is eventually called when
+    a disk is added to a zpool using our public API."""
+    disk = call("disk.get_unused")[0]["name"]
+    # create a GPT label and a 100MiB EXT4 partition
+    ssh(f"parted -s /dev/{disk} mklabel gpt mkpart ext4 16384s 100MiB; mkfs.ext4 /dev/{disk}1")
+    info = call("disk.list_partitions", disk)
+    assert len(info) == 1, info
+    assert info[0]["partition_type"] == "0fc63daf-8483-4772-8e79-3d69d8477de4"
+    assert info[0]["start_sector"] == 16384
+
+    # format the disk with a zfs data partition
+    # NOTE: this calls `disk.wipe` and so it should
+    # wipe the ext4 information
+    call("disk.format", disk)
+    info = call("disk.list_partitions", disk)
+    assert len(info) == 1, info
+    assert info[0]["partition_type"] == "6a898cc3-1dd2-11b2-99a6-080020736631"
+
+    # let's make sure wipefs (aka libblkid) doesn't report
+    # stale ext4 information
+    lines = ssh(f"wipefs /dev/{disk}1").splitlines()
+    for line in lines:
+        assert "ext4" not in line, line