Skip to content

Conversation

@guzu
Copy link

@guzu guzu commented Nov 20, 2025

To debug or just understand why a test failed it could be interesting to have more logs from the VM.
For instance, sometimes tests failed because a VM isn't able to get an IP in the time limit of test.

Using unix_vm_with_serial_console fixture, a test can ask to log serial output for a VM (Unix in this case, or could be Linux instead).
The fixture also ensure that the serial output is enabled by modifying the kernel command-line, because for the moment images we are importing for the tests don't have the console enabled.
This is debatable, but at least for now we try this patch set without modifying any of the current images.


The branch is on top eva/log-test-failure (#362) and ideally would also require that branch eva/local-cmd-generalize (#359) be merged also.
That's why the PR is not on top master.

Windows not supported (yet?). VM type (Unix or Windows) depends on test.
Distro classes only implement the console configuration and is specific to bootloader, but should move more methods from VM to UnixVM or WindowsVM.

Tested with:

uv run pytest \
    tests/misc/test_serial_console_logging.py::TestWithSessionLogging::test_vm_reboots \
    --hosts=10.1.30.2 --log-cli-level=debug --no-collect-logs-on-failure

With the following VMs on a XCP-ng 8.3 :

  almalinux-8.5-uefi-created_8.2-zstd.xva
  almalinux-9-05082022-uefi-created_8.2-zstd.xva
  alpine-minimal-uefi-3.21.3-created-8.2.zstd.xva
  centos-stream-9-04082022-uefi-created_8.2-zstd.xva
  centos6-32-PVINPVH-created_8.2-zstd.xva
  centos6-32-hvm-created_8.2-zstd.xva
  centos6-64-PVINPVH-created_8.2-zstd.xva
  debian-11.1-gui-created_8.2-zstd.xva
  debian-12-created_8.2-zstd.xva
  openSUSE-Leap-15-4-uefi-created_8.2-zstd.xva
  ubuntu-22.04-server-uefi-created_8.2.xva
  ubuntu-24.04-server-uefi-created_8.2.xva

Not tested:

 debian-12.1-uefi-created_8.2-zstd.xva
 debian-uefi-10.4.0-created_8.2-zstd-efitools.xva
 freebsd-13.2-uefi-created_8.2-zstd.xva
 freebsd-14-uefi-created_8.2-zstd.xva

@guzu guzu requested review from dinhngtu, stormi and ydirson November 20, 2025 13:30
@stormi stormi requested a review from a team November 20, 2025 15:53
@stormi
Copy link
Member

stormi commented Nov 20, 2025

I think reviewers are lacking context here to be able to review. The commit messages says what changes, but not the most important part: what problem you want to solve and why it's the appropriate solution (see https://docs.xcp-ng.org/project/development-process/commit-message-conventions/). Such large changes would need some bits of prior discussions before being thrown into review. Especially the second commits which could have large impacts on the tests, and is a new requirement I wasn't aware of.

@dinhngtu dinhngtu removed their request for review November 20, 2025 16:27
@guzu guzu force-pushed the eva/log-test-failure branch from 1ae1d60 to 0257aef Compare November 25, 2025 14:13
@guzu guzu force-pushed the eva/vm-uart-logging branch from bbd8ae5 to 911bee5 Compare November 25, 2025 14:17
@guzu guzu merged commit 911bee5 into eva/log-test-failure Nov 25, 2025
9 checks passed
@guzu guzu force-pushed the eva/log-test-failure branch from 0257aef to 911bee5 Compare November 25, 2025 14:17
@guzu guzu deleted the eva/vm-uart-logging branch November 25, 2025 14:17
@stormi
Copy link
Member

stormi commented Nov 25, 2025

Is there context from this PR that needs to be moved to #362 now?

@guzu guzu restored the eva/vm-uart-logging branch November 26, 2025 08:19
@guzu
Copy link
Author

guzu commented Nov 26, 2025

I don't know what I did but I did not want to merge that for sure

@stormi
Copy link
Member

stormi commented Nov 28, 2025

I think you'll have to recreate the branch from a hash, and maybe open a new PR, then.

@guzu
Copy link
Author

guzu commented Dec 1, 2025

I think you'll have to recreate the branch from a hash, and maybe open a new PR, then.

My bad. I force pushed the wrong commit, and Github marked the PR as merged... I can't go backward so I duplicated the PR here #370.

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