Skip to content

WSL2: ship kernel headers and perf in the kernel artifacts VHD - #41267

Open
Ben Hillis (benhillis) wants to merge 12 commits into
masterfrom
user/benhill/module_vhd_artifacts
Open

WSL2: ship kernel headers and perf in the kernel artifacts VHD#41267
Ben Hillis (benhillis) wants to merge 12 commits into
masterfrom
user/benhill/module_vhd_artifacts

Conversation

@benhillis

Copy link
Copy Markdown
Member

Summary

  • update WSL to Microsoft.WSL.Kernel 6.18.40.1-1 and package the unified artifacts.vhd
  • mount versioned kernel modules, headers, and perf tooling from the artifacts VHD while retaining deprecated legacy flat-layout support
  • expose matching kernel headers through /lib/modules/<release>/build and bind mount the matching perf binary at /usr/bin/perf
  • update development paths, documentation, and kernel artifact coverage

Testing

  • full x64 Debug build
  • bin\x64\Debug\test.bat /name:*Kernel* (8 passed)
  • python -m mkdocs build -f doc\mkdocs.yml

Copilot AI lite review requested due to automatic review settings August 6, 2026 01:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates WSL2’s kernel packaging to ship a unified artifacts.vhd containing kernel modules plus additional developer artifacts (kernel headers and perf), and wires up the Linux init + Windows service paths to mount these version-matched resources into the distro environment.

Changes:

  • Switch kernel artifact packaging/paths from modules.vhd to a unified artifacts.vhd across build/dev shortcuts and MSI payloads.
  • Extend Linux init to detect the new versioned/nested artifacts layout and (when present) expose headers via /lib/modules/<release>/build and perf via /usr/bin/perf.
  • Add/adjust Windows tests and documentation to cover the new artifacts behavior and mounting locations.

Reviewed changes

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

Show a summary per file
File Description
UserConfig.cmake.sample Updates dev-copy and ACL setup to use artifacts.vhd.
test/windows/UnitTests.cpp Updates modules mount expectation and adds a new KernelArtifacts test for headers + perf.
src/windows/service/exe/WslCoreVm.cpp Updates default kernel artifacts VHD filename to artifacts.vhd.
src/windows/service/exe/HcsVirtualMachine.cpp Updates default tools VHD filename to artifacts.vhd.
src/shared/inc/lxinitshared.h Adds env vars for passing headers/perf mount + target paths into distro init.
src/linux/init/main.cpp Adds nested-layout detection and binds headers/perf payloads from the artifacts VHD.
src/linux/init/config.cpp Moves mounts into the distro namespace and sets up /lib/modules/<release>/build and /usr/bin/perf.
packages.config Bumps Microsoft.WSL.Kernel dependency version.
msipackage/package.wix.in Ships artifacts.vhd in the MSI instead of modules.vhd.
doc/docs/technical-documentation/boot-process.md Documents the new headers/perf mounting behavior.
CMakeLists.txt Updates dev compile-time path definition to point at artifacts.vhd.

Comment thread src/linux/init/config.cpp Outdated
Copilot AI review requested due to automatic review settings August 6, 2026 03:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/linux/init/util.cpp:1745

  • UtilMountFile() now stats the source path, but it only checks for existence. If Source exists but is not a regular file (e.g., a directory due to a packaging/layout issue), the subsequent bind mount will fail after creating Destination, potentially leaving an empty executable stub behind (e.g., /usr/bin/perf). Validate the source is a regular file before creating/mutating the destination.
    struct stat sourceInfo{};
    THROW_LAST_ERROR_IF(stat(Source, &sourceInfo) < 0);

    // Is the file is a symlink, delete it since that would break the mount.
    if (std::filesystem::is_symlink(Destination))

src/linux/init/config.cpp:1158

  • When exposing kernel headers, the code creates /lib/modules//build as a symlink but ignores EEXIST. If the distro already has a stale/wrong symlink (or a leftover file) at that path, WSL will silently keep pointing at the wrong headers even though the correct headers were mounted. Consider removing an existing non-directory entry before creating the symlink, and log a clear warning if a directory prevents creating the symlink.
                        if (UtilMkdirPath(modulesDir.c_str(), 0755) == 0)
                        {
                            const std::string linkPath = modulesDir + "/build";
                            if ((symlink(headersRoot.c_str(), linkPath.c_str()) < 0) && (errno != EEXIST))
                            {

Copilot AI review requested due to automatic review settings August 6, 2026 03:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (1)

test/windows/UnitTests.cpp:3021

  • KernelArtifacts test compiles a C program with cc, which adds a dependency on a compiler toolchain being installed in the test distro. This is likely to be missing in minimal images and can make CI runs flaky/unrelated to kernel artifacts. Prefer validating the headers’ presence/recency by checking for expected symbols directly in the mounted header tree (no compiler required).
            LxsstuLaunchWsl(
                LR"BASH(bash -ec '
                    d=$(mktemp -d)
                    trap "rm -rf $d" EXIT
                    cat > "$d/t.c" <<EOF

Copilot AI review requested due to automatic review settings August 6, 2026 17:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@benhillis
Ben Hillis (benhillis) force-pushed the user/benhill/module_vhd_artifacts branch from 2c470ca to 7b17a3e Compare August 6, 2026 18:02
Copilot AI review requested due to automatic review settings August 6, 2026 18:02
@benhillis
Ben Hillis (benhillis) force-pushed the user/benhill/module_vhd_artifacts branch from 2c470ca to 7b17a3e Compare August 6, 2026 18:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@benhillis
Ben Hillis (benhillis) requested a lite review from Copilot August 6, 2026 19:52
Comment thread src/linux/init/config.cpp Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/linux/init/main.cpp:1634

  • The comment says distro init will move the kernel headers mount to /usr/src/linux-headers-<uname -r>/include, but the code passes a target of /usr/src/linux-headers-<release> and later expects headers under .../include/.... This mismatch is confusing and makes it harder to reason about the mount layout.
    // If kernel headers were mounted, move them to a temporary location and pass the desired
    // target path to the distro init via an environment variable. Distro init will move the
    // mount to /usr/src/linux-headers-<uname -r>/include and create the
    // /lib/modules/<release>/build symlink.

test/windows/UnitTests.cpp:3033

  • stat -Lc dereferences symlinks, so this check can pass even if /usr/bin/perf is just a symlink (or otherwise not a mount point). The later umount /usr/bin/perf in this test would then fail. It’s more robust to first assert /usr/bin/perf is actually mounted (e.g., via /proc/self/mountinfo) and compare inode/device without -L.
        VERIFY_ARE_EQUAL(
            LxsstuLaunchWsl(
                L"test \"$(stat -Lc %d:%i /usr/bin/perf)\" = \"$(stat -Lc %d:%i /usr/lib/linux-tools/$(uname -r)/bin/perf)\"", nullptr, nullptr, nullptr, nullptr),
            0u);

Copilot AI review requested due to automatic review settings August 7, 2026 15:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/linux/init/main.cpp:1634

  • This comment says the headers mount is moved to /usr/src/linux-headers-<uname -r>/include, but the code sets Config.KernelHeadersTarget to /usr/src/linux-headers-<release> (no /include) and the tests expect /lib/modules/<release>/build/include/... to exist. The comment should match the actual mount target.
    // If kernel headers were mounted, move them to a temporary location and pass the desired
    // target path to the distro init via an environment variable. Distro init will move the
    // mount to /usr/src/linux-headers-<uname -r>/include and create the
    // /lib/modules/<release>/build symlink.

src/linux/init/WSLCInit.cpp:858

  • In WSLC_MOUNT_MODULES handling, g_state.ModulesMountPoint is assigned before the bind-mount succeeds. If UtilMount() throws after the assignment, ModulesMountPoint remains set and later chroot mounts will attempt to MS_MOVE a non-existent mount, and subsequent WSLC_MOUNT_MODULES requests will hit the assert(!ModulesMountPoint). Also, this path always bind-mounts "//modules" and doesn’t fall back to the legacy flat-layout VHD (modules at filesystem root), which previously worked via the old WSLC_MOUNT::KernelModules path.
        assert(!g_state.ModulesMountPoint.has_value());

        utsname unameBuffer{};
        THROW_LAST_ERROR_IF(uname(&unameBuffer) < 0);

        const char* source = wsl::shared::string::FromSpan(Buffer, Message.SourceIndex);
        THROW_LAST_ERROR_IF(UtilMount(source, c_kernelModulesVhdMountPoint, "ext4", MS_RDONLY, nullptr, c_defaultRetryTimeout) < 0);

Copilot AI review requested due to automatic review settings August 7, 2026 16:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/linux/init/util.cpp:1745

  • Grammar in this comment is incorrect and it refers to the destination path, not the source file.
    // Is the file is a symlink, delete it since that would break the mount.

src/linux/init/config.cpp:1192

  • The perf shadowing logic only bind-mounts over /usr/bin/perf when it is a regular file. On many distros /usr/bin/perf is a symlink (e.g. alternatives), so the kernel-matched perf won’t be used and the distro perf may be picked up instead, contradicting the intent to always expose the kernel-matched perf when the distro provides one.
        struct stat existing{};
        if ((lstat(PERF_BINARY_PATH, &existing) == 0) && S_ISREG(existing.st_mode))
        {
            const std::string perfBinary = target + "/bin/perf";
            if (UtilMountFile(perfBinary.c_str(), PERF_BINARY_PATH) < 0)
            {
                LOG_ERROR("UtilMountFile({}, {}) failed {}", perfBinary, PERF_BINARY_PATH, errno);
            }
        }

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/linux/init/config.cpp:1185

  • A distro-provided /usr/bin/perf can be a symlink, but this condition only shadows regular files. Because ConfigAppendToPath appends the artifacts directory after /usr/bin, such a symlink remains the first perf found and the kernel-matched binary is not used. Include symlinks here; UtilMountFile already removes a symlink destination before creating the bind mount.
        if ((lstat(PERF_BINARY_PATH, &existing) == 0) && S_ISREG(existing.st_mode))

src/linux/init/config.cpp:1161

  • unlink() cannot remove an existing directory. If build is a directory, this logs twice and leaves that stale directory in place, so /lib/modules/<release>/build does not expose the bundled matching headers as promised. Handle non-symlink directory entries explicitly before creating the link.
        const std::string linkPath = kernelModulesPath + "/build";
        if ((unlink(linkPath.c_str()) < 0) && (errno != ENOENT))
        {
            LOG_ERROR("unlink({}) failed {}", linkPath, errno);
        }

src/linux/init/main.cpp:3216

  • The new legacy-layout compatibility branch is not exercised by KernelModules or KernelArtifacts; both use the new artifacts.vhd nested layout. Since preserving flat-layout custom module VHDs is an explicit compatibility requirement, add a test that boots with a flat VHD and verifies the root is selected as the overlay lower directory (and headers/perf remain unavailable).
            const bool NestedLayout = (stat(NestedModules.c_str(), &StatBuffer) == 0) && S_ISDIR(StatBuffer.st_mode);
            const std::string ModulesLower = NestedLayout ? NestedModules : std::string{KERNEL_MODULES_VHD_PATH};
            const bool LegacyLayout =
                !NestedLayout && (stat((ModulesLower + "/modules.dep").c_str(), &StatBuffer) == 0) && S_ISREG(StatBuffer.st_mode);

src/linux/init/main.cpp:1634

  • This comment says the mount is moved to the include subdirectory, but KernelHeadersTarget is /usr/src/linux-headers-<release> and MoveTemporaryMount moves the mount there. Correct the documented target to match the implementation.
    // If kernel headers were mounted, move them to a temporary location and pass the desired
    // target path to the distro init via an environment variable. Distro init will move the
    // mount to /usr/src/linux-headers-<uname -r>/include and create the
    // /lib/modules/<release>/build symlink.

Comment thread src/linux/init/config.cpp
Comment thread src/linux/init/main.cpp Outdated
Comment thread src/linux/init/util.cpp
Comment thread test/windows/UnitTests.cpp Outdated
Copilot AI review requested due to automatic review settings August 10, 2026 21:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/linux/init/config.cpp:1199

  • A distro-provided /usr/bin/perf can be a symlink, but this condition only shadows regular files. Because ConfigAppendToPath appends the bundled tools after the existing PATH (config.cpp:339-358), /usr/bin still wins and the distro's perf is executed instead of the kernel-matched binary. UtilMountFile already removes symlink destinations safely (util.cpp:1750-1754), so include symlinks here as well; the artifact test should also exercise this case.
        if ((lstat(PERF_BINARY_PATH, &existing) == 0) && S_ISREG(existing.st_mode))

src/linux/init/main.cpp:3215

  • The newly added legacy-layout compatibility path is not exercised by the kernel tests: they use the packaged artifacts.vhd, so NestedLayout is always true. Since retaining support for existing flat modules.vhd files is an explicit compatibility goal, add a test that boots with a flat VHD and verifies its modules tree is mounted/usable; otherwise this fallback can regress unnoticed.
            const bool LegacyLayout = !NestedLayout && std::filesystem::is_regular_file(ModulesLower + "/modules.dep", Error);

Comment thread src/linux/init/util.cpp Outdated
Copilot AI review requested due to automatic review settings August 10, 2026 21:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (6)

src/linux/init/config.cpp:1756

  • ConfigAppendToPath appends this directory after the distro's existing $PATH. Consequently, any pre-existing perf earlier in the path—especially a /usr/bin/perf symlink skipped by the check above—still wins, so perf may not match the running kernel. Prepend the artifact directory while preserving the existing path.
            ConfigAppendToPath(Environment, std::format("{}/bin", *Config.KernelPerfPath));

src/linux/init/main.cpp:3215

  • When the release-specific modules directory is absent, ModulesLower falls back to the VHD root even when LegacyLayout is false. A malformed artifacts VHD—or one built for a different kernel release—is therefore treated as a legacy modules tree and boot proceeds with an unusable overlay. Reject the VHD unless either layout was actually detected, and cover the retained flat-layout branch in the kernel tests.
            const bool NestedLayout = std::filesystem::is_directory(NestedModules, Error);
            const std::string ModulesLower = NestedLayout ? NestedModules : std::string{KERNEL_MODULES_VHD_PATH};
            const bool LegacyLayout = !NestedLayout && std::filesystem::is_regular_file(ModulesLower + "/modules.dep", Error);

src/linux/init/config.cpp:1200

  • This only shadows a regular /usr/bin/perf. If a distro supplies perf as a symlink, direct invocations of /usr/bin/perf continue to use the distro version, contrary to the documented shadowing behavior. Please handle symlink targets without permanently replacing the distro's symlink; the current UtilMountFile helper deletes symlinks, so simply broadening this condition would leave filesystem changes behind.
        struct stat existing{};
        if ((lstat(PERF_BINARY_PATH, &existing) == 0) && S_ISREG(existing.st_mode))
        {

src/linux/init/main.cpp:1634

  • This comment says the mount is moved to the include subdirectory, but KernelHeadersTarget and MoveTemporaryMount mount the artifact at /usr/src/linux-headers-<release> itself. Correct the documented target so it matches the implementation and tests.
    // If kernel headers were mounted, move them to a temporary location and pass the desired
    // target path to the distro init via an environment variable. Distro init will move the
    // mount to /usr/src/linux-headers-<uname -r>/include and create the
    // /lib/modules/<release>/build symlink.

doc/docs/technical-documentation/boot-process.md:86

  • This bullet incorrectly states that the InitialConfig message contains a kernel-artifacts mounting choice. The artifacts are discovered from the VHD identifier in EarlyConfig, and their mounts are moved into each distro while handling its launch. Move this explanation into the distribution-start section so the documented protocol remains accurate.
- Whether to mount the bundled Linux kernel headers and perf tooling (shipped in the kernel artifacts VHD; headers are mounted at `/usr/src/linux-headers-$(uname -r)` with `/lib/modules/$(uname -r)/build` symlinked to them, and perf is mounted at `/usr/lib/linux-tools/$(uname -r)`, added to the default `$PATH` and, when the distribution ships its own `/usr/bin/perf`, bind mounted over it)

test/windows/UnitTests.cpp:3033

  • The PR summary says the matching perf binary is bind mounted at /usr/bin/perf, but this test requires that path not to exist when the distro does not already provide it. Either always provide the documented path or clarify the PR contract to state that /usr/bin/perf is only shadowed when already present.
        // N.B. The test distro does not ship perf, so nothing should be created at /usr/bin/perf.
        VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"test -x /usr/lib/linux-tools/$(uname -r)/bin/perf", nullptr, nullptr, nullptr, nullptr), 0u);
        VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"test ! -e /usr/bin/perf", nullptr, nullptr, nullptr, nullptr), 0u);

