Skip to content

Commit

Permalink
NAS-132816 / 25.04 / Fix disk.format (#15068)
Browse files Browse the repository at this point in the history
* call disk.wipe in disk.format

* add explicit disk.format test
  • Loading branch information
yocalebo authored Dec 2, 2024
1 parent ac2a55f commit 383cc5c
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 28 deletions.
31 changes: 3 additions & 28 deletions src/middlewared/middlewared/plugins/disk_/format.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 26 additions & 0 deletions tests/api2/test_disk_wipe.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 383cc5c

Please sign in to comment.