[dm-devel] [PATCH 2/2] libmultipath: fix false removes in dmevents polling code

[Martin Wilck]

On Wed, 2018-11-28 at 00:13 -0600,  Benjamin Marzinski  wrote:
> dm_is_mpath() would return 0 if either a device was not a multipath
> device or if the libdevmapper command failed. Because dm_is_mpath()
> didn't distinguish between these situations, dm_get_events() could
> assume that a device was not really a multipath device, when in fact
> it
> was, and the libdevmapper command simply failed. This would cause the
> dmevents polling waiter to stop monitoring the device.
> 
> In reality, the call to dm_is_mpath() isn't necessary, because
> dm_get_events() will already verify that the device name is on the
> list
> of devices to wait for. However, if there are a large number of
> non-multipath devices on the system, ignoring them can be
> useful.  Thus,
> if dm_is_mpath() successfully runs the libdevmapper command and
> verifies
> that the device is not a multipath device, dm_get_events() should
> skip
> it. But if the libdevmapper command fails, dm_get_events() should
> still
> check the device list, to see if the device should be monitored.
> 
> This commit makes dm_is_mpath() return -1 for situations where
> the libdevmapper command failed, and makes dm_get_events() only
> ignore
> the device on a return of 0.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>
> ---

Two minor remarks:

1. You've changed dm_is_mpath() calls from "if (!result)" to 
"if (result == 0)" or "if (result != 1)" syntax, but you missed the
call in watch_dmevents(). I suggest that, for clarity, you change that
one as well.

2. We should add a condlog(2,...) to dm_is_mpath() if it returns -1.

Otherwise, ACK.

Martin