[dm-devel] [PATCH] multipath-tools: check sysfs path state for NVMe/NVMf

[Hannes Reinecke]

On 07/28/2017 03:28 AM, Guan Junxiong wrote:
> The previous code of path_offline checking was only valid for SCSI
> device. It returned PATH_UP for other devices and throwed path check-
> ing to chekers. This patch supplements checking sysfs path state for
> NVMe/NVMf devices. For example, if NVMe/NVMf path is reconnectting or
> resetting, we return PATH_PENDING in order to skip current path check-
> ing and reschedule path checking in the next tick as soon as possible.
> 
> Signed-off-by: Guan Junxiong <guanjunxiong at huawei.com>
> ---
>  libmultipath/discovery.c | 45 +++++++++++++++++++++++++++++++++++----------
>  1 file changed, 35 insertions(+), 10 deletions(-)
> 
Thanks for doing this.

> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 663c8eaa..b549ce0e 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -1383,14 +1383,22 @@ path_offline (struct path * pp)
>  	struct udev_device * parent;
>  	char buff[SCSI_STATE_SIZE];
>  	int err;
> +	const char *subsys_type;
>  
> -	if (pp->bus != SYSFS_BUS_SCSI)
> +	if (pp->bus == SYSFS_BUS_SCSI) {
> +		subsys_type = "scsi";
> +	}
> +	else if (pp->bus == SYSFS_BUS_NVME) {
> +		subsys_type = "nvme";
> +	}
> +	else {
>  		return PATH_UP;
> +	}
>  
>  	parent = pp->udev;
>  	while (parent) {
>  		const char *subsys = udev_device_get_subsystem(parent);
> -		if (subsys && !strncmp(subsys, "scsi", 4))
> +		if (subsys && !strncmp(subsys, subsys_type, 4))
>  			break;
>  		parent = udev_device_get_parent(parent);
>  	}
> @@ -1412,15 +1420,32 @@ path_offline (struct path * pp)
>  
>  	condlog(3, "%s: path state = %s", pp->dev, buff);
>  
> -	if (!strncmp(buff, "offline", 7)) {
> -		pp->offline = 1;
> -		return PATH_DOWN;
> +	if (pp->bus == SYSFS_BUS_SCSI) {
> +		if (!strncmp(buff, "offline", 7)) {
> +			pp->offline = 1;
> +			return PATH_DOWN;
> +		}
> +		pp->offline = 0;
> +		if (!strncmp(buff, "blocked", 7) ||
> +		    !strncmp(buff, "quiesce", 7))
> +			return PATH_PENDING;
> +		else if (!strncmp(buff, "running", 7))
> +			return PATH_UP;
> +
> +	}
> +	else if (pp->bus == SYSFS_BUS_NVME) {
> +		if (!strncmp(buff, "dead", 4)) {
> +			pp->offline = 1;
> +			return PATH_DOWN;
> +		}
> +		pp->offline = 0;
> +		if (!strncmp(buff, "new", 3) ||
> +		    !strncmp(buff, "reconnecting", 12) ||
> +		    !strncmp(buff, "resetting", 9))
> +			return PATH_PENDING;
> +		else if (!strncmp(buff, "live", 4))
> +			return PATH_UP;
>  	}
> -	pp->offline = 0;
> -	if (!strncmp(buff, "blocked", 7) || !strncmp(buff, "quiesce", 7))
> -		return PATH_PENDING;
> -	else if (!strncmp(buff, "running", 7))
> -		return PATH_UP;
>  
>  	return PATH_DOWN;
>  }
> 
Patch looks good.

Reviewed-by: Hannes Reinecke <hare at suse.com>

However, I do wonder if we really need a path checker for NVMf once we
have this. After all, the sysfs attribute reflects that KATO status,
which pretty much determines if we can send I/O _at all_.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare at suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)