Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kola/qemuexec: Add a wwn option for scsis disks #3772

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

jbtrystram
Copy link
Contributor

Add a customizable WWN option for kola DiskSpec to have reliable links under /dev/disk/by-id/. With this change kola qemuxec can be run like: kola qemuexec -D "5G:channel=scsi,wwn=007"

Resulting in the following links:

[core@localhost ~]$ rpm -qa sg3_utils
sg3_utils-1.48-1.fc40.x86_64
[core@localhost ~]$ ls -l /dev/disk/by-id
total 0
lrwxrwxrwx. 1 root root  9 Apr  5 09:05 scsi-30000000000000007 -> ../../sda
lrwxrwxrwx. 1 root root  9 Apr  5 09:05 wwn-0x0000000000000007 -> ../../sda

This is motivated by recent changes in sg3_utils [1] which removed some udev links.
At least one of our tests [2] relying on this started failing. This patch was suggested by @jlebon [3]

[1] https://listman.redhat.com/archives/dm-devel/2023-March/053645.html
[2] coreos/fedora-coreos-tracker#1670
[3] coreos/fedora-coreos-tracker#1670 (comment)

@jbtrystram jbtrystram requested review from jlebon and c4rt0 April 5, 2024 09:10
jbtrystram added a commit to jbtrystram/fedora-coreos-config that referenced this pull request Apr 5, 2024
Update the scsci-id test to set a WWN for the disk, and use reliable
udev symlinks to adjust for a change in sg3_utils [1]

Note that the `wwn` value set is converted to base 16 by QEMU,
so the symlink in the ignition config must reflects it.

This requires coreos/coreos-assembler#3772
See coreos/fedora-coreos-tracker#1670

[1] https://listman.redhat.com/archives/dm-devel/2023-March/053645.html
@jbtrystram
Copy link
Contributor Author

jbtrystram commented Apr 5, 2024

it looks like I messed up something:

Edit: Ok, solved. The issue was format=raw for the non multipath disk

/srv/bin/kola qemuexec -D "5G:channel=scsi,wwn=0011" --qemu-image fcos41.qcow2 --add-ignition autologin
[....]
[core@localhost ~]$ lsblk
NAME   MAJ:MIN RM   SIZE RO TYPE MOUNTPOINTS
sda      8:0    0 192.5K  0 disk

Wheras with /srv/bin/kola qemuexec -D "5G" I get:

[core@localhost ~]$ lsblk
NAME   MAJ:MIN RM  SIZE RO TYPE MOUNTPOINTS
vda    252:0    0    5G  0 disk

jlebon
jlebon previously approved these changes Apr 5, 2024
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Some minor comments, but looks sane overall!
We should also update run.md.

mantle/platform/qemu.go Outdated Show resolved Hide resolved
mantle/platform/qemu.go Show resolved Hide resolved
Add a customizable WWN option for kola DiskSpec to have reliable links
under `/dev/disk/by-id/`. With this change kola qemuxec can be run like:
`kola qemuexec -D "5G:channel=scsi,wwn=007"`

Resulting in the following links:
```
[core@localhost ~]$ rpm -qa sg3_utils
sg3_utils-1.48-1.fc40.x86_64
[core@localhost ~]$ ls -l /dev/disk/by-id
total 0
lrwxrwxrwx. 1 root root  9 Apr  5 09:05 scsi-30000000000000007 -> ../../sda
lrwxrwxrwx. 1 root root  9 Apr  5 09:05 wwn-0x0000000000000007 -> ../../sda
```

This is motivated by recent changes in sg3_utils [1] which
removed some udev links.
At least one of our tests [2] relying on this started failing.
This patch was suggested by @jlebon [3]

[1] https://listman.redhat.com/archives/dm-devel/2023-March/053645.html
[2] coreos/fedora-coreos-tracker#1670
[3] coreos/fedora-coreos-tracker#1670 (comment)
Copy link
Member

@c4rt0 c4rt0 left a comment

Choose a reason for hiding this comment

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

I went through the changes implemented here, and all I can say is that I am saving this PR for future reference. I'm afraid I can't spot anything that looks odd to me.

/lgtm

@jbtrystram jbtrystram requested a review from jlebon April 10, 2024 07:20
@jlebon jlebon enabled auto-merge (rebase) April 10, 2024 14:50
@jlebon
Copy link
Member

jlebon commented Apr 10, 2024

/retest

1 similar comment
@jbtrystram
Copy link
Contributor Author

/retest

jbtrystram added a commit to jbtrystram/fedora-coreos-config that referenced this pull request Apr 11, 2024
Update the scsci-id test to set a WWN for the disk, and use reliable
udev symlinks to adjust for a change in sg3_utils [1]

Note that the `wwn` value set is converted to base 16 by QEMU,
so the symlink in the ignition config must reflects it.

This requires coreos/coreos-assembler#3772
See coreos/fedora-coreos-tracker#1670

[1] https://listman.redhat.com/archives/dm-devel/2023-March/053645.html
@jlebon jlebon merged commit 13a18f7 into coreos:main Apr 11, 2024
5 checks passed
jbtrystram added a commit to coreos/fedora-coreos-config that referenced this pull request Apr 12, 2024
Update the scsci-id test to set a WWN for the disk, and use reliable
udev symlinks to adjust for a change in sg3_utils [1]

Note that the `wwn` value set is converted to base 16 by QEMU,
so the symlink in the ignition config must reflects it.

This requires coreos/coreos-assembler#3772
See coreos/fedora-coreos-tracker#1670

[1] https://listman.redhat.com/archives/dm-devel/2023-March/053645.html
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