Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os/bluestore: fix NVMEDevice::open failure if serial number ends with a … #12956

Merged
merged 2 commits into from Jan 24, 2017

Conversation

liuhongtong
Copy link

…number

The serial number consists of 16 hexadecimal characters.
If the serial number ends with a number, manager.try_get()
will deliver the truncated serial number.

Signed-off-by: Hongtong Liu hongtong.liu@istuary.com

@liuhongtong
Copy link
Author

@yuyuyu101 @optimistyzy Please take a look. Thanks.

@yuyuyu101
Copy link
Member

could you give a example?

@liuhongtong
Copy link
Author

liuhongtong commented Jan 17, 2017

@yuyuyu101

The serial number I get:
lspci -vvv -d 8086:0953 | grep "Device Serial Number"
Capabilities: [270 v1] Device Serial Number 55-cd-2e-41-4c-93-a4-e2

If without my fix, error will occur:
bdev probe_cb device serial number (55cd2e414c93a4e) not match 55cd2e414c93a4e2
bdev() open failed to get nvme device with sn 55cd2e414c93a4e
OSD::mkfs: ObjectStore::mkfs failed with error -1
^[[0;31m ** ERROR: error creating empty object store in /var/lib/ceph/osd/osd-device-0-data: (1) Operation not permitted

@@ -798,7 +799,7 @@ int NVMEDevice::open(string p)
derr << __func__ << " unable to read " << p << ": " << cpp_strerror(r) << dendl;
return r;
}
while (r > 0 && !isalpha(buf[r-1])) {
while (r > 0 && !isxdigit(buf[r-1])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if digit is at the ending?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suppose that buf is "55cd2e414c93a4e2" and r is 16, the length of buf.
In the 'while' loop, --r set r to be 15, and the number "2" is ignored, then serial_number will be "55cd2e414c93a4e", which as the first parameter of manager.try_get().
As you know, after a long procedure, probe_cb() gets the same serial number("55cd2e414c93a4e2") via the interface of spdk_pci_device_get_serial_number(),
but now "55cd2e414c93a4e" is not equal to "55cd2e414c93a4e2", probe_cb() returns false.

@liewegas liewegas changed the title bluestore: fix NVMEDevice::open failure if serial number ends with a … os/bluestore: fix NVMEDevice::open failure if serial number ends with a … Jan 17, 2017
@@ -798,7 +798,7 @@ int NVMEDevice::open(string p)
derr << __func__ << " unable to read " << p << ": " << cpp_strerror(r) << dendl;
return r;
}
while (r > 0 && !isalpha(buf[r-1])) {
while (r > 0 && !isxdigit(buf[r-1])) {
Copy link
Contributor

@ifed01 ifed01 Jan 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't be the better approach to scan buf from the beginning while isxdigit? This will protect from all the potential garbage that might be at the end of the read buffer after the serial number.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better implementation.

@liuhongtong
Copy link
Author

@liupan1111 please take a look.

@liupan1111
Copy link
Contributor

@liuhongtong , please modify your commit as the suggestion from
ifed01

Hongtong Liu added 2 commits January 22, 2017 17:25
… a number

buf in effect is the serial number in ceph.conf and
the serial number consists of 16 hexadecimal characters.
1. In order to avoid ignoring the numbers, scan buf
with isxdigit.
2. In order to ignore all the potential garbage,
scan buf from the beginning.

Signed-off-by: Hongtong Liu <hongtong.liu@istuary.com>
Fix assert failure in ~Task().

Signed-off-by: Hongtong Liu <hongtong.liu@istuary.com>
@liuhongtong
Copy link
Author

@ifed01 @liupan1111 modified.

@liupan1111
Copy link
Contributor

lgtm

@liuhongtong
Copy link
Author

@yuyuyu101 please merge it.

@yuyuyu101 yuyuyu101 merged commit b2df030 into ceph:master Jan 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants