On Tue, 2013-01-29 at 11:30 -0800, Nicholas A. Bellinger wrote:
> This bottom section is useless here, as config_item_name() will never
> return NULL for a normal se_device.
I only had it in as a defensive measure - kernel panics are ugly so I
thought a sanity check wouldn't be a bad idea. If we are guaranteed to
never return NULL and you hate the code then I can remove the check.
> This is still missing the ability to pass flag=0 to revert back to the
> old model name.
>
> Please make sure that any option that can be enabled via configfs, can
> also be disabled via configfs.
Yes, that is bad. I'll fix.
> > @@ -1468,8 +1510,12 @@ int target_configure_device(struct se_device *dev)
> > */
> > if (dev->transport->transport_type != TRANSPORT_PLUGIN_PHBA_PDEV) {
> > strncpy(&dev->t10_wwn.vendor[0], "LIO-ORG", 8);
> > - strncpy(&dev->t10_wwn.model[0],
> > - dev->transport->inquiry_prod, 16);
> > + if (dev->dev_attrib.emulate_model_alias) {
> > + dev_set_t10_wwn_model_alias(dev);
> > + } else {
> > + strncpy(&dev->t10_wwn.model[0],
> > + dev->transport->inquiry_prod, 16);
> > + }
>
> Please drop this code for the non default usage case.
>
> If someone want's to make this the default action, then they should
> modify user-space to set emulate_model_alias=1.
I'm not sure I understand what change you're looking for here. You
always want the device to be built with dev->transport->inquiry_prod and
then only changed by dev_set_t10_wwn_model_alias() at the time the
administrator modifies the user-space option? What about people like me
who set DA_EMULATE_MODEL_ALIAS to 1 and compile? Doesn't it make more
sense to check the option at the time we configure the device so that it
does the right thing according the value of the config option at that
moment (however it got that way)?
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
