Bug 838257
| Summary: | lvm2app.h defines uint64_t for signed properties | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Red Hat Enterprise Linux 6 | Reporter: | benscott | ||||||
| Component: | lvm2 | Assignee: | Tony Asleson <tasleson> | ||||||
| Status: | CLOSED ERRATA | QA Contact: | cluster-qe <cluster-qe> | ||||||
| Severity: | low | Docs Contact: | |||||||
| Priority: | unspecified | ||||||||
| Version: | 6.5 | CC: | agk, dwysocha, heinzm, jbrassow, msnitzer, prajnoha, prockai, rbednar, thornber, zkabelac | ||||||
| Target Milestone: | rc | ||||||||
| Target Release: | --- | ||||||||
| Hardware: | i686 | ||||||||
| OS: | Linux | ||||||||
| Whiteboard: | |||||||||
| Fixed In Version: | lvm2-2.02.140-1.el6 | Doc Type: | Bug Fix | ||||||
| Doc Text: | Story Points: | --- | |||||||
| Clone Of: | Environment: | ||||||||
| Last Closed: | 2016-05-11 01:19:34 UTC | 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: | |||||||||
| Attachments: |
|
||||||||
The following is also in lvm2app.h
/**
* This type defines a couple of special percent values. The PERCENT_0 and
* PERCENT_100 constants designate *exact* percentages: values are never
* rounded to either of these two.
*/
typedef enum {
PERCENT_0 = 0,
PERCENT_1 = 1000000,
PERCENT_100 = 100 * PERCENT_1,
PERCENT_INVALID = -1,
PERCENT_MERGE_FAILED = -2
} percent_range_t;
typedef int32_t percent_t;
This means that -1 or -2 may be returned by properties that are percentages.
How about we extend this by doing something like:
typedef struct lvm_property_value {
uint32_t is_settable:1;
uint32_t is_string:1;
uint32_t is_integer:1;
uint32_t is_valid:1;
uint32_t is_signed:1;
uint32_t padding:27;
union {
const char *string;
uint64_t integer;
int64_t signed_integer;
} value;
} lvm_property_value_t;
This should preserve backwards compatibility and provide a signed value. We could also add data type for double precision floating-point, although I'm not sure we have a need for it at this time.
Some of the values returned don't need 8 bytes for representation, but I don't think we improve things by adding numerous types for the different integer sizes.
Patches committed upstream for this: https://www.redhat.com/archives/lvm-devel/2015-May/msg00059.html https://www.redhat.com/archives/lvm-devel/2015-May/msg00060.html Created attachment 1083589 [details]
Test code for lvm_lv_get_property lvm2app fn
Test code using lvm2app and its lvm_lv_get_property to get integer property values is attached.
Compile with: gcc -llvm2app test_new.c -o test_new
Then test with: ./test_new vg_name lv_name lv_integer_field
For example:
$ ./test vg lvol0 lv_major
"lv_major" property value for vg/lvol0 is signed integer.
Found "lv_major" property for vg/lvol0: -1
$ ./test vg lvol0 lv_size
"lv_size" property value for vg/lvol0 is unsigned integer.
Found "lv_size" property for vg/lvol0: 4194304
Created attachment 1083602 [details]
Test code for lvm_lv_get_property lvm2app fn (old interface)
And the code for the older interface (the test_new.c code is not compilable with older lvm2app library!).
$ gcc -llvm2app test_old.c -o test_old
$ ./test_old vg lvol0 lv_major
Found "lv_major" property for vg/lvol0: 18446744073709551615
$ ./test_old vg lvol0 lv_size
Found "lv_size" property for vg/lvol0: 4194304
(So you can see that in the older version, the value is considered unsinged and hence it's incorrectly printed as unsigned number instead of signed one.)
Verified. # ./test_new vg_virt175 lv_root lv_major "lv_major" property value for vg_virt175/lv_root is signed integer. Found "lv_major" property for vg_virt175/lv_root: -1 # ./test_new vg_virt175 lv_root lv_size "lv_size" property value for vg_virt175/lv_root is unsigned integer. Found "lv_size" property for vg_virt175/lv_root: 7436500992 # ./test_old vg_virt175 lv_root lv_major Found "lv_major" property for vg_virt175/lv_root: 18446744073709551615 # ./test_old vg_virt175 lv_root lv_size Found "lv_size" property for vg_virt175/lv_root: 7436500992 Tested on: 2.6.32-616.el6.x86_64 lvm2-2.02.141-2.el6 BUILT: Wed Feb 10 14:49:03 CET 2016 lvm2-libs-2.02.141-2.el6 BUILT: Wed Feb 10 14:49:03 CET 2016 lvm2-cluster-2.02.141-2.el6 BUILT: Wed Feb 10 14:49:03 CET 2016 udev-147-2.71.el6 BUILT: Wed Feb 10 14:07:17 CET 2016 device-mapper-1.02.115-2.el6 BUILT: Wed Feb 10 14:49:03 CET 2016 device-mapper-libs-1.02.115-2.el6 BUILT: Wed Feb 10 14:49:03 CET 2016 device-mapper-event-1.02.115-2.el6 BUILT: Wed Feb 10 14:49:03 CET 2016 device-mapper-event-libs-1.02.115-2.el6 BUILT: Wed Feb 10 14:49:03 CET 2016 device-mapper-persistent-data-0.6.2-0.1.rc1.el6 BUILT: Wed Feb 10 16:52:15 CET 2016 Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://rhn.redhat.com/errata/RHBA-2016-0964.html |
lvs --version LVM version: 2.02.95(2) (2012-03-06) Library version: 1.02.74 (2012-03-06) Driver version: 4.22.0 Description of problem: in "lvm2app.h" there is the following: typedef struct lvm_property_value { uint32_t is_settable:1; uint32_t is_string:1; uint32_t is_integer:1; uint32_t is_valid:1; uint32_t padding:28; union { const char *string; uint64_t integer; } value; } lvm_property_value_t; However the properties lv_minor and lv_major may return a negative numer: #lvs -o+lv_minor,lv_major LV VG Attr LSize Pool Origin Data% Move Log Copy% Convert Min Maj lvol0 big -wi-a--- 8.92g -1 -1 As a result the definition "uint64_t integer" garbles the number unless cast to a signed int. benscott