Skip to content

chore(hyperv): add local disk deep inspection API - #12

Merged
solenoci merged 2 commits into
kubev2v:mainfrom
Hazanel:hyperv-local-deep-inspection
Sep 8, 2026
Merged

chore(hyperv): add local disk deep inspection API#12
solenoci merged 2 commits into
kubev2v:mainfrom
Hazanel:hyperv-local-deep-inspection

Conversation

@Hazanel

@Hazanel Hazanel commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add DetectLocal() so callers can run virt-inspector on locally mounted VHD/VHDX files (no VDDK or vSphere snapshot).
  • Relax NewDetector() so Hyper-V / local-path use does not require vCenter credentials.
  • Forklift Hyper-V preflight (MTV-6718 | add two-phase VHDX transfer and local deep inspection forklift#8378) currently hand-patches vendor/, this is the real module change that PR should consume.

Test plan

  • go test ./pkg/vmdetect/ ./internal/inspection/ ./internal/checks/
  • Forklift Hyper-V deep inspection against an SMB-mounted VHDX after bumping the module

Summary by CodeRabbit

  • New Features

    • Added local disk detection without requiring vSphere credentials or VDDK configuration.
    • Added support for inspecting locally mounted disks and reporting operating systems, applications, filesystems, and mount points.
    • Added disk format handling for local detection.
  • Bug Fixes

    • Improved inspection error handling, including encrypted-disk failures and interrupted operations.

Add DetectLocal() to the Detector that runs virt-inspector directly on
locally-mounted VHD/VHDX files without requiring VDDK or vSphere
snapshots. Used by Forklift for Hyper-V cold preflight inspection via
SMB-mounted disks.

Changes:
- pkg/vmdetect: add DetectLocal() and DetectLocalParams, relax
  NewDetector() credential validation for local-path usage
- internal/inspection: add InspectLocal() to VirtInspector for running
  virt-inspector on local disk files with proper format/arg ordering
- internal/persistent: add InspectLocal() to InspectorInterface and
  Inspector implementation

Signed-off-by: Elad Hazan <ehazan@redhat.com>
Cover local-path Detector construction, required DetectLocal
params, and virt-inspector --format-before -a argument order.

Signed-off-by: Elad Hazan <ehazan@redhat.com>
@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 6a55e6df-dfef-41fc-be23-f2210bbc2095

📥 Commits

Reviewing files that changed from the base of the PR and between 30e22fb and 88b13a6.

📒 Files selected for processing (4)
  • internal/inspection/virt_inspector.go
  • internal/persistent/inspection.go
  • pkg/vmdetect/runner.go
  • pkg/vmdetect/runner_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The PR adds local disk inspection through virt-inspector, exposes it through the persistent inspection interface, and adds local detection that validates fstab data and extracts inspection results. Detector creation now supports empty vSphere configuration for local detection.

Changes

Local disk detection

Layer / File(s) Summary
Local inspection pipeline
internal/inspection/virt_inspector.go, internal/persistent/inspection.go
The inspection interfaces and implementations add InspectLocal. The method runs virt-inspector on local disk arguments, handles encrypted-disk and command failures, parses inspection XML, and uses standard error handling for canceled or timed-out commands.
Local detection flow
pkg/vmdetect/runner.go, pkg/vmdetect/runner_test.go
NewDetector allows local detection without vSphere credentials. DetectLocal validates inputs, builds format-aware disk arguments, runs inspection, validates fstab data, and populates DetectResult. Tests cover configuration validation, input validation, and argument formatting.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to 88b13

This change allows local disk images to be inspected without vCenter credentials, which can expose metadata from any compatible file readable by the inspection process if an integration accepts untrusted paths. The PR is mergeable with explicit owner awareness that callers must authenticate requests, bind paths to the intended VM or tenant, and restrict access to approved disk locations.

Suggested reviewers: nirarg

Sequence Diagram(s)

sequenceDiagram
  participant DetectLocal
  participant Inspector
  participant VirtInspector
  participant virt-inspector
  DetectLocal->>Inspector: pass local virt-inspector arguments
  Inspector->>VirtInspector: InspectLocal(ctx, virtInspectorArgs)
  VirtInspector->>virt-inspector: inspect local disk files
  virt-inspector-->>VirtInspector: return inspection XML or error
  VirtInspector-->>Inspector: parse XML or return failure
  Inspector-->>DetectLocal: return inspection result
  DetectLocal->>DetectLocal: validate fstab and build DetectResult
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a local disk deep inspection API for Hyper-V.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@solenoci
solenoci merged commit a894471 into kubev2v:main Sep 8, 2026
6 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.

2 participants