On Tue, 2022-03-08 at 18:25 +0100, Paolo Bonzini wrote: > On 3/8/22 18:20, David Woodhouse wrote: > > > Yes, I'm just talking about the second hunk. The first is clear(ish). > > Oh, I see. > > > > When the oneshot timer has expired and hasn't been re-armed by the > > guest, we should return zero as 'expires_ns' so that it doesn't get re- > > armed in the past (and, hopefully, immediately retriggered) when the > > guest is restored. > > > > Also, we don't really want the timer firing*after* the guest vCPU > > state has been serialized, since the newly-injected interrupt might not > > get migrated. Hence using hrtimer_cancel() as our check for whether > > it's still pending or not. > > > > I think the two are different. The first is also clear, and that should > be fixed with a separate bool or possibly a special meaning for > expires_ns == -1 (or INT64_MAX, I don't speak Xen hypercalls :)). The value you're looking for is zero; that's what means "not set". The issue wasn't the Xen ABI or even the KVM_XEN_VCPU_ATTR ABI; it's just that we weren't *returning* zero when the timer has already fired and thus is it isn't considered to be 'set' any more. > The second should not be a problem. The newly-injected interrupt might > not get migrated, but it will be injected on the destination. So it > shouldn't be a problem, in fact anything that relies on that is probably > going to be racy. Sure. But if we consider it acceptable for the timer to fire again after live migration when it already fired on the source host why did we even bother fixing the first part above? :)
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
