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.
Hi Robie, /github. com/rhboot/ efivar/ commit/ ff696a432bf92af 1206e235a60ad28 b58ff0ab92 commentary:
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:/
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/
= 0001-Fix- the-error- path-in- set_disk_ and_part_ name.patch = 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.
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_
= 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 = nvme-subsystem parsing code that is required to solve this issue.
This adds the virtual/
= 0005-Fix- variable- sz-uninitialize d-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.