On Tue, Jan 31, 2023 at 01:09:57PM -0600, Mike Christie wrote:
>
> On 12/2/22 06:23, Dmitry Bogdanov wrote:
> > +
> > +static void tcm_remote_port_unlink(
> > + struct se_portal_group *se_tpg,
> > + struct se_lun *lun)
> > +{
> > + pr_debug("TCM_Remote_ConfigFS: Port Unlink LUN %lld Successful\n",
> > + lun->unpacked_lun);
> > +}
> > +
> > +/* End items for tcm_remote_port_cit */
> > +
> > +/* Start items for tcm_remote_naa_cit */
> > +
> > +static struct se_portal_group *tcm_remote_make_tpg(struct se_wwn *wwn,
> > + const char *name)
> > +{
>
> The patch seems ok.
>
> My only comments are on coding style:
>
> 1. I know we have a mismatch in other lio code like above where sometimes we
> don't put any args on the first line after the "(" in tcm_remote_port_unlink
> and sometimes we do the above. Since this is new code, could you do the more
> standard style?
Yes, I will do it.
> 2. Maybe for some of these callouts where most drivers are returning the same
> hard coded value we shouldn't make them mandatory. target_fabric_tf_ops_check
> should just set a default callout.
I see that those hardcoded values are different (0 or 1) in the drivers :)
Most likely they can be the same, but those values are exported to sysfs
and potentially it could break some userspace. That would add a much of
questions.
I would like to keep this patch as much as simple.
BR,
Dmitry