Copilot AI review requested due to automatic review settings August 10, 2026 22:10
@benhillis
Ben Hillis (benhillis) force-pushed the user/benhill/module_vhd_artifacts branch from b363666 to 27a3416 Compare August 10, 2026 22:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/linux/init/config.cpp:1199

  • lstat classifies /usr/bin/perf symlinks as S_IFLNK, so this skips the bind mount for a distro whose perf entry is a symlink to a regular binary. Because ConfigAppendToPath appends the artifacts directory, that earlier /usr/bin/perf still wins and the kernel-matched tool is not used. Follow the symlink here; UtilMountFile already removes a symlink destination before creating the bind-mount target.
        if ((lstat(PERF_BINARY_PATH, &existing) == 0) && S_ISREG(existing.st_mode))

test/windows/UnitTests.cpp:3020

  • This prefix comparison can accept mismatched headers: for example, header version 6.18.4 matches a running release beginning with 6.18.40. Require a release-component/suffix boundary after $v so the test actually verifies the package's headers match the running kernel.
                    case "$(uname -r)" in "$v"*) exit 0 ;; *) exit 8 ;; esac

src/linux/init/main.cpp:3215

  • The new flat-layout branch is the backward-compatibility path for existing custom module-only VHDs, but the updated KernelModules test only exercises the nested artifact lowerdir and no test supplies a root-level modules.dep. Add a flat-layout VHD case that boots with custom kernel/modules and verifies module loading, otherwise this promised compatibility can regress unnoticed.
            const std::string ModulesLower = NestedLayout ? NestedModules : std::string{KERNEL_MODULES_VHD_PATH};
            const bool LegacyLayout = !NestedLayout && std::filesystem::is_regular_file(ModulesLower + "/modules.dep", Error);

