Skip to content

misc: add NVMe device support to get_scratch_devices() - #2118

Open
deepssin wants to merge 1 commit into
ceph:mainfrom
deepssin:fix-nvme-scratch-devices
Open

misc: add NVMe device support to get_scratch_devices()#2118
deepssin wants to merge 1 commit into
ceph:mainfrom
deepssin:fix-nvme-scratch-devices

Conversation

@deepssin

Copy link
Copy Markdown
Contributor

When /scratch_devs file is missing, the function only detected traditional SCSI/VirtIO devices (/dev/sd? and /dev/vd?), causing failures on machines using NVMe devices.

This change:

  • Adds NVMe device detection (/dev/nvme*n1) as fallback
  • Filters out nvme0n1 as root device (similar to vda)
  • Fixes iteration bug when removing root devices
  • Filters out empty strings from device list

@deepssin
deepssin requested review from djgalloway and zmc December 24, 2025 10:45
@deepssin
deepssin requested a review from a team as a code owner December 24, 2025 10:45
@deepssin
deepssin requested review from amathuria and removed request for a team December 24, 2025 10:45
Comment thread teuthology/misc.py Outdated
devs.remove(dev)
if not dev:
continue
if 'vda' in dev or 'nvme0n1' in dev:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the configurations where are mixed devices installed, for example, where vda is boot and nvme are data, this code will ignore one device, which should not be ignored, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed root device filtering to only filter nvme0n1 when vda doesn't exist, preventing false positives in mixed configurations.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, here are another suggestions,
first, if the devices is listed in the scratch_devs file, there should be no exclusion, from my understanding, because if it is manually defined per machine type or hardware layout operator knows why it is there;
second, in order to exclude root in case there is no scratch_devs file we need to check which device is actually mounted as a root, the best way I guess is to look into mount table or even use findmnt --real
For example:

findmnt --real --json
{
   "filesystems": [
      {
         "target": "/",
         "source": "/dev/vda1",
         "fstype": "xfs",
         "options": "rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota"
      }
   ]
}

Just looking for "/" will give us device where the root is mounted, right?

$ findmnt --real --json | jq -r '.filesystems[] | select(.target == "/")| .source'
/dev/vda1

Comment thread teuthology/misc.py Outdated
When /scratch_devs file is missing, the function only detected
traditional SCSI/VirtIO devices (/dev/sd? and /dev/vd?), causing
failures on machines using NVMe devices.

This change:
- Adds NVMe device detection (/dev/nvme*n1) as fallback
- Filters out nvme0n1 as root device (similar to vda)
- Fixes iteration bug when removing root devices
- Filters out empty strings from device list

Signed-off-by: deepssin <deepssin@redhat.com>
@deepssin
deepssin force-pushed the fix-nvme-scratch-devices branch from 593fb6a to f84d2e3 Compare January 21, 2026 12:52

@zmc zmc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me - could we see a small test run using it?

@deepssin

Copy link
Copy Markdown
Contributor Author

I tested this earlier on NVMe hardware, but I didn't keep the logs and no longer have access to that setup to rerun it. The change is minimal, but a quick verification on NVMe backed hardware would be helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants