On 8/26/26 3:41 PM, Matthew Rosato wrote:
On 8/26/26 3:30 PM, Jared Rossi wrote:
On 8/26/26 1:33 PM, Matthew Rosato wrote:
+bool virtio_net_setup(void)
+{
+ switch (virtio_get_device()->ipl_type) {
+ case S390_IPL_TYPE_CCW:
+ return virtio_ccw_net_setup();
+ default:
+ return false;
+ }
+}
This patch is largely renaming, but this does seem to have a subtle
functional change right here.
AFAICT before this patch attempting to netboot with anything other than
a ccw device would hit
IPL_assert(iplb.pbt == S390_IPL_TYPE_CCW, "IPL_TYPE_CCW expected");
Now, we will never call virtio_ccw_net_setup(), return false and instead
bail out with
"No virtio net device found."
That is new behavior for !IPL_TYPE_CCW after this patch. For
IPL_TYPE_PCI, patch 4 will change the behavior again.
That's not a deal-breaker, but I do think it's worth a mention in the
commit message. I then also wonder if the message
"No virtio net device found"
would be more accurate if it instead read something like:
"No supported virtio net device found"
Thanks,
Matt
I’m not sure this is a valid concern. A non-ccw net device would be
rejected before getting to virtio_net_setup() earlier at the
find_boot_device() step either way. In the case of virtio-net-pci
specifically, it would fail because VIRTIO_ID_NET is not a supported PCI
type yet. For some sort of non-ccw non-pci netboot device, I believe there
wouldn't ever be an IPLB built for it, so it wouldn’t be recognized as
boot eligible at all. I don’t think this patch affects any of that.
Based on that description it sounds like we don't even ever to expect to
reach the new default: case then, as prior checks should have already
weeded out all but the supported IPL_TYPE_* values.
Should the default: case have an IPL_assert with its own message then?
Right, we shouldn’t ever hit the default case, but I don’t follow your
suggestion about an IPL_assert. What condition might be asserted there?
In my opinion simply returning false is appropriate, which will just print
the error and move on to the next boot device if there one. A full panic
could be justified since something weird would need to happen to get into
the default case at all, but I don’t think it’s really necessary.
If virtio_net_setup() does fail though, regardless of why, I do agree the
subsequent error message needs to be updated to clarify that no “valid” or
“supported” virtio network device was found rather than stating none was
found at all.
Regards,
Jared Rossi
