Add libvirt loader and qemu child plugin #654
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #654 +/- ##
==========================================
+ Coverage 74.83% 74.86% +0.03%
==========================================
Files 288 290 +2
Lines 24024 24058 +34
==========================================
+ Hits 17978 18012 +34
Misses 6046 6046
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
dissect/target/loaders/libvirt.py
Outdated
| if path.suffix.lower() != ".xml": | ||
| return False | ||
|
|
||
| return "<domain>" in path.read_text().lower() |
There was a problem hiding this comment.
Is this the first element? Maybe better to limit how many bytes you read to a sensible limit. You may accidentally read a .xml of several GB large.
dissect/target/loaders/libvirt.py
Outdated
| xml_data = ElementTree.XML(self.path.read_text()) | ||
| for disk in xml_data.findall("devices/disk/source"): | ||
| if file := disk.get("file"): | ||
| target.disk.add(container.open(file)) |
There was a problem hiding this comment.
Are these absolute paths? You should join on self.path regardless to ensure that you're reading the file from the same target (local Path or a bound TargetPath).
There was a problem hiding this comment.
As far as I've seen, these paths are absolute. But it is fair it should be joined with self.path
Miauwkeru
force-pushed
the
653-add-support-for-libvirtqemu-child-targets
branch
from
78ff93c
to
9ae003b
Compare
| return False | ||
|
|
||
| with path.open("rb") as fh: | ||
| return b"<domain>" in fh.read(512) |
There was a problem hiding this comment.
This doesn't seem to match the qemu.xml that you included in the test file. Also <domain> seems a little too generic, can you make this check more strict? Maybe include some of the other elements that are common/required in a libvirt XML file.
| xml_data = ElementTree.XML(self.path.read_text()) | ||
| for disk in xml_data.findall("devices/disk/source"): | ||
| if file := disk.get("file"): | ||
| target_file = self.path.joinpath(file) |
There was a problem hiding this comment.
Another question - in a common libvirt setup, is there a relation between where the .xml file is and where the virtual disk files are? E.g. are they in the same directory?
I'm asking because sometimes we get a bunch of files in a single directory as "evidence". In that case, we may want to check if a file with the disk name exists in the same directory as the .xml file. The original absolute path is usually only checked as a last resort. This is also done in all similar loaders (.vmx etc)
|
|
||
|
|
||
| @patch(f"{LibvirtLoader.__module__}.container.open") | ||
| def test_libvirt_map(mocked_container_open: Mock, target_bare: Target, fs_linux: VirtualFilesystem): |
There was a problem hiding this comment.
| def test_libvirt_map(mocked_container_open: Mock, target_bare: Target, fs_linux: VirtualFilesystem): | |
| def test_libvirt_map(mocked_container_open: Mock, target_bare: Target, fs_linux: VirtualFilesystem) -> None: |
| from tests.conftest import absolute_path | ||
|
|
||
|
|
||
| def test_libvirt_detection(target_bare: Target, tmp_path: Path) -> None: |
There was a problem hiding this comment.
Why not also test against the actual qemu.xml you included as test file?
| QemuChildTargetPlugin(target_linux).check_compatible() | ||
|
|
||
|
|
||
| def test_list_children(target_linux: Target, fs_linux: VirtualFilesystem): |
There was a problem hiding this comment.
| def test_list_children(target_linux: Target, fs_linux: VirtualFilesystem): | |
| def test_list_children(target_linux: Target, fs_linux: VirtualFilesystem) -> None: |
No description provided.