ceph-disk: partprobe should block udev induced BLKRRPART

[ghost]

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

[idryomov]

I would suggest a shorter sleep and more iterations, at least 50/5 or 100/2-3. Given that this race is so easy to hit that you thought the device had stayed busy, 25 might not cut it. I also think it's worth printing an info message on each 10th or 20th iteration.

[ghost]

@idryomov thanks for your quick review :-) Is this better ?

[idryomov]

Looks good to me. Did you try it on that box?

[ghost]

@idryomov not yet, I will

[ghost]

while : ; do
    for i in $(seq 50) ; do
        if partprobe /dev/vdc > /dev/null 2>&1 ; then
            echo " $i"
            break
        else
            echo -n .
            sleep 5
        fi
    done
done

to verify on a machine where the problem was observed originally that running partprobe 50 times is enough. The highest number of retry so far is 14 (which is really high...).

[ghost]

A better option could be to block udev BLKRRPART with flock -s /dev/vdd partprobe /dev/vdd.

[ghost]

@idryomov instead of retrying, partprobe is wrapped around flock (b7fff6a). Do you think this is sane ? I wrote a long explanation with pointers in the b7fff6a commit message. I've been running

[root@dhcp-126-126 ~]# while : ; do flock -s /dev/vdc partprobe /dev/vdc || break ; done

in a loop and saw no failure so far. The idea was suggested by Marius Vollmer and Brian C. Lane. It's simple and elegant and I think it fits well with how ceph-disk is currently using partprobe and sgdisk.

I also have verified that the flock in udev was there from the start (i.e. udev 214 which is a tag that includes 02ba8fb3357daf57f6120ac512fb464a4c623419 which adds the flock).

[idryomov]

I had flock on my mind too - that's what I meant by "udev backs off" in my reply on the tracker, but the problem is that udev started doing flock not too long ago. If we care about distros with an older udev (< 214), flock won't work. One such distro is Ubuntu 14.04, with parted 2.3 and a pre-systemd udev. Have you ever observed failures there, or is this specific to parted 3.1?

[ghost]

I had flock on my mind too - that's what I meant by "udev backs off" in my reply on the tracke

I was too uneducated at the time to understand what you meant, sorry about that :-(

If we care about distros with an older udev (< 214), flock won't work.

Prior to udev 214, no BLKRRPART was issued when the device open in write mode is closed. Therefore I don't think we need to worry about that. Or am I missing something ?

[ghost]

test this please

[idryomov]

In that case, I'd call this resolved ;)

(I lost track of all the udev-related problems, see http://tracker.ceph.com/issues/14737 for an example.)

[ghost]

ceph-workbench ceph-qa-suite --verbose --simultaneous-jobs 9 --ceph-qa-suite-git-url http://github.com/dachary/ceph-qa-suite --suite-branch wip-ceph-disk --ceph-git-url http://github.com/dachary/ceph --ceph wip-15176-partprobe --suite ceph-disk --upload --filter ubuntu_14

http://167.114.237.66:8081/ubuntu-2016-05-26_15:45:06-ceph-disk-wip-15176-partprobe---basic-openstack/

[ghost]

The bluestore part of the ceph-disk suite fails on master, therefore also fails with this patch. The rest of the suite passes on Ubuntu 14.04. Now trying with CentOS 7.2.

[ghost]

It also passes the ceph-disk suite on CentOS 7.2 (except for the bluestore and the multipath test that also fails on master).

[idealguo]

@dachary, I test the patch on jewel 10.2.2 for several times, it resolves the bug #15176. Do you has some plans to merge it to jewel branch?

[ghost]

I'm waiting for someone to review this pull request. I should ask explicitly now. @tchaikov would you have time for that ?

[tchaikov]

sure, lemme take a look at it.

[tchaikov]

note for myself, when trying to understand the racing here.

• ceph-disk: write(/dev/sdc), close(/dev/sdc) => inotify =>
  • systemd/udev: with flock(dev/sdc, exclusive=True): ioctl(BLKRRPART)
  • ceph-disk: partprobe: BLKPG commands

guard partprobe with flock() fixes this.

[tchaikov]

@dachary could you please add "Fixes: http://tracker.ceph.com/issues/15176" in your commit message. then it is good to merge.

[ghost]

@tchaikov thanks for your review. I amended the commit message and will merge as soon as make check returns.