Re: [Xen-devel] [PATCH v4 01/34] compat/x86: Remove unncessary #define.

[Konrad Rzeszutek Wilk]

On Wed, Mar 16, 2016 at 05:08:30AM -0600, Jan Beulich wrote:
> >>> On 15.03.16 at 18:56,  wrote:
> > It is not used.
> 
> Consistently please - either keep them all (just to cover the case
> that they might get used) or remove them all: xen_compile_info,
> xen_changeset_info, etc are all unused too. Otoh
> xennmi_callback is used, but xennmi_callback_t isn't. Which to me
> suggests that we should leave this alone.

Oddly enough taking an cleaver to it was OK.

From 7e3ed6faed6e083f27ad6be947ac528c3eaba9a1 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk 
Date: Wed, 2 Mar 2016 12:50:32 -0500
Subject: [PATCH v4 02/35] compat/x86: Remove unncessary #defines.

They are not used.

Signed-off-by: Konrad Rzeszutek Wilk 
Reviewed-by: Andrew Cooper 

---
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Keir Fraser 
Cc: Tim Deegan 

v2: Remove a lot more of them.
---
---
 xen/common/compat/kernel.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/xen/common/compat/kernel.c b/xen/common/compat/kernel.c
index df93fdd..dc898ae 100644
--- a/xen/common/compat/kernel.c
+++ b/xen/common/compat/kernel.c
@@ -18,30 +18,22 @@ asm(".file \"" __FILE__ "\"");
 
 extern xen_commandline_t saved_cmdline;
 
-#define xen_extraversion compat_extraversion
 #define xen_extraversion_t compat_extraversion_t
 
-#define xen_compile_info compat_compile_info
 #define xen_compile_info_t compat_compile_info_t
 
 CHECK_TYPE(capabilities_info);
 
-#define xen_platform_parameters compat_platform_parameters
 #define xen_platform_parameters_t compat_platform_parameters_t
 #undef HYPERVISOR_VIRT_START
 #define HYPERVISOR_VIRT_START HYPERVISOR_COMPAT_VIRT_START(current->domain)
 
-#define xen_changeset_info compat_changeset_info
 #define xen_changeset_info_t compat_changeset_info_t
 
-#define xen_feature_info compat_feature_info
 #define xen_feature_info_t compat_feature_info_t
 
 CHECK_TYPE(domain_handle);
 
-#define xennmi_callback compat_nmi_callback
-#define xennmi_callback_t compat_nmi_callback_t
-
 #ifdef COMPAT_VM_ASSIST_VALID
 #undef VM_ASSIST_VALID
 #define VM_ASSIST_VALID COMPAT_VM_ASSIST_VALID
-- 
2.5.0


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 01/34] compat/x86: Remove unncessary #define.

[Jan Beulich]

>>> On 17.03.16 at 01:44,  wrote:
> On Wed, Mar 16, 2016 at 05:08:30AM -0600, Jan Beulich wrote:
>> >>> On 15.03.16 at 18:56,  wrote:
>> > It is not used.
>> 
>> Consistently please - either keep them all (just to cover the case
>> that they might get used) or remove them all: xen_compile_info,
>> xen_changeset_info, etc are all unused too. Otoh
>> xennmi_callback is used, but xennmi_callback_t isn't. Which to me
>> suggests that we should leave this alone.
> 
> Oddly enough taking an cleaver to it was OK.
> 
> From 7e3ed6faed6e083f27ad6be947ac528c3eaba9a1 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk 
> Date: Wed, 2 Mar 2016 12:50:32 -0500
> Subject: [PATCH v4 02/35] compat/x86: Remove unncessary #defines.
> 
> They are not used.

This now goes too far.

> Signed-off-by: Konrad Rzeszutek Wilk 
> Reviewed-by: Andrew Cooper 

Hence this can't stay.