Ben Hillis and others added 12 commits August 11, 2026 09:08
Rework the kernel headers feature so kernel modules, headers, and the
perf binary all ship inside the existing kernel modules VHD (a single
"artifacts VHD"), replacing the loose UAPI headers mounted over a 9p
share and the separate kernelHeaders= config setting.

Guest init detects the nested <release>/{modules,linux-headers,perf}
layout, overlays the modules tree so depmod/modprobe work, and
bind-moves the headers and perf trees into each distro namespace,
exposing /lib/modules/<release>/build and /usr/bin/perf. Adds dmesg
warnings for malformed or mismatched artifacts VHDs (missing
modules.dep, headers, perf dir, or perf binary).

Copilot-Session: 43112c89-2bd9-412a-b971-8711b8581800
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: aa7f28d5-85ed-4573-8f45-d056c46f5372
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: aa7f28d5-85ed-4573-8f45-d056c46f5372
Validate file bind mount sources and replace stale kernel header build links before exposing packaged artifacts.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: aa7f28d5-85ed-4573-8f45-d056c46f5372
Mount the packaged artifacts VHD temporarily and bind the kernel-specific modules directory into the WSLC utility VM.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: aa7f28d5-85ed-4573-8f45-d056c46f5372
Move kernel artifacts mounting out of the generic mount flags and into a purpose-built WSLC protocol message.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: aa7f28d5-85ed-4573-8f45-d056c46f5372
Mount the kernel headers directory itself instead of its include subdirectory, share the temporary mount handling between the modules, headers and perf payloads, and reuse the modules mount point for the headers build symlink.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: aa7f28d5-85ed-4573-8f45-d056c46f5372
perf is now added to the default $PATH and only bind mounted over
/usr/bin/perf when the distribution already ships one as a regular file,
so distros without perf no longer get an empty stub created for them.

