Bug 838257 - lvm2app.h defines uint64_t for signed properties
lvm2app.h defines uint64_t for signed properties
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: lvm2 (Show other bugs)
6.5
i686 Linux
unspecified Severity low
: rc
: ---
Assigned To: Tony Asleson
cluster-qe@redhat.com
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-07 19:49 EDT by benscott
Modified: 2016-05-10 21:19 EDT (History)
10 users (show)

See Also:
Fixed In Version: lvm2-2.02.140-1.el6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-05-10 21:19:34 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Test code for lvm_lv_get_property lvm2app fn (1.28 KB, text/plain)
2015-10-16 07:34 EDT, Peter Rajnoha
no flags Details
Test code for lvm_lv_get_property lvm2app fn (old interface) (1.01 KB, text/plain)
2015-10-16 07:38 EDT, Peter Rajnoha
no flags Details

  None (edit)
Description benscott 2012-07-07 19:49:19 EDT
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@nwlink.com
Comment 2 benscott 2012-10-01 01:22:41 EDT
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.
Comment 4 Tony Asleson 2013-12-05 16:16:09 EST
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.
Comment 11 Peter Rajnoha 2015-10-16 07:34 EDT
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
Comment 12 Peter Rajnoha 2015-10-16 07:38 EDT
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.)
Comment 16 Roman Bednář 2016-02-24 06:28:02 EST
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
Comment 18 errata-xmlrpc 2016-05-10 21:19:34 EDT
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

Note You need to log in before you can comment on or make changes to this bug.