On 1/3/19 8:07 PM, Peter Maydell wrote: > On Fri, 21 Dec 2018 at 05:46, David Gibson <da...@gibson.dropbear.id.au> > wrote: >> >> From: Cédric Le Goater <c...@kaod.org> >> >> Introduce a new sPAPR IRQ handler to handle resend after migration >> when the machine is using a KVM XICS interrupt controller model. >> >> Signed-off-by: Cédric Le Goater <c...@kaod.org> >> Reviewed-by: David Gibson <da...@gibson.dropbear.id.au> >> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > >> @@ -1758,6 +1750,11 @@ static int spapr_post_load(void *opaque, int >> version_id) >> } >> } >> >> + err = spapr_irq_post_load(spapr, version_id); >> + if (err) { >> + return err; >> + } >> + > > Hi; this change causes Coverity to complain (CID 1398591) that > we're now overwriting the setting of err in the earlier > err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset); > without ever testing it. (We also do this in the existing > codepath that calls kvmppc_configure_v3_mmu().)
yes. I suppose we have been missing something like : @@ -1754,6 +1754,9 @@ static int spapr_post_load(void *opaque, * value into the RTC device */ if (version_id < 3) { err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset); + if (err) { + return err; + } } if (kvm_enabled() && spapr->patb_entry) { C. > > Should the call to spapr_rtc_import_offset() have its > own "if (err) do something" code, or should it simply > be ignoring its return value entirely, or something > more complicated ? > > thanks > -- PMM >