[SRU]'multipath -r' causes /dev/mapper/<wwid> being removed

Bug #1621340 reported by Liang Chen
18
This bug affects 1 person
Affects Status Importance Assigned to Milestone
multipath-tools (Ubuntu)
Fix Released
Medium
Hua Zhang
Xenial
Fix Released
Medium
Hua Zhang
Yakkety
Fix Released
Medium
Hua Zhang

Bug Description

[Impact]

"multipath -r" causes the /dev/mapper/<wwid> to disappear momentarily, which leads to some issue in consumer applications as such OpenStack.

[Test Case]

 * connect to an multipath iscsi target
 * multipath -r
 * /dev/mapper/<wwid> disappears momentarily

[Regression Potential]

 * None

"multipath -r" causes the /dev/mapper/<wwid> to disappear momentarily, which leads to some issue in consumer applications as such OpenStack. After some investigation, I found that /dev/mapper/<wwid> was deleted by udev during the reload, and it was re-created soon later by multipathd (livdevmapper code of cause). Detailed findings are as follows:

For reload in domap (rename as well),

        case ACT_RELOAD:
                r = dm_addmap_reload(mpp, params);
                if (r)
                        r = dm_simplecmd_noflush(DM_DEVICE_RESUME, mpp->alias,
                                                 0, MPATH_UDEV_RELOAD_FLAG);
                break;

it passes 0 to dm_simplecmd_noflush as argument for needsync, which makes dm_task_set_cookie call being skipped in dm_simplecmd,

        if (udev_wait_flag && !dm_task_set_cookie(dmt, &cookie, ((conf->daemon)? DM_UDEV_DISABLE_LIBRARY_FALLBACK : 0) | udev_flags)) {
                dm_udev_complete(cookie);
                goto out;
        }

because of the short-circuit evaluation. Thus _do_dm_ioctl in libdevmapper will add DM_UDEV_DISABLE_DM_RULES_FLAG flag to dmi->event_nr, and that will eventually be used in the udev rules (55-dm.rules),

ENV{DM_UDEV_DISABLE_DM_RULES_FLAG}!="1", ENV{DM_NAME}=="?*", SYMLINK+="mapper/$env{DM_NAME}"

Since the DM_UDEV_DISABLE_DM_RULES_FLAG is set, the rule will not match. As a result the link is removed.

Liang Chen (cbjchen)
Changed in multipath-tools (Ubuntu):
status: New → In Progress
assignee: nobody → Liang Chen (cbjchen)
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "xenial debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

tags: added: patch
Liang Chen (cbjchen)
Changed in multipath-tools (Ubuntu Xenial):
status: New → In Progress
assignee: nobody → Liang Chen (cbjchen)
Mathew Hodson (mhodson)
Changed in multipath-tools (Ubuntu):
importance: Undecided → Medium
Changed in multipath-tools (Ubuntu Xenial):
importance: Undecided → Medium
Liang Chen (cbjchen)
description: updated
tags: added: sts
Hua Zhang (zhhuabj)
Changed in multipath-tools (Ubuntu Xenial):
assignee: Liang Chen (cbjchen) → Hua Zhang (zhhuabj)
Changed in multipath-tools (Ubuntu):
assignee: Liang Chen (cbjchen) → Hua Zhang (zhhuabj)
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Hi Hua,
this is broken in zesty as well, but there it should/will be fixed by a merge of multipath-tools >=6.1
Thank you for the debdiff on X&Y.

I had to look up as I wanted to check if this is already upstream, it would be very kind if you could add valid dep3 headers to your patches http://dep.debian.net/deps/dep3/
Especially in cases like this it helps reviewers to quickly find if and where such a patch is already upstream.

The patches themself looks correct to me, if you would refresh them that would be great.
I'll add a Yakkety task already.

Then following the SRU policy this should be in Zesty before doing the SRUs - but I didn't hear anybody stating that they are working on multipath-tools merge already - I'll ask around.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Note that if a merge isn't close we can push the same fix into zesty as we do in yakkety.

