Bug 2215328

Summary: thin_rmap should fail and return an error message when using invalid range
Product: Red Hat Enterprise Linux 9 Reporter: Filip Suba <fsuba>
Component: device-mapper-persistent-dataAssignee: Ming-Hung Tsai <mtsai>
Status: ASSIGNED --- QA Contact: Filip Suba <fsuba>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 9.3CC: agk, heinzm, lvm-team, msnitzer, mtsai, thornber
Target Milestone: rcKeywords: Triaged
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Filip Suba 2023-06-15 14:36:57 UTC 
Description of problem:
thin_rmap returns 0 when using invalid range. It should return 1 and an error message. 


Version-Release number of selected component (if applicable):
device-mapper-persistent-data-1.0.4-1.el9


How reproducible:
always


Steps to Reproduce:
1. thin_rmap /dev/mapper/vgtest-swapvol --region 10..0

Actual results:
thin_rmap returns 0 and no output

Expected results:
Should return 1 for using invalid range and print error message similar to one in 0.9.0 version


Additional info:
In 0.9.0 thin_rmap returns following error message for invalid regions:

# thin_rmap /dev/mapper/vgtest-swapvol --region 0..0
badly formed region (end <= begin)

# thin_rmap /dev/mapper/vgtest-swapvol --region 10..0
badly formed region (end <= begin)
Comment 1 Ming-Hung Tsai 2023-07-11 12:17:47 UTC 
Send a patch upstream: https://github.com/jthornber/thin-provisioning-tools/commit/aa5a10af

Now the Rust version validates the range size (i.e., end > begin or not), and prohibits negative numbers (in contrast, the v0.9.0 allows negative numbers by type casting). Here are some negative test cases: 

thin_rmap --region 0..0   // end <= begin
thin_rmap --region 10..0  // end <= begin
thin_rmap --region -1..0  // negative begin
thin_rmap --region 0..-1  // negative end

Does that match our conclusions before?
Comment 2 Filip Suba 2023-07-11 14:43:10 UTC 
Yes, it matches what we've concluded.
Comment 3 Ming-Hung Tsai 2023-07-24 17:33:46 UTC 
Update: correct the commit id
https://github.com/jthornber/thin-provisioning-tools/commit/185cf53c15

and there are unit tests (via cargo test [--release] --test lib)
```
test commands::utils::range_parsing_tests::test_invalid_end ... ok
test commands::utils::range_parsing_tests::test_dots_only ... ok
test commands::utils::range_parsing_tests::test_invalid_begin ... ok
test commands::utils::range_parsing_tests::test_end_not_greater_than_begin ... ok
test commands::utils::range_parsing_tests::test_negative_begin ... ok
test commands::utils::range_parsing_tests::test_extra_characters ... ok
test commands::utils::range_parsing_tests::test_negative_end ... ok
test commands::utils::range_parsing_tests::test_normal ... ok
test commands::utils::range_parsing_tests::test_no_dots ... ok
```