On Wed, 6 Mar 2019 09:55:40 -0500 "Jason J. Herne" <jjhe...@linux.ibm.com> wrote:
> On 3/4/19 8:40 AM, Cornelia Huck wrote: > > On Fri, 1 Mar 2019 13:59:21 -0500 > > "Jason J. Herne" <jjhe...@linux.ibm.com> wrote: > >> @@ -532,7 +559,7 @@ void s390_ipl_reset_request(CPUState *cs, enum > >> s390_reset reset_type) > >> !ipl->netboot && > >> ipl->iplb.pbt == S390_IPL_TYPE_CCW && > >> is_virtio_scsi_device(&ipl->iplb)) { > >> - CcwDevice *ccw_dev = s390_get_ccw_device(get_boot_device(0)); > >> + CcwDevice *ccw_dev = s390_get_ccw_device(get_boot_device(0), > >> &devtype); > > > > It feels wrong to have a variable that is not used later... what about > > either > > - making s390_get_ccw_device() capable of handling a NULL second > > parameter, or > > - (what I think would be nicer) passing in the devtype as an optional > > parameter to gen_initial_iplb() so you don't need to do the casting > > dance twice? > > > > I'm with you on everything suggested for this patch except this last item. > I'm not sure > what you are suggesting here. I can easily visualize passing NULL for devtype > when we want > to ignore it but I'm not sure what you mean by 'optional parameter' Hm, actually you'd need the device as well :) Basically, static bool s390_gen_initial_iplb(S390IPLState *ipl, CcwDevice *ccw_dev, int devtype) { (...) if (!ccw_dev) { //get ccw_dev, which also gives us the devtype } if (ccw_dev) { //as before; devtype is valid here (...) return true; } return false; } So you can call it with s390_gen_initial_iplb(ipl, NULL, 0) if you don't have the values already.