Skip to content

Improve logging#760

Merged
joe-lawrence merged 8 commits intodynup:masterfrom
juergh:improve-logging
Nov 17, 2017
Merged

Improve logging#760
joe-lawrence merged 8 commits intodynup:masterfrom
juergh:improve-logging

Conversation

@juergh
Copy link
Copy Markdown

@juergh juergh commented Nov 13, 2017

No description provided.

This was introduced in commit 5352d8b ('build objects in separate
directory to fix caching') but is no longer necessary.

Fixes: 2e99d6b ('kpatch-build: build the kernel in ~/.kpatch/src again')
Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
Fixes: 8dc25d7 ('kpatch-build: let user specify kpatch module name')
Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
Replace stray spaces with tabs, except in the usage output where tabs
don't make much sense.

Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
This can be used for building a kpatch module for a non-running
kernel. Note that the correct kernel and debug packages still need
to be installed.

Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
The current checks never fail, because the first grep in the pipeline
doesn't write anything to stdout.

Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
When searching for 'Linux version ...' in vmlinux, stop after the first
match so that we don't keep reading a potentially huge file.

Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
@jpoimboe
Copy link
Copy Markdown
Member

This is a nice improvement, but as somebody who uses the -d flag a lot, the thing I most often care about is the kpatch-build xtrace (along with the preserving of ~/.kpatch/tmp).

So what do you think about the following?

  • -d: xtrace in kpatch-build only (not kpatch-gcc)
  • -d -d: all the other stuff (xtrace everywhere, log to stdout)

@juergh
Copy link
Copy Markdown
Author

juergh commented Nov 15, 2017

I usually want all the debug output on stdout but without the xtrace noise :-) How about we keep -d as is (without xtrace in kpatch-gcc) and add another flag for the new behaviour?

Add a logger funcition that can be used to log to both stdout and the
logfile or only to the logfile. This is needed for subsequent patches
where we introduce an alternate debug mode.

Since we're piping to a logger now, we need to set 'pipefail' otherwise
the return status of such a pipeline is always 0 (the exit status of the
logger) and we won't catch any errors.

From the bash manpage:
  The return status of a pipeline is the exit status of the last command,
  unless the pipefail option is enabled

Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
@juergh
Copy link
Copy Markdown
Author

juergh commented Nov 15, 2017

How about this: https://github.com/juergh/kpatch/tree/improve-logging-v2
And specifically commit juergh@9cfb5cb
which introduces a new commandline option to enable the alternate debug mode.

@jpoimboe
Copy link
Copy Markdown
Member

That could work, but I'd like to try to keep the debug options as simple as we can. I'm ok with running kpatch-build as bash -x kpatch-build to get an xtrace. But I still need the ~/.kpatch/tmp from being deleted. So how about

  • -d: don't delete ~/.kpatch/tmp
  • -d -d: ... and log to stdout

Then I can run bash -x kpatch-build -d and you can run kpatch-build -d -d. That sound ok?

@juergh
Copy link
Copy Markdown
Author

juergh commented Nov 16, 2017

Hmmm.... I want to make this non-intrusive, i.e., not change the current behaviour. How about adding -dd, -ddd, -dddd for the other debug modes. Simple enough: https://github.com/juergh/kpatch/tree/improve-logging-v3

@jpoimboe
Copy link
Copy Markdown
Member

Ok, that sounds good.

@juergh
Copy link
Copy Markdown
Author

juergh commented Nov 17, 2017

Force pushed to branch improve-logging.
@jpoimboe Do you have an estimate for when this might get merged? I don't want to be too far ahead of current master with my other changes.

@jpoimboe
Copy link
Copy Markdown
Member

Sorry, for the delay, we'll try to merge #759 soon.

This new commit isn't working for me. getopt is interpreting "-dd" as "-d -d", so it just enables xtrace but nothing else.

By specifying -d, --debug multiple times, the following additional
debug modes can be enabled:
  -d -d:       Writes everything that is written to the logfile also to
               stdout.
  -d -d -d:    Same as '-d -d' plus sets 'xtrace' in kpatch-build.
  -d -d -d -d: Same as '-d -d -d' plus sets 'xtrace' in kpatch-gcc.

Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
@jpoimboe
Copy link
Copy Markdown
Member

👍

@joe-lawrence
Copy link
Copy Markdown
Contributor

The latest version works as expected. Thanks @juergh

@joe-lawrence joe-lawrence merged commit 6d1425e into dynup:master Nov 17, 2017
aliceinwire pushed a commit to elivepatch/elivepatch-server that referenced this pull request Dec 12, 2017
debug option is now accepting multiple -d arguments
dynup/kpatch#760
aliceinwire pushed a commit to elivepatch/elivepatch-server that referenced this pull request Jun 5, 2018
debug option is now accepting multiple -d arguments
dynup/kpatch#760
aliceinwire pushed a commit to elivepatch/elivepatch-server that referenced this pull request Jun 5, 2018
debug option is now accepting multiple -d arguments
dynup/kpatch#760
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants