On 28/11/2016 18:12, Eduardo Habkost wrote: >>> > > >>> > > Why not use src_use_reliable_get_clock here, too? (Maybe rename >>> > > it to simply clock_is_reliable?) >> > >> > Because you end up mixing two mental ideas (one: whether the source >> > has KVM_GET_CLOCK, second: whether the destination has KVM_GET_CLOCK >> > into one variable). I find it more confusing than not. >> > Maybe its just my limited brain but i find it >> > confusing. > I find it simpler and easier to reason about. Me too---note that it's not "whether X had reliable KVM_GET_CLOCK" but rather "whether the last read came from a reliable KVM_GET_CLOCK". However... >>> > > You can change kvm_get_clock() to kvm_get_clock(KVMClockState*), >>> > > and make it update both s->clock and s->clock_is_reliable. With >>> > > this, s->clock and s->clock_is_reliable will be always in sync, >>> > > and have a single code path for both local restore and migration: >> > >> > There should not be a single path for migration/runtime cases: >> > on migration, we care whether the source used reliable KVM_GET_CLOCK. >> > >> > On runtime, we care whether the destination uses reliable KVM_GET_CLOCK. > The conditions may be different, but both can be translated to a > "should we trust the value in s->clock" flag, to simplify the > code (just like Paolo's suggestion on his reply to v1 today). ... I think I understand now: Marcelo is approaching the problem differently. For him it's not an issue of "should we trust the value" but rather it's "is the value going to mess up the guest with backwards time". Which is fine, it just requires some more comments because it's subtle and it looks a lot like the kernel API is being misused (while it's fine). Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
