[Qemu-devel] Re: [PATCH v2 1/2] Do not register kvmclock savevm section if kvmclock is disabled.
On Wed, 2010-12-08 at 17:31 -0200, Marcelo Tosatti wrote: On Tue, Dec 07, 2010 at 03:12:36PM -0200, Glauber Costa wrote: On Mon, 2010-12-06 at 19:04 -0200, Marcelo Tosatti wrote: On Mon, Dec 06, 2010 at 09:03:46AM -0500, Glauber Costa wrote: Usually nobody usually thinks about that scenario (me included and specially), but kvmclock can be actually disabled in the host. It happens in two scenarios: 1. host too old. 2. we passed -kvmclock to our -cpu parameter. In both cases, we should not register kvmclock savevm section. This patch achives that by registering this section only if kvmclock is actually currently enabled in cpuid. The only caveat is that we have to register the savevm section a little bit later, since we won't know the final kvmclock state before cpuid gets parsed. What is the problem of registering the section? Restoring the value if the host does not support it returns an error? Can't you ignore the error if kvmclock is not reported in cpuid, in the restore handler? We can change the restore handler, but not the restore handler of binaries that are already out there. The motivation here is precisely to address migration to hosts without kvmclock, so it's better to have a way to disable, than to count on the fact that the other side will be able to ignore it. OK. Can't you register conditionally on kvmclock cpuid bit at the end of kvm_arch_init_vcpu, in target-i386/kvm.c? Haven't looked at it, but will today. Actually, tsc has (obviously) the same problem and I plan to respin the patch today including a fix for it as well. Thanks!
[Qemu-devel] Re: [PATCH v2 1/2] Do not register kvmclock savevm section if kvmclock is disabled.
On Tue, Dec 07, 2010 at 03:12:36PM -0200, Glauber Costa wrote: On Mon, 2010-12-06 at 19:04 -0200, Marcelo Tosatti wrote: On Mon, Dec 06, 2010 at 09:03:46AM -0500, Glauber Costa wrote: Usually nobody usually thinks about that scenario (me included and specially), but kvmclock can be actually disabled in the host. It happens in two scenarios: 1. host too old. 2. we passed -kvmclock to our -cpu parameter. In both cases, we should not register kvmclock savevm section. This patch achives that by registering this section only if kvmclock is actually currently enabled in cpuid. The only caveat is that we have to register the savevm section a little bit later, since we won't know the final kvmclock state before cpuid gets parsed. What is the problem of registering the section? Restoring the value if the host does not support it returns an error? Can't you ignore the error if kvmclock is not reported in cpuid, in the restore handler? We can change the restore handler, but not the restore handler of binaries that are already out there. The motivation here is precisely to address migration to hosts without kvmclock, so it's better to have a way to disable, than to count on the fact that the other side will be able to ignore it. OK. Can't you register conditionally on kvmclock cpuid bit at the end of kvm_arch_init_vcpu, in target-i386/kvm.c?
[Qemu-devel] Re: [PATCH v2 1/2] Do not register kvmclock savevm section if kvmclock is disabled.
On Mon, 2010-12-06 at 19:04 -0200, Marcelo Tosatti wrote: On Mon, Dec 06, 2010 at 09:03:46AM -0500, Glauber Costa wrote: Usually nobody usually thinks about that scenario (me included and specially), but kvmclock can be actually disabled in the host. It happens in two scenarios: 1. host too old. 2. we passed -kvmclock to our -cpu parameter. In both cases, we should not register kvmclock savevm section. This patch achives that by registering this section only if kvmclock is actually currently enabled in cpuid. The only caveat is that we have to register the savevm section a little bit later, since we won't know the final kvmclock state before cpuid gets parsed. What is the problem of registering the section? Restoring the value if the host does not support it returns an error? Can't you ignore the error if kvmclock is not reported in cpuid, in the restore handler? We can change the restore handler, but not the restore handler of binaries that are already out there. The motivation here is precisely to address migration to hosts without kvmclock, so it's better to have a way to disable, than to count on the fact that the other side will be able to ignore it.
[Qemu-devel] Re: [PATCH v2 1/2] Do not register kvmclock savevm section if kvmclock is disabled.
On Mon, Dec 06, 2010 at 09:03:46AM -0500, Glauber Costa wrote: Usually nobody usually thinks about that scenario (me included and specially), but kvmclock can be actually disabled in the host. It happens in two scenarios: 1. host too old. 2. we passed -kvmclock to our -cpu parameter. In both cases, we should not register kvmclock savevm section. This patch achives that by registering this section only if kvmclock is actually currently enabled in cpuid. The only caveat is that we have to register the savevm section a little bit later, since we won't know the final kvmclock state before cpuid gets parsed. What is the problem of registering the section? Restoring the value if the host does not support it returns an error? Can't you ignore the error if kvmclock is not reported in cpuid, in the restore handler? Also, please generate against uq/master.
[Qemu-devel] Re: [PATCH v2 1/2] Do not register kvmclock savevm section if kvmclock is disabled.
On Mon, Dec 06, 2010 at 07:04:01PM -0200, Marcelo Tosatti wrote: On Mon, Dec 06, 2010 at 09:03:46AM -0500, Glauber Costa wrote: Usually nobody usually thinks about that scenario (me included and specially), but kvmclock can be actually disabled in the host. It happens in two scenarios: 1. host too old. 2. we passed -kvmclock to our -cpu parameter. In both cases, we should not register kvmclock savevm section. This patch achives that by registering this section only if kvmclock is actually currently enabled in cpuid. The only caveat is that we have to register the savevm section a little bit later, since we won't know the final kvmclock state before cpuid gets parsed. What is the problem of registering the section? Restoring the value if the host does not support it returns an error? Can't you ignore the error if kvmclock is not reported in cpuid, in the restore handler? Also, please generate against uq/master. Sorry, this is not upstream yet, so ignore the uq/master part.
