Comment 3 for bug 2039719

Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote (last edit ):

Hi Jorge,

Thanks for the nice SRU template and debdiff!

Patch 1 has one minor nit; no worries.
Patch 2 has an important question and probably needs a change.

It turns out that both patches have fixes upstream which should be applied,
they fix crashes introduced by these code changes.

I tried to reproduce the issue with GDB (time multipathd and map removal)
based on the SRU impact description/patch 2, but did not make it yet.

Therefore, we should probably have the original reporter verify these
new changes still work correctly (added missing hunk in patch 2, and
added fixup commits in patches 3-4).

Please find the debdiff attached, and test packages in ppa:mfo/test.

Thanks,
Mauricio

...

Patch 1:

Changed 'Origin:' keyword to backport; there are context lines changes to upstream:

 @ libmultipath/devmapper.h

 Upstream:
 ...
  void dm_init(int verbosity);
  int dm_prereq(unsigned int *v);
  void skip_libmp_dm_init(void);

 Patch:
 ...
  void dm_init(int verbosity);
  void libmp_dm_init(void);
  void libmp_udev_set_sync_support(int on);

 @ libmultipath/structs_vec.c

 Upstream:
 ...
  static void enter_recovery_mode(struct multipath *mpp)

 Patch:
 ...
  void enter_recovery_mode(struct multipath *mpp)

...

Patch 2:

No backport description or explanation of why changes to reinstate_path() were dropped.

Is that an oversight? The changes related to the different return type are included
(e.g., remove reinstate_path() from 2 `if` checks and call it without return checks).

Apparently, it looks like it; I re-introduced the hunk/backport usage of add_active.

...

Fixups:

These 2 patches have fixes upstream:

 $ git log --oneline -1 e282f54a5d57e
 e282f54a5d57 multipathd fix NULL dereference in check_path

 $ git describe --contains e282f54a5d57e
 0.8.6~1^2~17

 $ git log --oneline -1 7439a41fa12dd
 7439a41fa12d dm_get_map: fix segfault when can't found target

 $ git describe --contains 7439a41fa12dd
 0.9.0^2~8

They're both related to iSCSI, which IIRC can be used by Openstack Cinder volumes (Ubuntu is very popular for Openstack deployments) in multipath mode.

The second fixup should be included in Lunar [update: and Jammy!]:

 $ rmadison -a source multipath-tools
 ...
  multipath-tools | 0.8.3-1ubuntu2 | focal | source
  multipath-tools | 0.8.3-1ubuntu2.1 | focal-security | source
  multipath-tools | 0.8.3-1ubuntu2.1 | focal-updates | source
  multipath-tools | 0.8.3-1ubuntu2.2 | focal-proposed | source
  multipath-tools | 0.8.8-1ubuntu1 | jammy | source
  multipath-tools | 0.8.8-1ubuntu1.22.04.1 | jammy-security | source
  multipath-tools | 0.8.8-1ubuntu1.22.04.1 | jammy-updates | source
  multipath-tools | 0.8.8-1ubuntu1.22.04.3 | jammy-proposed | source
  multipath-tools | 0.8.8-1ubuntu2 | lunar | source
  multipath-tools | 0.8.8-1ubuntu2.2 | lunar-proposed | source
  multipath-tools | 0.9.4-5ubuntu3 | mantic | source
  multipath-tools | 0.9.4-5ubuntu3 | noble | source