Patchwork [v4,15/18] target-i386: Set migratable=yes by default

login
register
mail settings
Submitter Eduardo Habkost
Date April 30, 2014, 4:48 p.m.
Message ID <1398876525-28831-16-git-send-email-ehabkost@redhat.com>
Download mbox | patch
Permalink /patch/344241/
State New
Headers show

Comments

Eduardo Habkost - April 30, 2014, 4:48 p.m.
Having only migratable flags reported by default on the "host" CPU model
is safer for the following reasons:

 * Existing users may expect "-cpu host" to be migration-safe, if they
   take care of always using compatible host CPUs, host kernels, and
   QEMU versions.
 * Users who don't care aboug migration and want to enable all features
   supported by the host kernel can simply change their setup to use
   migratable=no.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Andreas Färber - May 15, 2014, 8:07 p.m.
Am 30.04.2014 18:48, schrieb Eduardo Habkost:
> Having only migratable flags reported by default on the "host" CPU model
> is safer for the following reasons:
> 
>  * Existing users may expect "-cpu host" to be migration-safe, if they
>    take care of always using compatible host CPUs, host kernels, and
>    QEMU versions.
>  * Users who don't care aboug migration and want to enable all features

"about"

>    supported by the host kernel can simply change their setup to use
>    migratable=no.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

I see no Reviewed-by or Acked-by for this change...

Shouldn't we at least add .compat_props if we change this behavior?

(NB at this point there are no unmigratable flags yet, but in 18/18.)

Regards,
Andreas

> ---
>  target-i386/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 9c30957..9ef27fc 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1232,7 +1232,7 @@ static int cpu_x86_fill_model_id(char *str)
>  static X86CPUDefinition host_cpudef;
>  
>  static Property x86_host_cpu_properties[] = {
> -    DEFINE_PROP_BOOL("migratable", X86CPU, migratable, false),
> +    DEFINE_PROP_BOOL("migratable", X86CPU, migratable, true),
>      DEFINE_PROP_END_OF_LIST()
>  };
>
Eduardo Habkost - May 15, 2014, 8:22 p.m.
On Thu, May 15, 2014 at 10:07:12PM +0200, Andreas Färber wrote:
> Am 30.04.2014 18:48, schrieb Eduardo Habkost:
> > Having only migratable flags reported by default on the "host" CPU model
> > is safer for the following reasons:
> > 
> >  * Existing users may expect "-cpu host" to be migration-safe, if they
> >    take care of always using compatible host CPUs, host kernels, and
> >    QEMU versions.
> >  * Users who don't care aboug migration and want to enable all features
> 
> "about"

Oops.

> 
> >    supported by the host kernel can simply change their setup to use
> >    migratable=no.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> I see no Reviewed-by or Acked-by for this change...

Paolo? Marcelo? Can you help here?

> 
> Shouldn't we at least add .compat_props if we change this behavior?

I don't believe we need it. Machine-types are about ABI stability, and
"-cpu host" has no ABI stability unless you have exactly the same host
CPU, same QEMU version and the same kernel version.

> 
> (NB at this point there are no unmigratable flags yet, but in 18/18.)

Actually, there are unmigratable flags, already: the ones supported by
the KVM kernel module but are still unknown to QEMU.

> 
> Regards,
> Andreas
> 
> > ---
> >  target-i386/cpu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 9c30957..9ef27fc 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1232,7 +1232,7 @@ static int cpu_x86_fill_model_id(char *str)
> >  static X86CPUDefinition host_cpudef;
> >  
> >  static Property x86_host_cpu_properties[] = {
> > -    DEFINE_PROP_BOOL("migratable", X86CPU, migratable, false),
> > +    DEFINE_PROP_BOOL("migratable", X86CPU, migratable, true),
> >      DEFINE_PROP_END_OF_LIST()
> >  };
> >  
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Marcelo Tosatti - May 16, 2014, 11:14 a.m.
On Wed, Apr 30, 2014 at 01:48:42PM -0300, Eduardo Habkost wrote:
> Having only migratable flags reported by default on the "host" CPU model
> is safer for the following reasons:
> 
>  * Existing users may expect "-cpu host" to be migration-safe, if they
>    take care of always using compatible host CPUs, host kernels, and
>    QEMU versions.
>  * Users who don't care aboug migration and want to enable all features
>    supported by the host kernel can simply change their setup to use
>    migratable=no.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 9c30957..9ef27fc 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1232,7 +1232,7 @@ static int cpu_x86_fill_model_id(char *str)
>  static X86CPUDefinition host_cpudef;
>  
>  static Property x86_host_cpu_properties[] = {
> -    DEFINE_PROP_BOOL("migratable", X86CPU, migratable, false),
> +    DEFINE_PROP_BOOL("migratable", X86CPU, migratable, true),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> -- 
> 1.9.0

Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>

Patch

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9c30957..9ef27fc 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1232,7 +1232,7 @@  static int cpu_x86_fill_model_id(char *str)
 static X86CPUDefinition host_cpudef;
 
 static Property x86_host_cpu_properties[] = {
-    DEFINE_PROP_BOOL("migratable", X86CPU, migratable, false),
+    DEFINE_PROP_BOOL("migratable", X86CPU, migratable, true),
     DEFINE_PROP_END_OF_LIST()
 };