On 27.02.2017 12:51, Cornelia Huck wrote:
> On Sat, 25 Feb 2017 07:18:29 +0100
> Thomas Huth <th...@redhat.com> wrote:
>
>> On 23.02.2017 13:20, Cornelia Huck wrote:
>>> From: Farhan Ali <al...@linux.vnet.ibm.com>
>>>
>>> Load the network boot image into guest RAM when the boot
>>> device selected is a network device. Use some of the reserved
>>> space in IplBlockCcw to store the start address of the netboot
>>> image.
>>>
>>> A user could also use 'chreipl'(diag 308/5) to change the boot device.
>>> So every time we update the IPLB, we need to verify if the selected
>>> boot device is a network device so we can appropriately load the
>>> network boot image.
>>>
>>> Signed-off-by: Farhan Ali <al...@linux.vnet.ibm.com>
>>> Signed-off-by: Cornelia Huck <cornelia.h...@de.ibm.com>
>>> ---
>>> hw/s390x/ipl.c | 89
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> hw/s390x/ipl.h | 4 ++-
>>> 2 files changed, 92 insertions(+), 1 deletion(-)
[...]
>>> + if (ipl->netboot) {
>>> + if (load_netboot_image(&err) < 0) {
>>> + error_report_err(err);
>>> + vm_stop(RUN_STATE_INTERNAL_ERROR);
>>> + }
>>> + ipl->iplb.ccw.netboot_start_addr = ipl->start_addr;
>>
>> Not sure whether it matters, but in case of early errors during
>> load_netboot_image(), ipl->start_addr could be used uninitialized here.
>> Maybe you should move the "ipl->start_addr = KERN_IMAGE_START;" there at
>> the beginning of the function, to make it also the default value for the
>> other error cases?
>
> ipl->start_addr has already been set to some value in the realize
> function (either the kernel entry address, or the bios address).
>
> But that should not matter with the vm_stop() on error anyway, no?
Likely not. It's just a little bit strange to see that the program flow
continues after the error in this function here ... maybe it would have
been clearer to put a "return" right after the "vm_stop()"? Anyway, it
likely does not really matter here, so never mind.
Thomas
