On Fri, 5 Apr 2019 15:26:24 +0200 Thomas Huth <th...@redhat.com> wrote:
> On 05/04/2019 15.11, Jason J. Herne wrote: > > Your analysis of the problem matches what I'm seeing as well. Here is > > what I'm proposing to fix it. If you like it, let me know if you want me > > to re-send just the final patch, or the entire series again with a v7 tag. If you (Jason) resend just that patch, I'm happy to give it a tag :) > > > > > > diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c > > index 03e90e3..3d1e9a4 100644 > > --- a/pc-bios/s390-ccw/main.c > > +++ b/pc-bios/s390-ccw/main.c > > @@ -67,6 +67,7 @@ static bool find_subch(int dev_no) > > { > > Schib schib; > > int i, r; > > + bool is_virtio; > > > > for (i = 0; i < 0x10000; i++) { > > blk_schid.sch_no = i; > > @@ -80,12 +81,13 @@ static bool find_subch(int dev_no) > > > > enable_subchannel(blk_schid); > > cutype = cu_type(blk_schid); > > + is_virtio = virtio_is_supported(blk_schid); > > > > /* No specific devno given, just return 1st possibly bootable > > device */ > > if (dev_no < 0) { > > switch (cutype) { > > case CU_TYPE_VIRTIO: > > - if (virtio_is_supported(blk_schid)) { > > + if (is_virtio) { > > /* > > * Skip net devices since no IPLB is created and > > therefore > > * no network bootloader has been loaded > > > > > > Looks fine to me. But maybe we should also add a comment before the new > "virtio_is_supported" line, saying something like "Note: we always have > to run virtio_is_supported() here to make sure that the vdev.senseid > data gets pre-initialized"? Otherwise, we might accidentally "revert" > this patch when somebody thinks that the code might be optimizable by > removing the is_virtio variable again... Agreed. > > If you like, I can squash these changes in when I pick up the patches, > so you don't have to resend. > > Thomas >