cholcombe973 / block-utils

Rust utilities for working with block devices
MIT License
22 stars 17 forks source link

Fix getting properties #23

Closed Apostoln closed 3 years ago

Apostoln commented 3 years ago

Close #22

There was a bug with incorrect syspath for udev. I've fixed it making an assumption we work only with block devices (and return the Error otherwise) and the syspath is always like /sys/class/block/.

I've tested it locally and with the integration tests in our project where we use block-utils. However, it's a bit complicated to write unit tests in the scope of this repo because it requires a kind of testing framework for loop devices, disks, partitions, filesystems, and so on. We have a kind of such one in progress, but it hasn't opensourced yet.

Well, I'm not sure by 100% that the assumption about /sys/class/block is general truth and this path exists for all block devices. There are a lot of other sys paths like /sys/devices/virtual/block/ and /sys/block/. Anyway, it looks like /sys/class/block is an expected syspath according to the examples in udev.

Apostoln commented 3 years ago

I've found some potential problems, please don't merge now.

Apostoln commented 3 years ago

Doesn't matter, it were unrelated problems with Fedora 33.

cholcombe973 commented 3 years ago

This PR looks reasonable to me. Would you like me to merge it?

Apostoln commented 3 years ago

@cholcombe973 Yes, thank you :)