On 8/2/2016 5:15 PM, J Freyensee wrote:
+int +fcloop_fcp_op(struct nvmet_fc_target_port *tgtport, + struct nvmefc_tgt_fcp_req *tgt_fcpreq) +{ + ... + + return 0;if this function returns an 'int', why would it always return 0 and not the fcp_err values (if there is an error)?
I'll look at it. The reason it's carried in the tgt_fcpreq is that a real LLDD would likely return from the fcp_op, and report an error later on. I'll see if things should be reorg'd.
+struct nvme_fc_port_template fctemplate = { + .create_queue = fcloop_create_queue, + .delete_queue = fcloop_delete_queue, + .ls_req = fcloop_ls_req, + .fcp_io = fcloop_fcp_req, + .ls_abort = fcloop_ls_abort, + .fcp_abort = fcloop_fcp_abort, + + .max_hw_queues = 1, + .max_sgl_segments = 256, + .max_dif_sgl_segments = 256, + .dma_boundary = 0xFFFFFFFF,Between here and "struct nvmet_fc_target_template tgttemplate" they are assigning the same magic values to the same variable names, so why not have these values as #defines for a tad easier maintainability?
sure.
+static ssize_t +fcloop_create_local_port(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct nvme_fc_port_info pinfo; + struct fcloop_ctrl_options *opts; + struct nvme_fc_local_port *localport; + struct fcloop_lport *lport; + int ret; + + opts = kzalloc(sizeof(*opts), GFP_KERNEL); + if (!opts) + return -ENOMEM; + + ret = fcloop_parse_options(opts, buf); + if (ret) + goto out_free_opts; + + /* everything there ? */ + if ((opts->mask & LPORT_OPTS) != LPORT_OPTS) { + ret = -EINVAL; + goto out_free_opts; + } + + pinfo.fabric_name = opts->fabric; + pinfo.node_name = opts->wwnn; + pinfo.port_name = opts->wwpn; + pinfo.port_role = opts->roles; + pinfo.port_id = opts->fcaddr; + + ret = nvme_fc_register_localport(&pinfo, &fctemplate, NULL, &localport); + if (!ret) { + /* success */ + lport = localport->private; + lport->localport = localport; + INIT_LIST_HEAD(&lport->list); + INIT_LIST_HEAD(&lport->rport_list); + list_add_tail(&lport->list, &fcloop_lports); + + /* mark all of the input buffer consumed */ + ret = count; + } + +out_free_opts: + kfree(opts); + return ret; +} + +static int __delete_local_port(struct fcloop_lport *lport) +{ + int ret; + + if (!list_empty(&lport->rport_list)) + return -EBUSY; + + list_del(&lport->list); +Is a mutex or locking mechanism not needed here for this list?
Yeah - could probably use. As it was mainly a test tool with a controlled sequence, I think I ignored it.
-- james -- 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
