Sorry for the late review comments. On 02/25/2011 05:08 AM, Vikas Chaudhary wrote:
+static int iscsi_match_ifaceid(struct device *dev, void *data) +{ + struct iscsi_iface *iface = iscsi_dev_to_iface(dev); + uint64_t *ifaceid = (uint64_t *) data; + + return *ifaceid == iface->id; +} + +struct iscsi_iface * +iscsi_create_iface(struct Scsi_Host *shost, struct iscsi_transport *transport, + uint32_t iface_type, uint32_t iface_num, int dd_size) +{ + struct device *dev; + struct iscsi_iface *iface; + uint64_t id; + int err; + + for (id = 1; id< ISCSI_MAX_IFACEID; id++) { + dev = class_find_device(&iscsi_iface_class, NULL,&id, + iscsi_match_ifaceid); + if (!dev) + break; + }
That loop was a mistake on my part. I think you should use idr. When I did that loop for the ep code I was thinking that we would only have a couple sessions - maybe 8 would be most common. However, with virtual ports we are ending up with lots and lots.
+ if (id == ISCSI_MAX_IFACEID) { + printk(KERN_ERR "Too many iface. Max supported %u\n", + ISCSI_MAX_IFACEID - 1); + return NULL; + } + + iface = kzalloc(sizeof(*iface) + dd_size, GFP_KERNEL); + if (!iface) + return NULL; + + iface->id = id; + iface->transport = transport; + iface->iface_type = iface_type; + iface->iface_num = iface_num; + iface->dev.class =&iscsi_iface_class; + /* parent reference */ + iface->dev.parent = get_device(&shost->shost_gendev); + if (iface_type == IFACE_TYPE_IPV4) + dev_set_name(&iface->dev, "ipv4-iface-%u-%u", shost->host_no, + iface_num); + else if (iface_type == IFACE_TYPE_IPV6) + dev_set_name(&iface->dev, "ipv6-iface-%u-%u", shost->host_no, + iface_num);
Is this supposed to have the "[" "]" around the address for ipv6, and who adds that. Is the driver or class? If drivers are adding it then fine, if not then we should have the api do it.
Thanks for working on this. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html