> ---
> Cc: Ian Jackson 
> Cc: Jan Beulich 
> Cc: Keir Fraser 
> Cc: Tim Deegan 
> 
> v2: Remove a lot more of them.

(Side note: Not only this time round I notice versioning mixup in
your patches: The subject says v4 here, yet the update to it
above says v2. In the previous round of the xSplice series - v3 -
I seem to recall there were patches showing a "history" up to
something like v9. I think in cases of such heavy mixing it should
be the patch with the oldest history that determines the version
of the entire series.)

> --- a/xen/common/compat/kernel.c
> +++ b/xen/common/compat/kernel.c
> @@ -18,30 +18,22 @@ asm(".file \"" __FILE__ "\"");
>  
>  extern xen_commandline_t saved_cmdline;
>  
> -#define xen_extraversion compat_extraversion
>  #define xen_extraversion_t compat_extraversion_t
>  
> -#define xen_compile_info compat_compile_info
>  #define xen_compile_info_t compat_compile_info_t
>  
>  CHECK_TYPE(capabilities_info);
>  
> -#define xen_platform_parameters compat_platform_parameters
>  #define xen_platform_parameters_t compat_platform_parameters_t
>  #undef HYPERVISOR_VIRT_START
>  #define HYPERVISOR_VIRT_START HYPERVISOR_COMPAT_VIRT_START(current->domain)
>  
> -#define xen_changeset_info compat_changeset_info
>  #define xen_changeset_info_t compat_changeset_info_t
>  
> -#define xen_feature_info compat_feature_info
>  #define xen_feature_info_t compat_feature_info_t
>  
>  CHECK_TYPE(domain_handle);
>  
> -#define xennmi_callback compat_nmi_callback
> -#define xennmi_callback_t compat_nmi_callback_t

The former definitely is being used; not getting a compilation
error here is not a sign of things being right. That's another
reason why - as I had suggested already - we'd probably
better leave things as they are: Introduction of uses of the
now removed identifiers might otherwise break compat code
without anyone noticing right away.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 01/34] compat/x86: Remove unncessary #define.

[Jan Beulich]

>>> On 15.03.16 at 18:56,  wrote:
> It is not used.

Consistently please - either keep them all (just to cover the case
that they might get used) or remove them all: xen_compile_info,
xen_changeset_info, etc are all unused too. Otoh
xennmi_callback is used, but xennmi_callback_t isn't. Which to me
suggests that we should leave this alone.

Jan

> --- a/xen/common/compat/kernel.c
> +++ b/xen/common/compat/kernel.c
> @@ -18,7 +18,6 @@ asm(".file \"" __FILE__ "\"");
>  
>  extern xen_commandline_t saved_cmdline;
>  
> -#define xen_extraversion compat_extraversion
>  #define xen_extraversion_t compat_extraversion_t
>  
>  #define xen_compile_info compat_compile_info
> -- 
> 2.5.0




___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 01/34] compat/x86: Remove unncessary #define.

[Andrew Cooper]

On 15/03/16 17:56, Konrad Rzeszutek Wilk wrote:
> It is not used.
>
> Signed-off-by: Konrad Rzeszutek Wilk 

Reviewed-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 01/34] compat/x86: Remove unncessary #define.

[Konrad Rzeszutek Wilk]

It is not used.

Signed-off-by: Konrad Rzeszutek Wilk 

---
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Keir Fraser 
Cc: Tim Deegan 
---
---
 xen/common/compat/kernel.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/xen/common/compat/kernel.c b/xen/common/compat/kernel.c
index df93fdd..e1b9013 100644
--- a/xen/common/compat/kernel.c
+++ b/xen/common/compat/kernel.c
@@ -18,7 +18,6 @@ asm(".file \"" __FILE__ "\"");
 
 extern xen_commandline_t saved_cmdline;
 
-#define xen_extraversion compat_extraversion
 #define xen_extraversion_t compat_extraversion_t
 
 #define xen_compile_info compat_compile_info
-- 
2.5.0


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel