-
Notifications
You must be signed in to change notification settings - Fork 19
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
dm: add dmsetup table support for linear target #56
Conversation
Here is an example of the output: >>> dm.show_dm_table(prog) ocivolume-oled: 0 20971520 linear 8:3 [sda3] 2048 ocivolume-root: 0 60940288 linear 8:3 [sda3] 20973568 Signed-off-by: Junxiao Bi <[email protected]>
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.
This looks good to me. I made a few notes but nothing worth holding up merging this on.
disk_name = bdev.bd_disk.disk_name.string_().decode() | ||
if partno == 0: | ||
return disk_name | ||
if disk_name[len(disk_name) - 1].isdigit(): |
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.
Python has a neat feature where you can index with a negative integer, and it will instead index backwards from the end of whatever you're indexing. This means you can omit len(disk_name)
here, it's quite nice:
if disk_name[len(disk_name) - 1].isdigit(): | |
if disk_name[-1].isdigit(): |
Anyway, this is just a note for the future :) Not something to block the PR on!
table_type = dm.prog_.type( | ||
"struct dm_table *", "drivers/md/dm-table.c" | ||
) |
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.
Oof. This is quite frustrating, especially because CTF has no way to represent the filename where a type is declared. It resolves name collisions in its own way, which has to do with kernel module names, but it's not perfect either.
Unfortunately there's not much to do about this in the context of this PR. It's merely another concern for me to keep in mind for CTF.
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.
Interestingly enough, I took a look at the UEK4 CTF archives, and found that the "undefined" variant of dm_table is not even included in the CTF archive, since it never gets used by anything. I guess CTF is "smart enough" to handle this situation.
Also, UEK4 is not currently planned to be supported for CTF, so that's not much of an issue anyway.
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.
CTF is awesome!
|
||
def dm_target_name(dm: Object) -> str: | ||
table = dm_table(dm) | ||
if table.value_() == 0x0: |
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.
Again, another note that for NULL testing, you can simply do if not table
. It's not a big deal though, I wouldn't block the PR over it.
Thanks for the review. I will put those changes in my next PR with dm. |
Here is an example of the output:
I would like this gets reviewed/merged before multipath pull request, there were multiple generic dm helpers that can be reused by multipath helpers.
Only linear mapping is supported, i would like support multipath target after multipath helpers get merged, also i may add snapshot target support in future.