-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add DDF volume system support #403
Conversation
Codecov Report
@@ Coverage Diff @@
## main #403 +/- ##
==========================================
- Coverage 74.04% 74.02% -0.03%
==========================================
Files 257 258 +1
Lines 20550 20605 +55
==========================================
+ Hits 15216 15252 +36
- Misses 5334 5353 +19
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@@ -22,6 +22,8 @@ | |||
"""A lazy import of :mod:`dissect.target.volumes.vmfs`.""" | |||
md = import_lazy("dissect.target.volumes.md") | |||
"""A lazy import of :mod:`dissect.target.volumes.md`.""" | |||
ddf = import_lazy("dissect.target.volumes.ddf") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to open a RAID-0 target consisting of 2 DDF RAID-0 disks with EXT4 trough the multirawloader but it does not seem to work? Is there some wiring missing somewhere?
target-query s1.img+s2.img -f users
s1.img+s2.img: Attempting to use loader: <lazyattr dissect.target.loaders.multiraw.MultiRawLoader loaded=True> [dissect.target.target]
2023-10-18T14:42:59.404293Z [debug ] <Target s1.img+s2.img>: LVM volumes found: [<Volume name=None size=10737418240 fs=None>, <Volume name=None size=10737418240 fs=None>] [dissect.target.target]
2023-10-18T14:42:59.404357Z [debug ] <Target s1.img+s2.img>: Encrypted volumes found: [] [dissect.target.target]
2023-10-18T14:42:59.411051Z [debug ] <Target s1.img+s2.img>: Opened LVM: <DdfVolumeSystem serial=None> [dissect.target.target]
2023-10-18T14:42:59.471100Z [warning ] <Target s1.img+s2.img>: Can't identify filesystem: <Volume name='0' size=21407727616 fs=None> [dissect.target.target]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you share these files with me for debugging?
(DIS-2156) Co-authored-by: pyrco <[email protected]> Co-authored-by: cecinestpasunepipe <[email protected]> Co-authored-by: Erik Schamper <[email protected]>
This function is now mandatory to be implemented by all OSPlugin subclasses. (DIS-2386)
(DIS-2466) --------- Co-authored-by: JSCU-CNI <[email protected]>
vs = volume.open(vol) | ||
except Exception: | ||
# If opening a volume system fails, there's likely none, so open as a filesystem instead | ||
self.open(vol) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend to add a log message here, for diagnostic purposes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically this is the happy path though (it's relatively rare that a volume has another default volume system), so this will result in a lot of unnecessary log messages. Would you still want a log message then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the code then not be restructured to make it reflect this fact better?
Or is that highly impractical?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a suggestion for doing so? Other than adding a comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did some research, I see that it is not possible currently.
vs = volume.open(vol) | ||
except Exception: | ||
# If opening a volume system fails, there's likely none, so open as a filesystem instead | ||
self.open(vol) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the code then not be restructured to make it reflect this fact better?
Or is that highly impractical?
I tried to merge it with the MD volume system but didn't really like the way it ended up, so opted to just make it a separate one for now.
There's a lot of reusable code in the whole volume system stuff anyway, something for a future cleanup perhaps.