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