PERF_EXEC_PATH is also set since perf is built with a prefix that does
not match where it is mounted.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: aa7f28d5-85ed-4573-8f45-d056c46f5372
Allow PERF_EXEC_PATH to be overridden via WSLENV, and make the perf
portion of the KernelArtifacts test clean up on failure paths so that a
leftover /usr/bin/perf can't break subsequent runs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: aa7f28d5-85ed-4573-8f45-d056c46f5372
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: aa7f28d5-85ed-4573-8f45-d056c46f5372
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: aa7f28d5-85ed-4573-8f45-d056c46f5372
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: aa7f28d5-85ed-4573-8f45-d056c46f5372
@benhillis
Ben Hillis (benhillis) force-pushed the user/benhill/module_vhd_artifacts branch from 27a3416 to 4f8fe90 Compare August 11, 2026 16:21
Copilot AI review requested due to automatic review settings August 11, 2026 16:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/linux/init/main.cpp:1633

  • This comment names the wrong mount target: KernelHeadersTarget is /usr/src/linux-headers-<release>, and the test accesses its include child through the build symlink. Saying the mount itself moves to .../include makes the documented flow disagree with the implementation.
    // mount to /usr/src/linux-headers-<uname -r>/include and create the

src/linux/init/main.cpp:3215

  • The deprecated flat-layout fallback is a compatibility promise introduced here, but the updated KernelModules test only verifies the nested default artifact path. Add coverage that boots with a flat module-only VHD and verifies the overlay/module loading path before relying on this fallback for existing custom VHD users.
            const bool LegacyLayout = !NestedLayout && std::filesystem::is_regular_file(ModulesLower + "/modules.dep", Error);

src/linux/init/main.cpp:3210

  • The newly introduced locals in this artifacts-layout block use PascalCase (Release, ArtifactsBase, NestedModules, and others), while the project convention requires camelCase for local variables. Rename them (for example, release and artifactsBase) and update their references.
            const std::string Release{UnameBuffer.release};

            const std::string ArtifactsBase = std::format("{}/{}", KERNEL_MODULES_VHD_PATH, Release);
            const std::string NestedModules = ArtifactsBase + "/modules";

src/linux/init/config.cpp:1205

  • lstat excludes distributions where /usr/bin/perf is a symlink to their packaged perf binary. Because the bundled tools directory is appended to PATH, that symlink continues to resolve first and users run the stale distro perf. Follow the symlink for classification and bind over its resolved regular-file target so the distro filesystem is restored when the mount disappears.
        if ((lstat(PERF_BINARY_PATH, &existing) == 0) && S_ISREG(existing.st_mode))

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