css_clear_io_interrupt() aborts on unexpected ioctl() errors, and I
wonder whether that's appropriate. Let's have a closer look:
static void css_clear_io_interrupt(uint16_t subchannel_id,
uint16_t subchannel_nr)
{
Error *err = NULL;
static bool no_clear_irq;
S390FLICState *fs = s390_get_flic();
S390FLICStateClass *fsc = s390_get_flic_class(fs);
int r;
if (unlikely(no_clear_irq)) {
return;
}
r = fsc->clear_io_irq(fs, subchannel_id, subchannel_nr);
switch (r) {
case 0:
break;
case -ENOSYS:
no_clear_irq = true;
/*
* Ignore unavailability, as the user can't do anything
* about it anyway.
*/
break;
default:
error_setg_errno(&err, -r, "unexpected error condition");
error_propagate(&error_abort, err);
}
}
The default case is abort() with a liberal amount of lipstick applied.
Let's ignore the lipstick and focus on the abort().
fsc->clear_io_irq ist either qemu_s390_clear_io_flic() order
kvm_s390_clear_io_flic().
Only kvm_s390_clear_io_flic() can return non-zero: -errno when ioctl()
fails.
The ioctl() is KVM_SET_DEVICE_ATTR for KVM_DEV_FLIC_CLEAR_IO_IRQ with
subchannel_id and subchannel_nr. I.e. we assume that this can only fail
with ENOSYS, und crash hard when the assumption turns out to be wrong.
Is this error condition a programming error? I figure it can be one
only if the ioctl()'s contract promises us it cannot fail in any other
way unless we violate preconditions.
Is the error condition fatal, i.e. continuing would be unsafe?
If it's a fatal programming error, then abort() is appropriate.
If it's fatal, but not a programming error, we should exit(1) instead.
If it's a survivable programming error, use of abort() is a matter of
taste.
