Skip to content

fix(probe): fetch block size and drive type for partitions#537

Merged
akhilerm merged 1 commit intoopenebs-archive:masterfrom
z0marlin:sysfs-partiion-fix
Jan 25, 2021
Merged

fix(probe): fetch block size and drive type for partitions#537
akhilerm merged 1 commit intoopenebs-archive:masterfrom
z0marlin:sysfs-partiion-fix

Conversation

@z0marlin
Copy link
Copy Markdown
Contributor

@z0marlin z0marlin commented Jan 18, 2021

sysfsprobe fetches logical/physical blocksize, hw sector size,
and drive type from sysfs using <syspath>/queue directory.
This directory is available only for parent disks and not partitions.
These details for partitions are same as their parent disks. Use
their parent disks to fetch the details.

Testing

Manually tested by running in a minikube cluster
Screenshot from 2021-01-20 17-15-00

Checklist:

@akhilerm akhilerm requested review from akhilerm and removed request for akhilerm January 20, 2021 15:19
@z0marlin z0marlin marked this pull request as ready for review January 20, 2021 15:30
Comment thread cmd/ndm_daemonset/probe/sysfsprobe.go Outdated
klog.Errorf("unable to get sysfs device for device: %s, err: %v", blockDevice.DevPath, err)
return
}
pSysFsDevice := sysFsDevice
Copy link
Copy Markdown
Contributor

@akhilerm akhilerm Jan 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this assignment done? If the Parent device is present it can be assigned to sysFsDevice after https://github.com/openebs/node-disk-manager/pull/537/files#diff-8ab703382db619675921914253cefd11e57af1e4dd2a260aa24bfa029da7ad0fR107

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because sysFsDevice is still being used to calculate the capacity here - https://github.com/openebs/node-disk-manager/blob/507409ecc94216ddc467a38e84c1899e4228da0c/cmd/ndm_daemonset/probe/sysfsprobe.go#L158
So a reference to the original blockdevice is still needed

Copy link
Copy Markdown
Contributor Author

@z0marlin z0marlin Jan 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another way would be to move the call which requires a reference to the original blockdevice above all the other calls, and reassign sysFsDevice after that

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another way would be to move the call which requires a reference to the original blockdevice above all the other calls, and reassign sysFsDevice after that

@z0marlin , I thnk this method ^^^ will be better. Anything that requires parent device comes after reassining. Also can you rename psysfsDevice to parentSysFSDevice and the same with pDev to parentDev

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread cmd/ndm_daemonset/probe/sysfsprobe.go Outdated
Copy link
Copy Markdown
Contributor

@akhilerm akhilerm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@z0marlin Have given some comments.

@z0marlin z0marlin force-pushed the sysfs-partiion-fix branch 2 times, most recently from f071c20 to f58a5c9 Compare January 21, 2021 18:24
sysfsprobe fetches logical/physical blocksize, hw sector size,
and drive type from sysfs using `<syspath>/queue` directory.
This directory is available only for parent disks and not partitions.
These details for partitions are same as their parent disks. Use
their parent disks to fetch the details.

Fixes openebs/openebs#3134

Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
Copy link
Copy Markdown
Contributor

@akhilerm akhilerm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@akhilerm akhilerm merged commit 6bfbfe7 into openebs-archive:master Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support to fetch block size and drive type of partitions from parent device

3 participants