Add --debug flag for step-level execution logging - #312
Conversation
When --debug is enabled, record detailed execution logs for debugging test failures and guest behavior: - steps.log: step-by-step log with commands, exit codes, stdout/stderr, and timing for each test step - Per-guest log directories (<guest_id>/): - qemu-command.log: full QEMU command line - qemu-boot.log: serial console output (kernel dmesg) - qemu-error.log: QEMU stderr for launch failures - guest-journal.log: journald logs fetched via vsock before vm_stop - Add guest_id field to vm_launch steps for identifying multi-VM runs - Track executed commands in StepResult and StepHandlerResult - Add fetch_guest_journal() helper for guest journal retrieval via vsock - Update effective_vm_profile() to write logs to artifact_dir - Truncate boot log before QEMU starts to avoid stale data Log files are copied to artifact directories before each launch because QEMU overwrites logs in the default location on subsequent runs. Signed-off-by: Harika Nittala <lnittala@amd.com>
amd-aliem
left a comment
There was a problem hiding this comment.
I tried it out and it's fabulous! Super nice to have all the qemu and journal logs now, debugging will be much easier.
The main issue I see is just getting the behavior to match the README - it claims that --debug is required to generate the new files, but it appears most of them will be written or pulled even if it is not specified.
| profile.image_path, | ||
| "-device", | ||
| _build_vsock_device(profile), | ||
| # Serial console to capture guest boot logs (dmesg output) |
There was a problem hiding this comment.
This results in always writing the serial log, whether or not --debug is specified - need to append it to the command conditionally (on the next line).
| else: | ||
| f.write("(no errors)\n") | ||
|
|
||
| def _copy_guest_error_log(self, error_log_path: str) -> None: |
There was a problem hiding this comment.
Can we create the vm subdirectory before the VM launches and then write the relevant log files directly into that subdirectory, to prevent having to copy?
| ) | ||
| else: | ||
| # Fetch guest journald logs before stopping the VM | ||
| try: |
There was a problem hiding this comment.
Could optimize the normal (not --debug) case by not fetching the journal if we're not going to write it to a file.
|
|
||
| def _verify_result(mode: str) -> StepHandlerResult: | ||
| """Compare Reported vs Platform TCB; used by callable steps and the CLI.""" | ||
| cmd = "snphost show tcb" |
There was a problem hiding this comment.
hmmm defining this here might make logging errors in the future (if we forget to update both).
I also see that in other functions you also define the command and pass it to both the subprocess.run() and the StepHandlerResult.
I think it would be cleaner to wrap subprocess.run() in another function that returns the StepHandlerResult directly:
def run_command(cmd: list[str], **kwargs) -> StepHandlerResult:
I realize this might be a lot of work for this PR so no worries if do not wish to handle it as a part of this PR, we can do a follow up.
When
--debugis enabled, sev-verify framework records the detailed execution logs for debugging test failures and guest behavior:Log files are copied to artifact directories before each launch because QEMU overwrites logs in the default location on subsequent runs.