Properly detect dm devices #301
Conversation
|
@avishayt I apologize for the delay in getting to these reviews, sir. Is there a reason you closed out your PRs? |
|
Wasn't sure the project was still active :) |
There was a problem hiding this comment.
thanks for the change! the code per se looks good! It would be real nice to have some tests to cover this change, in order to avoid future breakage.
It would be also real nice to have an example of the sysfs directory tree involved here.
Perhaps ghw-snapshot can help to capture that?
Fixes: jaypipes#298 Signed-off-by: Avishay Traeger <avishay@redhat.com>
|
Added unit tests |
| @@ -166,6 +169,7 @@ type Disk struct { | |||
| SerialNumber string `json:"serial_number"` | |||
| WWN string `json:"wwn"` | |||
| Partitions []*Partition `json:"partitions"` | |||
| Members []string `json:"members"` | |||
There was a problem hiding this comment.
LGTM but let's hear from @jaypipes because of the API-level change
| @@ -271,7 +284,7 @@ func disks(ctx *context.Context, paths *linuxpath.Paths) []*Disk { | |||
| driveType, storageController := diskTypes(dname) | |||
| // TODO(jaypipes): Move this into diskTypes() once abstracting | |||
| // diskIsRotational for ease of unit testing | |||
| if !diskIsRotational(ctx, paths, dname) { | |||
| if driveType != DRIVE_TYPE_MAPPER && !diskIsRotational(ctx, paths, dname) { | |||
There was a problem hiding this comment.
we will probably need to review this flow more deeply, but that's totally material for another PR. This change LGTM for this PR
Fixes: #298