Revision history for this message
Hua Zhang (zhhuabj) wrote :
Revision history for this message
Hua Zhang (zhhuabj) wrote :
Revision history for this message
Hua Zhang (zhhuabj) wrote :
Revision history for this message
Hua Zhang (zhhuabj) wrote :

Hi paelzer,

I have updated debdiff based on the following base packages.

zesty.debdiff is based on zesty=0.5.0+git1.656f8865-5ubuntu8, not 6.1
yakkety.debdiff is based on yakkety-proposed=0.5.0+git1.656f8865-5ubuntu7.1
xenial.debdiff is based on xenial-updates=0.5.0+git1.656f8865-5ubuntu2.3

and the upstream patch [1] is generated by 'git format-patch' command so it's compatible with dep3 now. thank you very much.

[1] http://git.opensvc.com/gitweb.cgi?p=multipath-tools/.git;a=commit;h=f99464031273ea325ab8ae56fb12b6f42f81b2b7

tags: added: sts-sru
Hua Zhang (zhhuabj)
summary: - 'multipath -r' causes /dev/mapper/<wwid> being removed
+ [SRU]'multipath -r' causes /dev/mapper/<wwid> being removed
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thank you Hua,
the merge of a newer multipath-tools is current WIP.
I'd wait til that is done and then consider the SRU instead of doing an interim upload to zesty if you are ok with that approach.

If this issue is urgent for you or your peers please let me know.

Revision history for this message
Hua Zhang (zhhuabj) wrote :

Hi paelzer,

Does newer multipath-tools mean multipath-tools>=6.1 you mentioned above, if yes, the fixed patch has been already in it. It's too great to have with that approach -:)

BTW, do we have the tracking link to know the process status for the newer multipath-tools merge work, and an estimate about how long it will take? thanks.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote : Re: [Bug 1621340] Re: [SRU]'multipath -r' causes /dev/mapper/<wwid> being removed

On Mon, Nov 28, 2016 at 9:13 AM, Hua Zhang <email address hidden>
wrote:

> Does newer multipath-tools mean multipath-tools>=6.1 you mentioned
> above, if yes, the fixed patch has been already in it. It's too great to
> have with that approach -:)
>

Would be 6.3 and yes due to that I'd not need the zesty debdiff.

> BTW, do we have the tracking link to know the process status for the
> newer multipath-tools merge work, and an estimate about how long it will
> take? thanks.
>

There is no extra tracking, but I already included this bug in the
changelog.
So once an upload appears this bug will get updated.

The initial merge is more or less done, the next stages will be about
Testing the new revision and reviewing my merge.
I'll add you onto the related bugs and reviews so that you are updated on
the status.

--
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

Mathew Hodson (mhodson)
Changed in multipath-tools (Ubuntu Yakkety):
importance: Undecided → Medium
Revision history for this message
Hua Zhang (zhhuabj) wrote :

Hi paelzer,

Is the merge of a newer multipath-tools=6.3 for Zesty done? I saw the link [1] said 'MIR is done', but I did not see any update in the link [2].

I am tracking the current status of SRU work for Xenial because this issue is urgent for us, can I have an estimate about when it will start? thanks.

[1] https://bugs.launchpad.net/ubuntu/+source/multipath-tools/+bug/1645274
[2] https://launchpad.net/ubuntu/+source/multipath-tools/+publishinghistory

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Sorry Hua for the Delay, TL;DR waiting for merge review.

I just discussed last week with my peers, we agreed that if nothing happens I'll upload as-is soon.

Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (6.7 KiB)

This bug was fixed in the package multipath-tools - 0.6.4-3ubuntu1

