Re: [Qemu-devel] [PATCH 08/20] target-i386: compile kvm only functions if CONFIG_KVM is defined

[Igor Mammedov <imammedo@redhat.com>]

On Wed, 19 Dec 2012 14:42:31 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Mon, Dec 17, 2012 at 05:01:20PM +0100, Igor Mammedov wrote:
> [...]
> >  
> >  static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void
> > *opaque, const char *name, Error **errp)
> > @@ -1273,7 +1271,9 @@ static int cpu_x86_find_by_name(x86_def_t
> > *x86_cpu_def, const char *name) }
> >      }
> >      if (kvm_enabled() && name && strcmp(name, "host") == 0) {
> > +#ifdef CONFIG_KVM
> >          kvm_cpu_fill_host(x86_cpu_def);
> > +#endif
> 
> Is this really better than the existing code that generates an empty
> stub function (that will never be called anyway)?
Following patch that moves kvm_check_features_against_host() inside
of #ifdef CONFIG_KVM in realize_fn(), makes build failure *-user with
warning that kvm_check_features_against_host() is unused and if
we make stub from kvm_check_features_against_host() as well then we will have
to ifdef unavailable_host_feature() as well. As result it makes +2 more
ifdefs one of which excludes whole function.

This patch makes one big ifdef block of kvm specific functions and the next
one keep amount of ifdef the same as before these patches.

And as bonus we get cleaner build and won't get unused symbols & calls to
empty functions on debug builds.

> 
> I am not strongly inclined either way, but I prefer the existing style.
> 
> 
> >      } else if (!def) {
> >          return -1;
> >      } else {
> > @@ -1428,10 +1428,12 @@ static int cpu_x86_parse_featurestr(x86_def_t
> > *x86_cpu_def, char *features) x86_cpu_def->kvm_features &=
> > ~minus_kvm_features; x86_cpu_def->svm_features &= ~minus_svm_features;
> >      x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_7_0_ebx_features;
> > +#ifdef CONFIG_KVM
> >      if (check_cpuid && kvm_enabled()) {
> >          if (kvm_check_features_against_host(x86_cpu_def) &&
> > enforce_cpuid) goto error;
> >      }
> > +#endif
> >      return 0;
> >  
> >  error:
> > -- 
> > 1.7.1
> > 
> > 
>