On Thu, 2018-10-18 at 09:28 +0800, s90006763 wrote:
> When the iSCSI path information is displayed using the multipath
> command, the host WWNN in the path information is not fully
> displayed, as follows:
> Multipathd show paths format "%N%R"
> Host WWNN host WWPN
> [undef] iqn.xxx-xx.com.xx:oceanstor:xxxx:: xx:x.x.x.x
> [undef] iqn.xxx-xx.com.xx:oceanstor:xxxx:: xx:x.x.x.x
>
> This patch solves this problem and gets the complete display of host
> WWNN related fields,as follows:
> Multipathd show paths format "%N%R"
> Host WWNN host WWPN
> Iqn.xx-x.com.redhat:86329 iqn.xxx-xx.com.xx:oceanstor:xxxx::
> xx:x.x.x.x
> Iqn.xx-x.com.redhat:86329 iqn.xxx-xx.com.xx:oceanstor:xxxx::
> xx:x.x.x.x
> ---
> libmultipath/discovery.c | 70
> ++++++++++++++++++++++++++++++++++++++++++++++++
> libmultipath/print.c | 4 ++-
> libmultipath/structs.h | 1 +
> 3 files changed, 74 insertions(+), 1 deletion(-)
Dear Sunao,
thank you for the updated patch, it's appreciated. It's much improved
over the previous version. I was able to apply the patch and verify
that it works as intended for iSCSI. See further comments below.
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 873035e5..a974c358 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -404,6 +404,71 @@ sysfs_get_tgt_nodename (struct path *pp, char *
> node)
> return 0;
> }
>
> +int
> +sysfs_get_host_nodename (struct path *pp, char * node)
> +{
> + const char *hostname, *value;
> + struct udev_device *parent, *hostdev;
> + int host, tgtid = -1;
> +
> + parent = udev_device_get_parent_with_subsystem_devtype(pp-
> >udev, "scsi", "scsi_host");
> + if (!parent)
> + return 1;
> +
You have trailing whitespace here. Please always check your patches for
such issues before submission (I recommend you use
scripts/checkpatch.pl from the linux kernel source tree).
> + /* Check for FibreChannel */
> + value = udev_device_get_sysname(parent);
> + if (sscanf(value, "host%d",
> + &host) == 1) {
This test is not necessary, as udev_device_new_from_subsystem_sysname()
will fail if you pass an invalid name. I recommend you code this the
same way as in print.c: snprint_host_attr().
> + hostdev = udev_device_new_from_subsystem_sysname(udev,
> + "fc_host", value);
> + if (hostdev) {
> + condlog(0, "SCSI host %d -> "
> + "FC host %d",
> + pp->sg_id.host_no, host);
condlog(0) is only for fatal errors. This is a debug message. Use
condlog(4), here, if at all.
> + value = udev_device_get_sysattr_value(hostdev,
> + "node_nam
> e");
> + if (value) {
> + strncpy(node, value, NODE_NAME_SIZE);
> + udev_device_unref(hostdev);
> + return 0;
> + } else
> + udev_device_unref(hostdev);
Put the udev_device_unref() before the "if" clause, and delete the
"else" clause.
> + }
> + }
> +
> + /* Check for iSCSI */
> + parent = pp->udev;
> + hostname = NULL;
> + while (parent) {
> + hostname = udev_device_get_sysname(parent);
> + if (hostname && sscanf(hostname , "session%d", &tgtid)
> == 1)
> + break;
> + parent = udev_device_get_parent(parent);
> + hostname = NULL;
> + tgtid = -1;
> + }
> + if (parent && hostname) {
> + hostdev = udev_device_new_from_subsystem_sysname(udev,
> + "iscsi_session", hostname);
> + if (hostdev) {
> + const char *value;
> +
> + value = udev_device_get_sysattr_value(hostdev,
> "initiatorname");
> + if (value) {
> + strncpy(node, value, NODE_NAME_SIZE);
> + udev_device_unref(hostdev);
> + return 0;
> + }
> + else
> + udev_device_unref(hostdev);
See above wrt udev_device_unref().
> + }
> + }
> +
> + /* Unknown SCSI transport. Keep fingers crossed */
> + pp->sg_id.proto_id = SCSI_PROTOCOL_UNSPEC;
Don't set sg_id.proto_id. We currently assign sg_id.proto_id in
sysfs_get_tgt_nodename(), which is a bit arbitrary, but there's no
strong cause to change it at this time. We definitely don't want to set
it in more than one place.
I'd rather recommend to call sysfs_get_host_nodename() after
sysfs_get_tgt_nodename(), and use the already initialized
sg_id.proto_id to determine how to figure out the node name rather than
repeating the sysfs searches that were already done.
> + return 0;
> +}
> +
> int sysfs_get_host_adapter_name(const struct path *pp, char
> *adapter_name)
> {
> int proto_id;
> @@ -1191,6 +1256,11 @@ scsi_sysfs_pathinfo (struct path * pp, vector
> hwtable)
> pp->sg_id.channel,
> pp->sg_id.scsi_id,
> pp->sg_id.lun);
> + /*
> + * host node name
> + */
> + if(sysfs_get_host_nodename(pp, pp->host_node_name))
> + return PATHINFO_FAILED;
Not being able to determine the nodename is a minor issue.
There's no reason to fail pathinfo() altogether. Rather, use a
condlog(2) message if you have found a device that *should* provide a
host WWNN (FC or iSCSI), and you fail to retrieve the attribute.
>
> /*
> * target node name
> diff --git a/libmultipath/print.c b/libmultipath/print.c
> index 7b610b94..d512ce19 100644
> --- a/libmultipath/print.c
> +++ b/libmultipath/print.c
> @@ -554,7 +554,9 @@ out:
> int
> snprint_host_wwnn (char * buff, size_t len, const struct path * pp)
> {
> - return snprint_host_attr(buff, len, pp, "node_name");
> + if (pp->host_node_name[0] == '\0')
> + return snprintf(buff, len, "[undef]");
> + return snprint_str(buff, len, pp->host_node_name);
> }
>
> int
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index 0a2623a0..63bd3941 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -253,6 +253,7 @@ struct path {
> char product_id[PATH_PRODUCT_SIZE];
> char rev[PATH_REV_SIZE];
> char serial[SERIAL_SIZE];
> + char host_node_name[NODE_NAME_SIZE];
The only place where the host_node_name is used is for printing
information for the user. I'd prefer not to blow up struct path further
for this. Can't you simply call sysfs_get_host_nodename() in the
snprint_host_wwnn() code path?
Regards,
Martin
> char tgt_node_name[NODE_NAME_SIZE];
> unsigned long long size;
> unsigned int checkint;
--
Dr. Martin Wilck <mwilck@xxxxxxxx>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
