Re: [Xen-devel] [PATCH 28/28 v2] Separate runtime debug output from debug symbols

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Fri, Sep 26, 2014 at 03:43:19PM +0100, Jan Beulich wrote:
> >>> On 26.09.14 at 16:02, <olaf@xxxxxxxxx> wrote:
> > On Fri, Sep 26, Jan Beulich wrote:
> > 
> >> >>> On 26.09.14 at 13:40, <olaf@xxxxxxxxx> wrote:
> >> > --- a/xen/Rules.mk
> >> > +++ b/xen/Rules.mk
> >> > @@ -10,6 +10,10 @@ lock_profile  ?= n
> >> >  crash_debug   ?= n
> >> >  frame_pointer ?= n
> >> >  lto           ?= n
> >> > +debug_xen     ?= y
> >> > +debuginfo_xen ?= y
> >> > +debug         := $(debug_xen)
> >> > +debug_symbols := $(debuginfo_xen)
> >> 
> >> I'm not sure I want to see this do away with the old mechanism
> >> altogether - various of my testing build flavors depend on overriding
> >> the "debug" variable, and I wouldn't want to change all their .config
> >> files. At least for a certain period of time the latter two should
> >> probably continue to use ?=.
> > 
> > I think .config will continue to work with a "debug := y" and also 'make
> > debug=y'.
> 
> Ah, right, you do this before the inclusion point of Config.mk.
> 
> >> > @@ -43,7 +47,10 @@ ALL_OBJS-$(x86)          += 
> >> > $(BASEDIR)/crypto/built_in.o
> >> >  
> >> >  CFLAGS += -fno-builtin -fno-common
> >> >  CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
> >> > -CFLAGS += -pipe -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
> >> > +ifeq ($(debug_symbols),y)
> >> > +CFLAGS += -g
> >> > +endif
> >> > +CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h
> >> >  CFLAGS += -nostdinc
> >> >  
> >> >  CFLAGS-$(XSM_ENABLE)    += -DXSM_ENABLE
> >> 
> >> The context here shows that CFLAGS-y gets used elsewhere, so
> >> please do so too above. And if possible this should be considered
> >> elsewhere too.
> > 
> > I'm not sure what you mean? The hunk just makes "-g" conditional.
> 
> Looks like you nevertheless understood, as your shrunk down
> patch does what I was asking for here (quoted below).
> 
> >> In any event I don't see this change as important enough to
> >> warrant overriding the feature freeze.
> > 
> > Regarding feature freeze, that small version below for 'make debug=yn
> > debug_symbols=yn' will work for me as well.
> 
> This indeed seems more acceptable at this point. But the final
> say would be with Konrad anyway.

Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

I don't see this introducing an regressions. If there is an regression
it will be during build time so pretty easy to spot.


> 
> > --- a/xen/Rules.mk
> > +++ b/xen/Rules.mk
> > @@ -43,7 +43,8 @@ ALL_OBJS-$(x86)          += $(BASEDIR)/crypto/built_in.o
> >  
> >  CFLAGS += -fno-builtin -fno-common
> >  CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
> > -CFLAGS += -pipe -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
> > +CFLAGS-$(debug_symbols) += -g
> 
> (This one.)
> 
> Jan
> 

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