[dm-devel] [PATCH] multipath-tools: don't discard uevent for NVMe-Fabrics device

[Guan Junxiong]

Hi，Martin：

On 2017/7/13 18:04, Martin Wilck wrote:
> On Thu, 2017-07-13 at 15:47 +0800, Guan Junxiong wrote:
>> The devpath of uevent of NVMe-Fabrics device is like this
>> "../devices/virtual/nvme-fabrics/ctl/nvme0/nvme0n1" which
>> doesn't contains the "/block/" string. So when new uvents
>> of such nvme devices arise, the multipathd daemon still ignores
>> them, which results the DM-multipath doesn't update the table.
>> This patch fixes this by introducing a new helper to filter
>> "/block/" and "nvme-fabrics/ctl".
>>
>> Signed-off-by: Junxiong Guan <guanjunxiong at huawei.com>
> 
> The analysis is correct, but NAK nonetheless for the patch. The whole
> uevent_can_discard_by_devpath() approach is broken. I'll send a patch
> dropping that function entirely later today.
> 
> Martin
> 

I haven't seen your another patch since yesterday.  So I update a new patch
in the following. Does it looks good for you ? If so, I will send it out formally.

--
>From 2a9df3671fbad532f96918f209ddb0f76ffe28c4 Mon Sep 17 00:00:00 2001
From: Junxiong Guan <guanjunxiong at huawei.com>
Date: Fri, 14 Jul 2017 11:19:39 -0400
Subject: [PATCH v2] multipath-tools: don't discard uevent for NVMe-Fabrics device

The devpath of uevent of NVMe-Fabrics device is like this
"../devices/virtual/nvme-fabrics/ctl/nvme0/nvme0n1" which
doesn't contains the "/block/" string. So when new uvents
of such nvme devices arise, the multipathd daemon still ignores
them, which results the DM-multipath doesn't update the table.
This patch fixes this by dropping the uevent_discard_by_devpath()
function and checking whether the device type of the associated
uevent is "disk".

Signed-off-by: Junxiong Guan <guanjunxiong at huawei.com>
---

Changes since v1:
* Dropped uevent_discard_by_devpath function, use __disk__ device
type to filter uevent. (Suggested by Martin Wilck)
* Modify commit comments to keep consistent with the new approach

 libmultipath/uevent.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 4fbd1dfb..1164c982 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -143,35 +143,23 @@ uevent_need_merge(void)
 	return need_merge;
 }

-static bool
-uevent_can_discard_by_devpath(const char *devpath)
-{
-	static const char BLOCK[] = "/block/";
-	const char *tmp = strstr(devpath, BLOCK);

-	if (tmp == NULL) {
-		condlog(4, "no /block/ in '%s'", devpath);
-		return true;
-	}
-	tmp += sizeof(BLOCK) - 1;
-	if (*tmp == '\0')
-		/* just ".../block/" - discard */
-		return true;
-	/*
-	 * If there are more path elements after ".../block/xyz",
-	 * it's a partition - discard it; but don't discard ".../block/sda/".
-	 */
-	tmp = strchr(tmp, '/');
-	return tmp != NULL && *(tmp + 1) != '\0';
-}

 bool
 uevent_can_discard(struct uevent *uev)
 {
 	struct config * conf;
+	const char *devtype = NULL;

-	if (uevent_can_discard_by_devpath(uev->devpath))
+	if (uev->devpath == NULL)
 		return true;
+	if (uev->udev)
+		devtype = udev_device_get_devtype(uev->udev);
+	if (!devtype || strncmp(devtype, "disk", 4)) {
+		condlog(4, "discard '%s' , devtype = '%s'",
+			devtype == NULL ? "" : devtype, uev->devpath);
+		return true;
+	}

 	/*
 	 * do not filter dm devices by devnode
-- 
2.11.1