Skip to content

[DNM, WIP] feat: rework BD naming to avoid requiring WWNs#266

Draft
tserong wants to merge 10 commits intoharvester:masterfrom
tserong:wip-dont-require-wwn
Draft

[DNM, WIP] feat: rework BD naming to avoid requiring WWNs#266
tserong wants to merge 10 commits intoharvester:masterfrom
tserong:wip-dont-require-wwn

Conversation

@tserong
Copy link
Member

@tserong tserong commented Feb 20, 2026

Problem:
We can't really cope properly with disks that don't have WWNs, or whose WWNs change (the latter should never happen, but it did at least once in the past when a kernel update resulted in IDs for a certain make and model of NVMe changing).

Solution:

  • Identify disks by, in order:
    1. UUID if present (will be there for provisioned LHv1 volumes and LVM volumes)
    2. WWN if present
    3. Vendor+Model+SerialNumber+BusPath if none of the above exist.
  • BD CR names become non-deterministic (they're UUIDs for any new BDs after this change), so when scanning we don't check existing BDs by name, we check if there's one with the same UUID, then WWN, then Vendor+Model+Serial+BusPath.
  • This automatically prevents adding devices with duplicate FS/LVM UUIDs or WWNs
  • It would be nice if we could also interrogate provisioned LHv2 devices when scanned but that's rather more problematic (long story, I'll write up the details soon, TL;DR: this new implementation shouldn't be any more broken than the old one was for LHv2 disks without WWNs)

TODO: explain WTF I'm doing here in more detail ;-)

Related Issue:
harvester/harvester#6261

Test plan:
TBD

NOTE:
I'm still working on this. Don't worry too much about reviewing it yet.

@tserong tserong requested review from a team, Vicente-Cheng and Yu-Jack as code owners February 20, 2026 08:25
@tserong tserong marked this pull request as draft February 20, 2026 08:25
@tserong tserong force-pushed the wip-dont-require-wwn branch 4 times, most recently from 283c11a to 8d79c78 Compare February 23, 2026 06:33
…ices

harvester#253 updated BD CRs
to use stable /dev/mapper paths for multipath devices.  These paths need
to be resolved back to /dev/dm-0 when calling GetDiskByDevPath(),
otherwise fetching details like the WWN and Serial number from
/sys/block will fail, which means we lose the device WWN, SerialNumber
and Capacity information when multipath devices are provisioned.

Signed-off-by: Tim Serong <[email protected]>
TODO: Drop all the PartFilter stuff (should no longer apply)

Signed-off-by: Tim Serong <[email protected]>
With debug logging turned on, you will periodically see something like
the following:

time="2026-02-19T09:55:30Z" level=debug msg="Prepare to checking block device ac1b8d0086729ec4840eac4e2319cfaf"
time="2026-02-19T09:55:30Z" level=debug msg="Update block device ac1b8d0086729ec4840eac4e2319cfaf tags (Status) from [] to []"

Note that it's triggering an update even though the tags don't appear to
have changed (they're both rendered in the logs as empty slices).

The problem is that bd.Status.Tags is specified as `json:"tags,omitempty"`
but disk.Tags is specified as `json:"tags"`. This means that if both are
empty, bd.Status.Tags will be nil, while disk.Tags will be an empty slice.
In this case, reflect.DeepEqual() will return false, which will result
in an unnecessary update.  We can avoid this by skipping the DeepCopy
and DeepEqual entirely if both bd.Status.Tags and disk.Tags are already
empty.

Signed-off-by: Tim Serong <[email protected]>
@tserong tserong force-pushed the wip-dont-require-wwn branch from 54c17c2 to d6011c8 Compare February 25, 2026 09:46
TODO: explain exactly WTF I've done here ;-)

Related-to: harvester/harvester#6261
Signed-off-by: Tim Serong <[email protected]>
There's a couple of changes we need to make here:

1. We're actually starting with two blockdevices now, not one,
   because the new NDM code picks up /dev/vdb in the vagrant host
2. The duplicated WWN device needs a new backing file, otherwise
   it'll pick up the duplicate disk UUID, not the duplicate WWN.

Signed-off-by: Tim Serong <[email protected]>
There'll be more than the two expected block devices thanks for NDM now
picking up /dev/vdb.  It's more robust to check how many we started with
then expect to end up with two more than whatever that original number
was.

Signed-off-by: Tim Serong <[email protected]>
Signed-off-by: Tim Serong <[email protected]>
@tserong tserong force-pushed the wip-dont-require-wwn branch from d6011c8 to aa16be7 Compare February 27, 2026 08:39
@tserong tserong force-pushed the wip-dont-require-wwn branch from aa16be7 to b800238 Compare February 27, 2026 09:14
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.

1 participant