Hi Tregaron,
On Thu, 2013-01-31 at 15:30 -0700, Tregaron Bayly wrote:
> This patch changes LIO to use the configfs backend device name as the
> model if you echo '1' to an individual device's emulate_model_alias attribute.
> This is a valid operation only on devices with an export count of 0.
>
> Signed-off-by: Tregaron Bayly <tbayly@xxxxxxxxxxxx>
> ---
This looks good. Just a few minor items below that I've ended up
changing ahead of applying your patch.
> drivers/target/target_core_configfs.c | 4 ++++
> drivers/target/target_core_device.c | 40 +++++++++++++++++++++++++++++++++++
> drivers/target/target_core_internal.h | 1 +
> include/target/target_core_base.h | 3 +++
> 4 files changed, 48 insertions(+)
>
> 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 39d56eb..595bd80 100644
> --- a/drivers/target/target_core_device.c
> +++ b/drivers/target/target_core_device.c
> @@ -713,6 +713,45 @@ int se_dev_set_max_write_same_len(
> return 0;
> }
>
> +static void dev_set_t10_wwn_model_alias(struct se_device *dev)
> +{
> + const char *configname;
> +
> + configname = config_item_name(&dev->dev_group.cg_item);
> + strncpy(&dev->t10_wwn.model[0], configname, 16);
> + if (strlen(configname) >= 16) {
> + pr_warn("dev[%p]: Backstore name '%s' is too long for "
> + "INQUIRY_MODEL, truncating to 16 bytes", dev, configname);
> + }
> + dev->t10_wwn.model[15] = '\0';
> +}
> +
The strncpy() + '\0' assignment can be made into a snprintf() instead.
Changing the above into the following slightly cleaner version:
static void dev_set_t10_wwn_model_alias(struct se_device *dev)
{
const char *configname;
configname = config_item_name(&dev->dev_group.cg_item);
if (strlen(configname) >= 16) {
pr_warn("dev[%p]: Backstore name '%s' is too long for "
"INQUIRY_MODEL, truncating to 16 bytes\n", dev,
configname);
}
snprintf(&dev->t10_wwn.model[0], 16, "%s", configname);
}
> +int se_dev_set_emulate_model_alias(struct se_device *dev, int flag)
> +{
> + if (dev->export_count) {
> + pr_err("dev[%p]: Unable to change model alias"
> + " while export_count is %d\n",
> + dev, dev->export_count);
> + return -EINVAL;
> + }
> +
> + if (flag != 0 && flag != 1) {
> + pr_err("Illegal value %d\n", flag);
> + return -EINVAL;
> + }
> +
> + if (flag) {
> + dev_set_t10_wwn_model_alias(dev);
> + }
> + else {
Minor style nit. Changing this 'else {' to be '} else {'
Folding these two changes into this patch, and applying now to
target-pending/for-next.
Thanks Tregaron!
--nab
> + strncpy(&dev->t10_wwn.model[0],
> + dev->transport->inquiry_prod, 16);
> + }
> + dev->dev_attrib.emulate_model_alias = flag;
> +
> + return 0;
> +}
> +
> int se_dev_set_emulate_dpo(struct se_device *dev, int flag)
> {
> if (flag != 0 && flag != 1) {
> @@ -1390,6 +1429,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;
> diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
> index 93e9c1f..fdc51f8 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);
> diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
> index 01e8601..d6b30d7 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
