Skip to content

Conversation

@klihub
Copy link
Member

@klihub klihub commented Nov 18, 2025

Always log device injection summary, also in verbose mode.

Copy link
Contributor

@chrishenzie chrishenzie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM outside of one comment

Log injection and other adjustment summary also in verbose mode.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
@klihub klihub force-pushed the fixes/log-device-injection-summary branch from ee54476 to 14cc2e2 Compare November 19, 2025 07:47
@klihub klihub requested a review from mikebrow November 21, 2025 06:30
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdyt about:
if verbose .. log.Infof
else log.Debugf

@chrishenzie
Copy link
Contributor

@mikebrow I would expect applications to always write info/warn/error logs and not condition that behind anything, but optionally (additionally) log debug logs. Maybe this would be simpler if instead of verbose, we just added a log-level flag.

@mikebrow
Copy link
Member

@mikebrow I would expect applications to always write info/warn/error logs and not condition that behind anything, but optionally (additionally) log debug logs. Maybe this would be simpler if instead of verbose, we just added a log-level flag.

nod.. guess we have not been consistent on the plugin log level vs verbosity flag options handled by each main() impl...

maybe set the default in config.. anyhow I suppose this PR is better and we could use a discussion and normalization across the samples/plugins.

@klihub klihub requested a review from mikebrow December 3, 2025 09:02
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mikebrow mikebrow merged commit 649a151 into containerd:main Dec 3, 2025
16 checks passed
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