Skip to content

Conversation

@benhillis
Copy link
Member

Resolve a few minor issues preventing some virtiofs tests from passing.

  1. An incorrect error check when trying to map an app execution alias. This was causing the test to segfault and exit early. Was able to track this down because of test: improve test logging infra #13811.
  2. Some test logic that needed to be updated to account for what virtiofs supports.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR resolves two distinct issues preventing virtiofs tests from passing reliably:

  1. Fixed incorrect mmap error handling - Changed from LxtCheckNullErrno to LxtCheckMapErrno macro and properly initialized the mapping pointer with MAP_FAILED instead of NULL. This prevented segfaults when testing app execution aliases.
  2. Updated test logic for virtiofs support - Added virtiofs-specific skip conditions to match existing plan9 behavior, and properly threaded the DrvFsMode parameter to enable conditional test logic.
  • Updated DrvFsFat test to skip for VirtioFS (only supports full drive mounts)
  • Added virtiofs checks alongside plan9 in multiple test skip conditions
  • Fixed initialization order to set config before launching WSL
  • Updated comments to reflect virtiofs limitations

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/windows/DrvFsTests.cpp Added DrvFsMode parameter to DrvFsFat, added VirtioFS skip logic, fixed test setup initialization order to apply config before launching WSL
test/linux/unit_tests/drvfs.c Fixed mmap error checking (LxtCheckMapErrno + MAP_FAILED), added virtiofs skip conditions matching plan9 behavior, updated comments

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@benhillis benhillis merged commit 2844e4a into master Dec 2, 2025
12 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