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

nixos/nixos-generate-config: include new device ID for virtio_scsi #123357

Merged
merged 1 commit into from Apr 19, 2023

Conversation

r-vdp
Copy link
Contributor

@r-vdp r-vdp commented May 17, 2021

Motivation for this change

nixos-generate-config is currently not correctly detecting some virtio_scsi devices, causing systems to be rendered unbootable. This change includes an additional vendor ID to make sure that the presence of VirtIO devices is detected correctly.

Fixes #76980.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@r-vdp
Copy link
Contributor Author

r-vdp commented May 17, 2021

@FRidh: I see that you merged the last PR that modified these vendor IDs. Would you mind reviewing this change?

Thanks!

@dminuoso
Copy link
Contributor

Hi, I'm the original author for that code.

I just realized we should just look at the VirtIO specification and see what valid Device IDs might be generated.

https://docs.oasis-open.org/virtio/virtio/v1.1/virtio-v1.1.html
4.1.2 PCI Device Discovery

Any PCI device with PCI Vendor ID 0x1AF4, and **PCI Device ID 0x1000 through 0x107F** inclusive is a virtio device.

Emphasis added by me. Let's future proof this and check for all possible Device IDs?

@r-vdp
Copy link
Contributor Author

r-vdp commented Jan 31, 2022

@dminuoso I'd really like to fix this, but I'm completely ignorant when it comes to Perl.

How would you test this code? Do you happen to know which packages to install with nix-shell to be able to run this code?
Also, I'd like to use List::Util::first to search for the device ID in the range 0x1000..0x107F, is that module a dependency that we already have? Or should I do something specific for this?

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 30, 2022
@RaitoBezarius
Copy link
Member

Can I do something to help getting the "semantic" version of this merged?

@delroth
Copy link
Contributor

delroth commented Apr 18, 2023

Emphasis added by me. Let's future proof this and check for all possible Device IDs?

Semantically this seems wrong to me -- we're not looking for "any virtio device", we're looking for VirtIO SCSI hosts (virtio_scsi module). The documentation you linked clearly says that's 0x1040 (base) + 8 (SCSI host device ID) = 0x1048, or 0x1004 for transitional. My reading of the docs is that no other value should be used for a virtio scsi host.

I found this PR because someone told me today they hit this problem on a Hetzner cloud instance. This seems like a footgun we should get rid of. Any objection to merging this as is, given what I've mentioned in the previous paragraph?

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 18, 2023
@lukegb lukegb changed the title nixos/nixos-generate-config: include new vendor ID for virtio_scsi nixos/nixos-generate-config: include new device ID for virtio_scsi Apr 18, 2023
@github-actions
Copy link
Contributor

Successfully created backport PR for release-22.11:

@r-vdp r-vdp deleted the virtio_scsi_vendor branch April 20, 2023 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VirtIO SCSI kernel module not detected by nixos-generate-config
5 participants