ceph-disk: fix signed integer is greater than maximum when call major #19196
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
tchaikov
requested changes
Nov 28, 2017
src/ceph-disk/ceph_disk/main.py
Outdated
| @@ -603,7 +603,8 @@ def block_path(dev): | |||
| return dev | |||
| path = os.path.realpath(dev) | |||
| rdev = os.stat(path).st_rdev | |||
| (M, m) = (os.major(rdev), os.minor(rdev)) | |||
| M = ((rdev >> 8) & 0xfff) | ((rdev >> 32) & ~0xfff) | |||
There was a problem hiding this comment.
i'd suggest monkey patching os.major() and os.minor(). in other words, add following code snippet at the beginning of main.py, right before class Ptype(object)
try:
# see https://bugs.python.org/issue23098
os.major(0x80002b00)
except OverflowError:
os.major = lambda devid: ((devid >> 8) & 0xfff) | ((devid >> 32) & ~0xfff)
os.minor = lambda devid: (devid & 0xff) | ((devid >> 12) & ~0xff)There was a problem hiding this comment.
@tchaikov , thanks your detailed instruction. ^^
i've done with above monkey patch, please review again.
There was a problem hiding this comment.
please drop this change. as it is not necessary anymore.
There was a problem hiding this comment.
no, you reverted the change i suggested.
shun-s
force-pushed
the
wip-cephdisk-fix-major-error
branch
from
0805e8d
to
61294ef
Compare
tchaikov
requested changes
Nov 28, 2017
src/ceph-disk/ceph_disk/main.py
Outdated
| @@ -603,7 +603,8 @@ def block_path(dev): | |||
| return dev | |||
| path = os.path.realpath(dev) | |||
| rdev = os.stat(path).st_rdev | |||
| (M, m) = (os.major(rdev), os.minor(rdev)) | |||
| M = ((rdev >> 8) & 0xfff) | ((rdev >> 32) & ~0xfff) | |||
There was a problem hiding this comment.
please drop this change. as it is not necessary anymore.
shun-s
force-pushed
the
wip-cephdisk-fix-major-error
branch
3 times, most recently
from
35775eb
to
f77934b
Compare
fix signed integer is greater than maximum when call os.major using python 2.7.5 in Centos 7 Signed-off-by: Song Shun <song.shun3@zte.com.cn>
|
@tchaikov , i think this time i totally get what you mean, please review. |
tchaikov
approved these changes
Nov 30, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix signed integer is greater than maximum when call os.major
using python 2.7.5 in Centos 7
when calling os.major in block_path, it may broken due to already exist bugs in python 2.7.5 [1].
As you see, this bug has been fixed in python after 2.7.14 (also teseted in my enviroment), just upgradeing python is Ok.
but when i think 2.7.5 is largely used in Centos 7, upgrading could have influenced to other applications.
so i think implementing os.major and os.minor like major and minor in glibc [2] mayb be a good idea.
but i'm not sure that if there is any other problems? please take a review, thanks
[1] Dan MacDonald told me that "os.mknod()" should accept devices >32 bits.
https://bugs.python.org/issue23098
[2] https://github.com/lattera/glibc/blob/master/sysdeps/unix/sysv/linux/sys/sysmacros.h
Signed-off-by: Song Shun song.shun3@zte.com.cn