On 19/07/2021 13:46, Jan Beulich wrote:
> On 19.07.2021 13:14, Andrew Cooper wrote:
>> hvm_load() is currently a mix of -errno and -1 style error handling, which
>> aliases -EPERM. This leads to the following confusing diagnostics:
>>
>> From userspace:
>> xc: info: Restoring domain
>> xc: error: Unable to restore HVM context (1 = Operation not permitted):
>> Internal error
>> xc: error: Restore failed (1 = Operation not permitted): Internal error
>> xc_domain_restore: [1] Restore failed (1 = Operation not permitted)
>>
>> From Xen:
>> (XEN) HVM10.0 restore: inconsistent xsave state (feat=0x2ff accum=0x21f
>> xcr0=0x7 bv=0x3 err=-22)
>> (XEN) HVM10 restore: failed to load entry 16/0
>>
>> The actual error was a bad backport, but the -EINVAL got converted to -EPERM
>> on the way out of the hypercall.
>>
>> The overwhelming majority of *_load() handlers already use -errno
>> consistenty.
>> Fix up the rest to be consistent, and fix a few other errors noticed along
>> the
>> way.
>>
>> * Failures of hvm_load_entry() indicate a truncated record or other bad data
>> size. Use -ENODATA.
> But then ...
>
>> @@ -421,8 +421,8 @@ static int pit_load(struct domain *d,
>> hvm_domain_context_t *h)
>>
>> if ( hvm_load_entry(PIT, h, &pit->hw) )
>> {
>> - spin_unlock(&pit->lock);
>> - return 1;
>> + rc = -ENODEV;
>> + goto out;
>> }
> ... use -ENODATA here as well?
Hmm - that was intended to be ENODATA. Will fix.
> Preferably with the adjustment
> Reviewed-by: Jan Beulich <[email protected]>
Thanks,
> I'll pick this up for backporting once I see it in the tree.
I don't know how much the call tree has changed over time. Every
handler will need a quick check on each release.
~Andrew
