Hi Robie, Thank you for the review. I don't believe a surgical fix is practical here. I did look to see if we could somehow back out the code that introduced the regression. That was introduced upstream between versions v35 & v36 in this commit: https://github.com/rhboot/efivar/commit/ff696a432bf92af1206e235a60ad28b58ff0ab92 That was a significant refactoring itself and taking a v37-based-package back to the "old way" didn't seem feasible. So I think we're stuck evolving the new code to deal with these devices, and I don't have an alternate solution to following what upstream has done. Here's some patch-by-patch justification/commentary:
= 0001-Fix-the-error-path-in-set_disk_and_part_name.patch = TBH, this is one patch that could be dropped, and I could refactor later patches to leave the old behavior. It fixes an obvious bug - that is, even if set_disk_and_part_name() fails to parse the device, it will always return 0. The calling code in device_get() checks for non-zero before proceeding to dereference strings that set_disk_and_part_name() would only set if it *actually* succeeded. Since technically this bug is unrelated to what I'm fixing here, I'm open to dropping it if you prefer or, alternatively, I could file a new bug and update the patch comments to make it's justification appear less arbitrary. = 0002-sysfs-parsers-make-all-the-sys-block-link-parsers-wo.patch = While the upstream version of the patch I backported does change all parsers, I restricted the changes to the NVMe parser to reduce regression risk. This patch is part of the evolution to add support for nvme-subsys devices, and I think trying to avoid it would be riskier than including it. = 0003-Try-even-harder-to-find-disk-device-symlinks-in-sysf.patch = The refactoring here is introduced to change sysfs parsing behavior, where NVMe devices are the given example in the commit log/comments. I don't see a practical way of avoiding it. = 0004-Handle-sys-devices-virtual-nvme-fabrics-nvme-subsyst.patch = This adds the virtual/nvme-subsystem parsing code that is required to solve this issue. = 0005-Fix-variable-sz-uninitialized-error.patch = A bugfix for the code introduced above. = 0006-Fix-parsing-for-nvme-subsystem-devices.patch = Another bugfix that makes the nvme-subsystem code actually do what the existing comments assert it should do. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1891718 Title: [Regression] breaks GRUB install on an nvme device To manage notifications about this bug go to: https://bugs.launchpad.net/efivar/+bug/1891718/+subscriptions -- ubuntu-bugs mailing list [email protected] https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
