| Message ID | 1501205323-13440-1-git-send-email-guanjunxiong@huawei.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
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@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@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
Hi Hannes, On 2017/7/28 14:12, Hannes Reinecke wrote: > 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@huawei.com> >> --- >> libmultipath/discovery.c | 45 +++++++++++++++++++++++++++++++++++---------- >> 1 file changed, 35 insertions(+), 10 deletions(-) >> > Thanks for doing this. > >> > Patch looks good. > > Reviewed-by: Hannes Reinecke <hare@suse.com> > Nice to have any review. Thanks. > 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 IMO, I think Keep Alive feature is in controller granularity , _not_ namespace granularity. A namespace can be detached ( or disabled with nvmetcli tools) from controller which has many namespaces attached, but the controller sysfs state attribute is still "live". Therefor, we do need further path checker after we know controller is live. Refer to the following quoted from NVMe Spec 1.3: "The Keep Alive feature is used by the host to determine that the controller is operational and by the controller to determine that the host is operational. The host and controller are operational when each is accessible and able to issue or execute commands." Regards Guan . -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Merged. Thanks. On Fri, Jul 28, 2017 at 8:54 AM, Guan Junxiong <guanjunxiong@huawei.com> wrote: > Hi Hannes, > > On 2017/7/28 14:12, Hannes Reinecke wrote: > > 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@huawei.com> > >> --- > >> libmultipath/discovery.c | 45 ++++++++++++++++++++++++++++++ > +++++---------- > >> 1 file changed, 35 insertions(+), 10 deletions(-) > >> > > Thanks for doing this. > > > >> > > Patch looks good. > > > > Reviewed-by: Hannes Reinecke <hare@suse.com> > > > Nice to have any review. Thanks. > > > 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 > > IMO, I think Keep Alive feature is in controller granularity , _not_ > namespace > granularity. A namespace can be detached ( or disabled with nvmetcli tools) > from controller which has many namespaces attached, but the controller > sysfs > state attribute is still "live". Therefor, we do need further path checker > after we know controller is live. > > Refer to the following quoted from NVMe Spec 1.3: > "The Keep Alive feature is used by the host to determine that the > controller > is operational and by the controller to determine that the host is > operational. > The host and controller are operational when each is accessible and able to > issue or execute commands." > > > Regards > Guan > > > . > > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
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; }
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@huawei.com> --- libmultipath/discovery.c | 45 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 10 deletions(-)