Re: [Qemu-ppc] [PULL 35/40] spapr: extend the sPAPR IRQ backend for XICS migration

[Cédric Le Goater]

On 1/3/19 8:07 PM, Peter Maydell wrote:
> On Fri, 21 Dec 2018 at 05:46, David Gibson <address@hidden> wrote:
>>
>> From: Cédric Le Goater <address@hidden>
>>
>> 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 <address@hidden>
>> Reviewed-by: David Gibson <address@hidden>
>> Signed-off-by: David Gibson <address@hidden>
> 
>> @@ -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
>