lib/host: Added raw_disk_is_available() using blkid for reliable raw disk checks.
#301
Conversation
|
I fear regressions here. We use and reuse the same disks over and over in CI. After a test, they do have contents. We don't wipe them fresh everytime we unmount them. |
Do you mean that the content is "required to be kept" when unmounting or it's just not cleaned in those CI? May be a Else we could introduce a different function |
|
It's just not cleaned, and we didn't have a reason to enforce that the disk be void of anything before starting tests. The SR creation takes care of overwriting what exists. That's why an available disk is defined as a disk that is not mounted. If XOSTOR has different requirements, maybe it should use a separate fixture? |
rushikeshjadhav
force-pushed
the
issue-276-available-disk
branch
from
84039b3 to
1451730
Compare
|
In XOSTOR, both used and unused disks are not mounted, so while finding a free disk on server that has "used" disk(s) during pool expansion, the current |
rushikeshjadhav
force-pushed
the
issue-276-available-disk
branch
2 times, most recently
from
9232983 to
8498419
Compare
lib/host.py
Outdated
| @@ -516,10 +516,12 @@ def disks(self): | |||
| disks.sort() | |||
| return disks | |||
|
|
|||
| def disk_is_available(self, disk): | |||
| def disk_is_available(self, disk, disk_raw=False): | |||
There was a problem hiding this comment.
I'm not sure this new param is explicit for everyone.
I prefer something like: check_unformatted, check_no_fs or check_wiped.
There was a problem hiding this comment.
I also find its naming unclear
There was a problem hiding this comment.
We can rename the parameter from disk_raw to check_raw, else for more elaborate use type option can be introduced. Where type can be raw, LVM2_member, ext3, part etc. and it can be found from blkid or suitable disk management command based on type.
e.g disk_is_available(self, disk, type='raw')
There was a problem hiding this comment.
Could be interesting to have but I think it should be a different function that the parameter check_raw could use and keep it a bool.
There was a problem hiding this comment.
I have added raw_disk_is_available() and kept the current function untouched.
rushikeshjadhav
force-pushed
the
issue-276-available-disk
branch
from
8498419 to
c06bae7
Compare
rushikeshjadhav
changed the title
disk_is_available() using blkid for reliable disk checks.raw_disk_is_available() using blkid for reliable raw disk checks.
lib/host.py
Outdated
| def raw_disk_is_available(self, disk): | ||
| return self.ssh_with_result(['blkid', '/dev/' + disk]).returncode == 2 | ||
|
|
There was a problem hiding this comment.
A docstring explaining what this new method wrt disk_is_available would be useful, as well as one for the latter for easier comparison.
It could also be good to add type hints on such new functions.
…hecks. Added docstring explaining what this new method does vs `disk_is_available`. Added type hints. Signed-off-by: Rushikesh Jadhav <[email protected]>
rushikeshjadhav
force-pushed
the
issue-276-available-disk
branch
from
c06bae7 to
72bb94a
Compare
An issue (#276) occurred while expanding a pool using/finding free disks of the hosts (PR #282), hence fixing with this PR.
blkidreturns 2 if there is nothing on the disk e.g. LVM, gpt, etc. hence markingdisk_is_available.