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

mimic: krbd: fix rbd map hang due to udev return subsystem unordered #30176

Merged
merged 1 commit into from Oct 8, 2019

Conversation

@smithfarm
Copy link
Contributor

smithfarm commented Sep 5, 2019

@smithfarm smithfarm added this to the mimic milestone Sep 5, 2019
@smithfarm smithfarm added the rbd label Sep 5, 2019
@smithfarm smithfarm requested review from idryomov and dillaman Sep 5, 2019
@@ -222,37 +224,50 @@ static int wait_for_udev_add(struct udev_monitor *mon, const char *pool,
this_image && strcmp(this_image, image) == 0 &&
this_snap && strcmp(this_snap, snap) == 0) {
bus_dev = dev;
continue;
goto check;

This comment has been minimized.

Copy link
@smithfarm

smithfarm Sep 5, 2019

Author Contributor

This conditional block is where I had to resolve conflicts manually, and I'm not super confident the conflict resolution is correct.

This comment has been minimized.

Copy link
@idryomov

idryomov Sep 5, 2019

Contributor

This looks fine.

@@ -222,37 +224,50 @@ static int wait_for_udev_add(struct udev_monitor *mon, const char *pool,
this_image && strcmp(this_image, image) == 0 &&
this_snap && strcmp(this_snap, snap) == 0) {
bus_dev = dev;
continue;
goto check;

This comment has been minimized.

Copy link
@idryomov

idryomov Sep 5, 2019

Contributor

This looks fine.

src/krbd.cc Outdated
assert(!minor ^ have_minor_attr());
for (auto p : block_dev_vec) {
const char *this_major = udev_device_get_property_value(p, "MAJOR");
const char *this_minor = udev_device_get_property_value(p, "MINOR");

if (strcmp(this_major, major) == 0 &&
(!minor || strcmp(this_minor, minor) == 0)) {
string name = get_kernel_rbd_name(udev_device_get_sysname(bus_dev));

assert(strcmp(udev_device_get_devnode(dev), name.c_str()) == 0);

This comment has been minimized.

Copy link
@idryomov

idryomov Sep 5, 2019

Contributor

This is wrong: the original patch changes udev_device_get_devnode(dev) to udev_device_get_devnode(p).

This comment has been minimized.

Copy link
@smithfarm

smithfarm Sep 5, 2019

Author Contributor

oops! fixing

The order of subsystem returned by udev_device_get_subsystem
might not be same order as adding subsystem by
udev_monitor_filter_add_match_subsystem_devtype. So if block
event is returned first and rbd event is returned next, then
further poll will get nothing back until timed-out.

Fixes: http://tracker.ceph.com/issues/39089

Signed-off-by: Zhi Zhang <zhangz.david@outlook.com>
(cherry picked from 42dd1ea)

Conflicts:
        src/krbd.cc
@smithfarm smithfarm force-pushed the smithfarm:wip-39316-mimic branch from 41b3766 to 2cdf092 Sep 5, 2019
@smithfarm

This comment has been minimized.

Copy link
Contributor Author

smithfarm commented Sep 5, 2019

@idryomov Please have a second look. Thanks.

Copy link
Contributor

idryomov left a comment

LGTM

@yuriw

This comment has been minimized.

Copy link
Contributor

yuriw commented Oct 4, 2019

@yuriw yuriw merged commit cef2e73 into ceph:mimic Oct 8, 2019
4 checks passed
4 checks passed
Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
@smithfarm smithfarm deleted the smithfarm:wip-39316-mimic branch Oct 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.