debug () call in external_command () should have a 'running: ' prefix#38
Open
sparimi-rh wants to merge 1 commit intolibguestfs:masterfrom
Open
debug () call in external_command () should have a 'running: ' prefix#38sparimi-rh wants to merge 1 commit intolibguestfs:masterfrom
sparimi-rh wants to merge 1 commit intolibguestfs:masterfrom
Conversation
crobinso
requested changes
Apr 7, 2026
Collaborator
crobinso
left a comment
crobinso
left a comment
There was a problem hiding this comment.
Commit message needs to be improved too. A title like mltools: improve command debug() output and then a body explaining this makes debug logs more clear where commands are originating
| let external_command ?(echo_cmd = true) ?help cmd = | ||
| if echo_cmd then | ||
| debug "%s" cmd; | ||
| debug "%s: running: %s" !prog cmd; |
Collaborator
There was a problem hiding this comment.
after you adjust #37 like I suggested there, the !prog handling should be in the debug function itself, so drop it here.
In testing I noticed there are 2 other command running functions that need adjustment: shell_command like I mentioned elsewhere, and do_run. So since there are 3 call sites here I think we should instead embed the function name in the debug statement, which makes it easier to track down. so this case would be debug "external_command: %s" cmd
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change adds prefix
"virt-v2v: running:" to the Log statements generated byvirt-v2v` tool when it invokes the external program.Adding prefix to the generated log statements helps any external Log viewers to use the prefix in the framing the filter rules. For e.g. virt-v2v invokes openssl, nbdkit or nbdinfo or any other external programs, the prefix helps in filtering specific log statements.