On 01/15/2013 01:01 PM, Nicholas A. Bellinger wrote:
> On Tue, 2013-01-15 at 10:06 -0800, Andy Grover wrote:
>> From: Tregaron Bayly <tbayly@xxxxxxxxxxxx>
>>
>> This patch changes LIO to use the configfs backend device name as the
>> model if you either change the DA_EMULATE_MODEL_ALIAS in
>> include/target/target_core_base.h before compiling (changes the default
>> behavior for all devices) or echo '1' to an individual device's
>> emulate_model_alias attribute (changes only that device).
>>
>> Signed-off-by: Tregaron Bayly <tbayly@xxxxxxxxxxxx>
>> Signed-off-by: Andy Grover <agrover@xxxxxxxxxx>
>> ---
>
> So I'm willing to take this for-3.9.. Just a few minor comments below..
Thanks for your feedback. Tregaron would you like to update the patch,
or shall I?
Thanks -- Andy
>
>> drivers/target/target_core_configfs.c | 4 ++++
>> drivers/target/target_core_device.c | 28 ++++++++++++++++++++++++++++
>> drivers/target/target_core_internal.h | 2 ++
>> include/target/target_core_base.h | 3 +++
>> 4 files changed, 37 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
>> index 4efb61b..43b7ac6 100644
>> --- a/drivers/target/target_core_configfs.c
>> +++ b/drivers/target/target_core_configfs.c
>> @@ -609,6 +609,9 @@ static struct target_core_dev_attrib_attribute \
>> __CONFIGFS_EATTR_RO(_name, \
>> target_core_dev_show_attr_##_name);
>>
>> +DEF_DEV_ATTRIB(emulate_model_alias);
>> +SE_DEV_ATTR(emulate_model_alias, S_IRUGO | S_IWUSR);
>> +
>> DEF_DEV_ATTRIB(emulate_dpo);
>> SE_DEV_ATTR(emulate_dpo, S_IRUGO | S_IWUSR);
>>
>> @@ -681,6 +684,7 @@ SE_DEV_ATTR(max_write_same_len, S_IRUGO | S_IWUSR);
>> CONFIGFS_EATTR_OPS(target_core_dev_attrib, se_dev_attrib, da_group);
>>
>> static struct configfs_attribute *target_core_dev_attrib_attrs[] = {
>> + &target_core_dev_attrib_emulate_model_alias.attr,
>> &target_core_dev_attrib_emulate_dpo.attr,
>> &target_core_dev_attrib_emulate_fua_write.attr,
>> &target_core_dev_attrib_emulate_fua_read.attr,
>> diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
>> index e269510..735784c 100644
>> --- a/drivers/target/target_core_device.c
>> +++ b/drivers/target/target_core_device.c
>> @@ -713,6 +713,20 @@ int se_dev_set_max_write_same_len(
>> return 0;
>> }
>>
>> +int se_dev_set_emulate_model_alias(struct se_device *dev, int flag)
>> +{
>
> As initiators may end up going hay-wire if their MODEL string changes
> underneath them during active export, please add a dev->export_count
> check here similar to se_dev_set_block_size() et al, and fail if there
> are active fabric exports.
>
>> + if (flag != 0 && flag != 1) {
>> + pr_err("Illegal value %d\n", flag);
>> + return -EINVAL;
>> + }
>> +
>> + if (flag) {
>> + dev_set_t10_wwn_model_alias(dev);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> int se_dev_set_emulate_dpo(struct se_device *dev, int flag)
>> {
>> if (flag != 0 && flag != 1) {
>> @@ -1384,6 +1398,7 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name)
>> dev->t10_alua.t10_dev = dev;
>>
>> dev->dev_attrib.da_dev = dev;
>> + dev->dev_attrib.emulate_model_alias = DA_EMULATE_MODEL_ALIAS;
>> dev->dev_attrib.emulate_dpo = DA_EMULATE_DPO;
>> dev->dev_attrib.emulate_fua_write = DA_EMULATE_FUA_WRITE;
>> dev->dev_attrib.emulate_fua_read = DA_EMULATE_FUA_READ;
>> @@ -1474,6 +1489,9 @@ int target_configure_device(struct se_device *dev)
>> dev->transport->inquiry_rev, 4);
>> }
>>
>> + if (dev->dev_attrib.emulate_model_alias)
>> + dev_set_t10_wwn_model_alias(dev);
>> +
>
> Move this up into the (transport_type != TRANSPORT_PLUGIN_PHBA_PDEV)
> check above, and make the else {} block set model from
> dev->transport->inquiry_prod[] for the default cause.
>
>> scsi_dump_inquiry(dev);
>>
>> spin_lock(&hba->device_lock);
>> @@ -1545,6 +1563,16 @@ out_free_hba:
>> return ret;
>> }
>>
>> +void dev_set_t10_wwn_model_alias(struct se_device *dev)
>> +{
>> + const char *configname;
>> +
>> + configname = config_item_name(&dev->dev_group.cg_item);
>> + if (configname) {
>> + strncpy(&dev->t10_wwn.model[0], configname, 16);
>> + dev->t10_wwn.model[15] = '\0';
>> + }
>> +}
>
> This should be statically defined. Please make it static and move it
> ahead of se_dev_set_emulate_model_alias() usage above.
>
> Also, there probably should be some pr_warn() here if the configname
> exceeds the 16 bytes allocated for INQUIRY MODEL.
>
>>
>> void core_dev_release_virtual_lun0(void)
>> {
>> diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
>> index 93e9c1f..572a947 100644
>> --- a/drivers/target/target_core_internal.h
>> +++ b/drivers/target/target_core_internal.h
>> @@ -25,6 +25,7 @@ int se_dev_set_max_unmap_block_desc_count(struct se_device *, u32);
>> int se_dev_set_unmap_granularity(struct se_device *, u32);
>> int se_dev_set_unmap_granularity_alignment(struct se_device *, u32);
>> int se_dev_set_max_write_same_len(struct se_device *, u32);
>> +int se_dev_set_emulate_model_alias(struct se_device *, int);
>> int se_dev_set_emulate_dpo(struct se_device *, int);
>> int se_dev_set_emulate_fua_write(struct se_device *, int);
>> int se_dev_set_emulate_fua_read(struct se_device *, int);
>> @@ -57,6 +58,7 @@ void core_dev_release_virtual_lun0(void);
>> struct se_device *target_alloc_device(struct se_hba *hba, const char *name);
>> int target_configure_device(struct se_device *dev);
>> void target_free_device(struct se_device *);
>> +void dev_set_t10_wwn_model_alias(struct se_device *dev);
>>
>
> and drop this unnecessary function prototype.
>
>> /* target_core_hba.c */
>> struct se_hba *core_alloc_hba(const char *, u32, u32);
>> diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
>> index 02ed017..aa9743c 100644
>> --- a/include/target/target_core_base.h
>> +++ b/include/target/target_core_base.h
>> @@ -75,6 +75,8 @@
>> #define DA_MAX_WRITE_SAME_LEN 0
>> /* Default max transfer length */
>> #define DA_FABRIC_MAX_SECTORS 8192
>> +/* Use a model alias based on the configfs backend device name */
>> +#define DA_EMULATE_MODEL_ALIAS 0
>> /* Emulation for Direct Page Out */
>> #define DA_EMULATE_DPO 0
>> /* Emulation for Forced Unit Access WRITEs */
>> @@ -590,6 +592,7 @@ struct se_dev_entry {
>> };
>>
>> struct se_dev_attrib {
>> + int emulate_model_alias;
>> int emulate_dpo;
>> int emulate_fua_write;
>> int emulate_fua_read;
>
>
--
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
