Update scsi host and added scsi devices with scsiinfo module #156
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oracle-contributor-agreement
bot
added
the
OCA Required
oracle-contributor-agreement
bot
added
OCA Verified
biger410
reviewed
May 7, 2025
vashanmu
force-pushed
the
main
branch
3 times, most recently
from
56e6fe2 to
fd4a6b8
Compare
biger410
reviewed
May 16, 2025
There was a problem hiding this comment.
Hi Rajan, thanks for addressing the comments. I have post some more comments, please review, beside that, two generate comments:
- please run gitlab test to run vmcore test
- can you please merge the two patches or at lease make the second patch only for inflight commands, because when i review the first patch, i made some comments, then later when review the second patch, i found the commented code get removed, this happened when i made first round review and this time again, i think this will help other reviewers as well. Thanks.
| "shost_data", | ||
| "hostdata", | ||
| ] | ||
| ] |
There was a problem hiding this comment.
These are host information, it can be dump with print_scsi_hosts to avoid duplicate information. To connect scsi device with host, you can add a column for scsi host address for each device dump, to have device linking to its host information.
drgn_tools/scsi.py
Outdated
|
|
||
| for shost in for_each_scsi_host(prog): | ||
| if host_module_name(shost) == "ahci": | ||
| continue |
There was a problem hiding this comment.
Sorry for the confusion, i mean why skip "ahci" here, but anyway i see you removed it from 2nd patch
|
Ok, I’ll use the version under /drgn_tools/device.py
Thanks,
Rajan
From: Stephen Brennan ***@***.***>
Date: Monday, May 19, 2025 at 11:28 AM
To: oracle-samples/drgn-tools ***@***.***>
Cc: Rajan Shanmugavelu ***@***.***>, Author ***@***.***>
Subject: Re: [oracle-samples/drgn-tools] Update scsi host and added scsi devices with scsiinfo module (PR #156)
@brenns10 commented on this pull request.
________________________________
In drgn_tools/scsi.py<#156 (comment)>:
from drgn import container_of
from drgn import FaultError
from drgn import Object
from drgn import Program
+from drgn.helpers.linux.block import _class_to_subsys
Let's use my version from drgn_tools/device.py. While we will be increasing the minimum supported drgn version to 0.0.31 very soon, the _class_to_subsys function you're importing here is not a public helper in drgn (due to the leading underscore).
We do use private drgn APIs in a few critical places in drgn-tools (namely the slab allocator helpers) but really we want to keep it to a minimum. (Each one is one that I need to monitor for breakages as new drgn changes get merged :P ). This is a small enough (and simple enough) function that I don't mind keeping our own copy. If we want to use the drgn version, we should send a patch to make it a public helper first.
—
Reply to this email directly, view it on GitHub<#156 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIQN7X47D6DRQXWPG7AOWLT27IPDFAVCNFSM6AAAAAB3DWRRNCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQNJRGY3DSMRZGY>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Signed-off-by: Rajan Shanmugavelu <[email protected]
|
Sample devices per SCSI host controller port output, the host data here is just the address which is common for all the devices. |
biger410
reviewed
May 22, 2025
drgn_tools/scsi.py
Outdated
| scsi_disk = cast("struct scsi_disk *", disk.private_data) | ||
| scsi_device = cast("struct scsi_device *", scsi_disk.device) | ||
| if not scsi_disk or not scsi_device: | ||
| continue |
There was a problem hiding this comment.
This still not address? Can you run the helper in an system with nvme disk and use fio to push lots of io to the nvme disk, when disk is nvme disk, all these casting to scsi data structure will cause unexpected result.
I think the right way to do it is using for_each_scsi_host and for_each_scsi_host_device, that will make sure it is only operating on real scsi disk.
|
Hi Junxiao,
Had to make small change to avoid NVMe disks.
diff --git a/drgn_tools/scsi.py b/drgn_tools/scsi.py
index a2119f6..fdcbaad 100644
--- a/drgn_tools/scsi.py
+++ b/drgn_tools/scsi.py
@@ -331,7 +331,8 @@ def print_inflight_scsi_cmnds(prog: Program):
for disk in for_each_disk(prog):
scsi_disk = cast("struct scsi_disk *", disk.private_data)
scsi_device = cast("struct scsi_device *", scsi_disk.device)
- if not scsi_disk or not scsi_device:
+ if not scsi_disk.disk or not scsi_device:
+ # skipping non-SCSI device like NVMe LUNs
continue
counter = 0
output = [
Have run this test on system with NVMe and NVMe-of (over fibrechannel) luns. They don’t show the NVMe luns or the pending commands that are on NVMe channel. It does pick up all SCSI devices that are on fibrechannel, SAS and iSCSI.
Thanks,
Rajan
From: Junxiao Bi ***@***.***>
Date: Thursday, May 22, 2025 at 2:47 PM
To: oracle-samples/drgn-tools ***@***.***>
Cc: Rajan Shanmugavelu ***@***.***>, Author ***@***.***>
Subject: Re: [oracle-samples/drgn-tools] Update scsi host and added scsi devices with scsiinfo module (PR #156)
@biger410 commented on this pull request.
Only one more comment, other looks good.
________________________________
In drgn_tools/scsi.py<#156 (comment)>:
]
)
print_table(output)
+def print_inflight_scsi_cmnds(prog: Program):
+ TotalInflight = 0
+ for disk in for_each_disk(prog):
+ scsi_disk = cast("struct scsi_disk *", disk.private_data)
+ scsi_device = cast("struct scsi_device *", scsi_disk.device)
+ if not scsi_disk or not scsi_device:
+ continue
This still not address? Can you run the helper in an system with nvme disk and use fio to push lots of io to the nvme disk, when disk is nvme disk, all these casting to scsi data structure will cause unexpected result.
I think the right way to do it is using for_each_scsi_host and for_each_scsi_host_device, that will make sure it is only operating on real scsi disk.
—
Reply to this email directly, view it on GitHub<#156 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIQN7X6DYEMXV3MTR2AY3M327ZAU3AVCNFSM6AAAAAB3DWRRNCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQNRSGYZDINBYHA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
vashanmu
force-pushed
the
main
branch
2 times, most recently
from
1c70166 to
20facfa
Compare
Signed-off-by: Rajan <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Rajan Shanmugavelu <[email protected]