diff mbox

[v5,1/6] build: convert debug to Kconfig

Message ID 1464098199-19851-2-git-send-email-cardoe@cardoe.com (mailing list archive)
State New, archived
Headers show

Commit Message

Douglas Goldstein May 24, 2016, 1:56 p.m. UTC 
Enabling debug will disable NDEBUG which will result in more debug
prints.  There are a number of debugging options for Xen so place the
debug option under a menu for different debugging options to have a way
to group them all together.

Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wei.liu2@citrix.com>
---
 xen/Kconfig              |  2 ++
 xen/Kconfig.debug        | 14 ++++++++++++++
 xen/Rules.mk             |  5 +++--
 xen/include/xen/config.h |  4 ++++
 4 files changed, 23 insertions(+), 2 deletions(-)
 create mode 100644 xen/Kconfig.debug

Comments

Jan Beulich May 24, 2016, 2:53 p.m. UTC | #1 
>>> On 24.05.16 at 15:56, <cardoe@cardoe.com> wrote:
> Enabling debug will disable NDEBUG which will result in more debug
> prints.  There are a number of debugging options for Xen so place the
> debug option under a menu for different debugging options to have a way
> to group them all together.
> 
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
Julien Grall June 8, 2016, 4:53 p.m. UTC | #2 
Hi Doug,

On 24/05/16 14:56, Doug Goldstein wrote:
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index 961d533..da2f490 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -20,13 +20,14 @@ include $(XEN_ROOT)/Config.mk
>   ifeq ($(debug),y)
>   verbose       := y
>   frame_pointer := y
> -else
> -CFLAGS += -DNDEBUG
>   endif
>   ifeq ($(perfc_arrays),y)
>   perfc := y
>   endif
>
> +ifeq ($(origin debug),command line)
> +$(warning "You must use 'make menuconfig' to enable/disable debug now.")

While building Xen with "debug=.." on the command Line, I got the 
warning because I have to use Kconfig now. This warning is lost among 
compilation logs.

As this is a warning, I would expect debug=... to work as previously. 
However debug= is just ignored. So I think we should replace the warning 
by an error to avoiding people spending time to understanding why debug 
has not been enabled.

Any opinions?

Regards,
Douglas Goldstein June 8, 2016, 5:34 p.m. UTC | #3 
On 6/8/16 12:53 PM, Julien Grall wrote:
> Hi Doug,
> 
> On 24/05/16 14:56, Doug Goldstein wrote:
>> diff --git a/xen/Rules.mk b/xen/Rules.mk
>> index 961d533..da2f490 100644
>> --- a/xen/Rules.mk
>> +++ b/xen/Rules.mk
>> @@ -20,13 +20,14 @@ include $(XEN_ROOT)/Config.mk
>>   ifeq ($(debug),y)
>>   verbose       := y
>>   frame_pointer := y
>> -else
>> -CFLAGS += -DNDEBUG
>>   endif
>>   ifeq ($(perfc_arrays),y)
>>   perfc := y
>>   endif
>>
>> +ifeq ($(origin debug),command line)
>> +$(warning "You must use 'make menuconfig' to enable/disable debug now.")
> 
> While building Xen with "debug=.." on the command Line, I got the
> warning because I have to use Kconfig now. This warning is lost among
> compilation logs.
> 
> As this is a warning, I would expect debug=... to work as previously.
> However debug= is just ignored. So I think we should replace the warning
> by an error to avoiding people spending time to understanding why debug
> has not been enabled.
> 
> Any opinions?
> 
> Regards,
> 

Julien,

Yes it needs to become an error. Right now its a warning because its
actually set at the top level. Jan suggested dropping it from the top
level and converting this to an error. But before that we need to give
the tools directory an --{enable,disable}-debug. I've got that written
but I'm at the OpenXT Summit and my dev box is off at home.

I apologize that you got bit by this.
diff mbox

Patch

diff --git a/xen/Kconfig b/xen/Kconfig
index fa8b27c..0fe7a1a 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -26,3 +26,5 @@  config DEFCONFIG_LIST
 config EXPERT
 	string
 	option env="XEN_CONFIG_EXPERT"
+
+source "Kconfig.debug"
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
new file mode 100644
index 0000000..796af0c
--- /dev/null
+++ b/xen/Kconfig.debug
@@ -0,0 +1,14 @@ 
+
+menu "Debugging Options"
+
+config DEBUG
+	bool "Developer Checks"
+	default y
+	---help---
+	  If you say Y here this will enable developer checks such as asserts
+	  and extra printks. This option is intended for development purposes
+	  only, and not for production use.
+
+	  You probably want to say 'N' here.
+
+endmenu
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 961d533..da2f490 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -20,13 +20,14 @@  include $(XEN_ROOT)/Config.mk
 ifeq ($(debug),y)
 verbose       := y
 frame_pointer := y
-else
-CFLAGS += -DNDEBUG
 endif
 ifeq ($(perfc_arrays),y)
 perfc := y
 endif
 
+ifeq ($(origin debug),command line)
+$(warning "You must use 'make menuconfig' to enable/disable debug now.")
+endif
 ifneq ($(origin kexec),undefined)
 $(error "You must use 'make menuconfig' to enable/disable kexec now.")
 endif
diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
index ef6e5ee..473c5e8 100644
--- a/xen/include/xen/config.h
+++ b/xen/include/xen/config.h
@@ -81,4 +81,8 @@ 
 /* allow existing code to work with Kconfig variable */
 #define NR_CPUS CONFIG_NR_CPUS
 
+#ifndef CONFIG_DEBUG
+#define NDEBUG
+#endif
+
 #endif /* __XEN_CONFIG_H__ */