---------------
multipath-tools (0.6.4-3ubuntu1) zesty; urgency=medium

  * Merge from Debian. (LP: #1621340, LP: #1645274) Remaining changes:
    - d/control:
      - Bump udev dependencies
      - multipath-udeb: add sg3-udeb Depends
    - d/rules: Move udev rules to priority 95, because rules that load modules
      should be >90.
    - d/multipath-tools.preinst: modprobe dm-multipath; This will make sure
      that multipathd will be able to start.
    - Split kpartx initramfs bits into kpartx-boot for dmraid (LP: #941874)
      - d/initramfs/kpartx.hook
      - d/kpartx-boot.postinst
      - d/kpartx-boot.postrm
      - d/control: Add kpartx-boot package for dmraid
      - d/rules: Install kpartx initramfs hook
      - d/kpartx.install: install all arch /lib* kpartx udev rules
    - patches (some refreshed to new version) to multipath source
      - d/p/1000--set-umask-in-multipathd.patch: Set umask in multipathd.
      - d/p/path_selector.patch: switch the default path selector
        back to round-robin while service-time isn't available to the installer
        multipath-modules.
      - d/p/kpartx_more_loopback_fixes.patch: fix loopback mounted
        files some more: since we stat() the loopback device node, we can't rely
        on S_ISREG() tests to handle this case, and should look at the device
        itself instead. (LP: #1543430)
      - d/p/enable-find-multipaths.patch: re-enable find_multipaths
        by default -- see the removed 'add_find-multipaths.patch' (LP: #1463046)
   - multipath initramfs fixes for booting from multipathed devices
      - d/initramfs/hooks: also copy wwids file on the installed system to
        ensure all paths come up on boot. (LP: #1479929)
      - d/initramfs/hooks: install multipathd and required directories.
      - d/initramfs/hooks: copy dm-mpath-lvm & multipath udev rules to initramfs
      - d/initramfs/hooks: do not copy kpartx rules to initramfs
      - d/initramfs/local-bottom: remember to stop multipathd.
      - d/initramfs/local-premount: wait for udev to settle before the call to
        resolve_device() in local_mount_root(), so the by-uuid/ symlinks have a
        chance to be updated by the multipath udev rules (LP: #1503286).
      - d/initramfs/local-premount: Run multipath with -B so not to assign names
        nor change /etc/multipath/bindings during initramfs (LP: #1561103)
      - d/rules: install d/initramfs/local-bottom
      - d/rules: install d/initramfs/local-premount
   - Remove partition device nodes of individual paths (for LVM on multipath)
     (LP: #1540401)
   - Disable -fexceptions on multipath-udeb (LP: #1489379): the flag causes
     libchecktur.so to link with libgcc_s.so.1 (even with -static-libgcc),
     which is not available in the installer environment.
     - d/p/disable-fexceptions-udeb.patch: conditionally disable -fexceptions
       with CFLAGS_DISABLE_FEXCEPTIONS.
     - d/rules: set CFLAGS_DISABLE_FEXCEPTIONS to build multipath-udeb.
   - d/tests/kpartx-file-loopback: add an autopkgtest to catch future cases
     where uploads might break kpartx's loopback file handling.
   -...

Read more...

Changed in multipath-tools (Ubuntu):
status: In Progress → Fix Released
Changed in multipath-tools (Ubuntu Yakkety):
status: New → In Progress
assignee: nobody → Hua Zhang (zhhuabj)
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Fix is now in the Development release, ready to SRU it back.

@zhhuabi - your patches still look good and there was no burnt version number.
So aside some extra testing I think they can be taken as-is.

Yet while working on the multipath-tools merge I happened to realize that the server Team doesn't have upload rights on that yet (we had for a few hours last week :-) ).

I'm working on that, but to get you going I'm subscribing cyphermox who usually does that for the foundations Team.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

@cyphermox - to me the patches look good to me, could you take a look at sponsoring them into Yakkety Xenial?

Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

Reviewing the yakkety debdiff; all looks good, but I have some nitpicks:

- I expect more details in changelog: not just what was changed but some idea why.
- Fewer abbreviations when possible, when things can fit on a line: write debian/patches/... longform unless things otherwise really don't fit on a single line for the patch filename.
- Changelog lines should be limited to 80 columns.

I'm fixing this directly in changelog, there's no point in delaying the SRUs just for nitpicks in the changelog entries; I'll sponsor the changes as soon as I'm done with smoke-testing.

Revision history for this message
Robie Basak (racb) wrote :

Where does this patch come from please? Is it upstream and in an upstream master branch, or released upstream? I see no Origin: dep-3 header.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I already pointed proper dep3 headers out in comment #4 - there is also a link on the spec.
But I missed when they got added that they were not complete.
Good catch Robie, neither me nor cyphermox saw that.

FYI - Comment #9 has the link to the commit.

Who is rerolling the patch - zhhuabj or you in a quick reupload cyphermox?

Revision history for this message
Hua Zhang (zhhuabj) wrote :
Revision history for this message
Hua Zhang (zhhuabj) wrote :
Revision history for this message
Hua Zhang (zhhuabj) wrote :

Hi @racb @paelzer, I added two deb3 headers (Origin and Bug) now

Origin: upstream, http://git.opensvc.com/gitweb.cgi?p=multipath-tools/.git;a=commit;h=f9946403
Bug: LP: #1621340

Hi @cyphermox, I also delivered your comment #18

  * Cherrypick from upstream (LP: #1621340)
   - Added debian/patches/wait-for-udev-in-dm_simplecmd_noflush.patch

Many thanks for your review

Revision history for this message
Łukasz Zemczak (sil2100) wrote :

Hey, so I assume the current uploads in UNAPPROVED should be rejected. I'll sponsor the new versions of the uploads but then someone else would have to approve them to -proposed.

Revision history for this message
Chris J Arges (arges) wrote : Please test proposed package

Hello Liang, or anyone else affected,

Accepted multipath-tools into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/multipath-tools/0.5.0+git1.656f8865-5ubuntu2.4 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in multipath-tools (Ubuntu Xenial):
status: In Progress → Fix Committed
tags: added: verification-needed
Changed in multipath-tools (Ubuntu Yakkety):
status: In Progress → Fix Committed
Revision history for this message
Chris J Arges (arges) wrote :

Hello Liang, or anyone else affected,

Accepted multipath-tools into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/multipath-tools/0.5.0+git1.656f8865-5ubuntu7.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Revision history for this message
Hua Zhang (zhhuabj) wrote :

Verified successfully on both xenial-proposed and yakkety-proposed.

The test result can refer the link [1], and I refer the link [2] to set up the test env.

Before applying the patch we can see:

Feb 16 08:52:45 juju-zhhuabj-machine-9 systemd[13815]: dev-disk-by\x2did-scsi\x2d360000000000000000e00000000010001.device: Dev dev-disk-by\x2did-scsi\x2d360000000000000000e00000000010001.device appeared twice with different sysfs paths /sys/devices/platform/host13/session12/target13:0:0/13:0:0:1/block/sda and /sys/devices/virtual/block/dm-0

After applying the patch we can see:

Feb 16 09:06:10 juju-zhhuabj-machine-9 systemd-udevd[21289]: conflicting device node '/dev/mapper/360000000000000000e00000000010001' found, link to '/dev/dm-0' will not be created

[1] http://pastebin.ubuntu.com/24006257/
[2] https://gist.github.com/niedbalski/ae85dac1a3ea0b82ad05

tags: added: verification-done
removed: verification-needed
tags: added: verification-xenial-done verification-yakkety-done
Revision history for this message
Brian Murray (brian-murray) wrote : Update Released

The verification of the Stable Release Update for multipath-tools has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package multipath-tools - 0.5.0+git1.656f8865-5ubuntu2.4

---------------
multipath-tools (0.5.0+git1.656f8865-5ubuntu2.4) xenial; urgency=medium

  * debian/patches/wait-for-udev-in-dm_simplecmd_noflush.patch:
    - Cherrypick from upstream: wait for udev in dm_simplecmd_noflush().
      (LP: #1621340)

 -- Hua Zhang <email address hidden> Thu, 09 Feb 2017 20:08:57 +0800

Changed in multipath-tools (Ubuntu Xenial):
status: Fix Committed → Fix Released
Changed in multipath-tools (Ubuntu Yakkety):
status: Fix Committed → Fix Released
tags: added: sts-sru-done
removed: sts-sru
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.