On Wed, Dec 19, 2012 at 06:16:08PM +0100, Igor Mammedov wrote: > On Wed, 19 Dec 2012 14:42:31 -0200 > Eduardo Habkost <ehabk...@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.
I see. So my question above doesn't apply anymore once patch 09/20 is applied (and the rest of the changes in this patch make sense). I think you could just squash both patches together, and it should be obvious that you are adding/moving #ifdefs around the functions just because the function call is now inside an #ifdef. > > 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 > > > > > > > > > -- Eduardo