| Message ID | 1499500354-6452-1-git-send-email-mchristi@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, 2017-07-08 at 02:52 -0500, Mike Christie wrote: > Userspace tools like targetcli are not able to dynamically detect if a > tcmu device supports kernel features ALUA or PGRs. In some kernels it > can create ALUA groups, but writing/reading ALUA files can lead to > nice error codes but also crashes and hangs. > > This patch, made over Nicks's for next branch, adds a device configfs > attribute to report the kernel features the device supports, so tools > can check that before attempting to activate some of these features > that were partially supported in older kernels. > > It can also be used for future feautes like possibly inquiry suppport > if we do end up moving that to the kernel so we can properly report > port info. > > Signed-off-by: Mike Christie <mchristi@redhat.com> > --- > drivers/target/target_core_user.c | 13 +++++++++++++ > include/uapi/linux/target_core_user.h | 3 +++ > 2 files changed, 16 insertions(+) Applied to target-pending/for-next. Thanks MNC. -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 2017-07-08 at 02:52 -0500, Mike Christie wrote: > Userspace tools like targetcli are not able to dynamically detect if a > tcmu device supports kernel features ALUA or PGRs. In some kernels it > can create ALUA groups, but writing/reading ALUA files can lead to > nice error codes but also crashes and hangs. > > This patch, made over Nicks's for next branch, adds a device configfs > attribute to report the kernel features the device supports, so tools > can check that before attempting to activate some of these features > that were partially supported in older kernels. > > It can also be used for future feautes like possibly inquiry suppport > if we do end up moving that to the kernel so we can properly report > port info. > > [ ... ] > +static ssize_t tcmu_features_show(struct config_item *item, char *page) > +{ > + struct se_dev_attrib *da = container_of(to_config_group(item), > + struct se_dev_attrib, da_group); > + struct tcmu_dev *udev = TCMU_DEV(da->da_dev); > + > + return snprintf(page, PAGE_SIZE, "0x%llx\n", udev->features); > +} > +CONFIGFS_ATTR_RO(tcmu_, features); > > [ ... ] > > diff --git a/include/uapi/linux/target_core_user.h b/include/uapi/linux/target_core_user.h > index 24a1c4e..03de6e9 100644 > --- a/include/uapi/linux/target_core_user.h > +++ b/include/uapi/linux/target_core_user.h > @@ -154,4 +154,7 @@ enum tcmu_genl_attr { > }; > #define TCMU_ATTR_MAX (__TCMU_ATTR_MAX - 1) > > +#define TCMU_KERN_ALUA (1ULL << 0) > +#define TCMU_KERN_PGR (1ULL << 1) Hello Mike, Wouldn't it be more convenient for shell scripts if the state of these flags would be exported through two configfs attributes instead of a single bitfield? Wouldn't that be more consistent with other sysfs/configfs drivers that export boolean values? Thanks, Bart.
On 07/10/2017 07:27 AM, Bart Van Assche wrote: > On Sat, 2017-07-08 at 02:52 -0500, Mike Christie wrote: >> Userspace tools like targetcli are not able to dynamically detect if a >> tcmu device supports kernel features ALUA or PGRs. In some kernels it >> can create ALUA groups, but writing/reading ALUA files can lead to >> nice error codes but also crashes and hangs. >> >> This patch, made over Nicks's for next branch, adds a device configfs >> attribute to report the kernel features the device supports, so tools >> can check that before attempting to activate some of these features >> that were partially supported in older kernels. >> >> It can also be used for future feautes like possibly inquiry suppport >> if we do end up moving that to the kernel so we can properly report >> port info. >> >> [ ... ] >> +static ssize_t tcmu_features_show(struct config_item *item, char *page) >> +{ >> + struct se_dev_attrib *da = container_of(to_config_group(item), >> + struct se_dev_attrib, da_group); >> + struct tcmu_dev *udev = TCMU_DEV(da->da_dev); >> + >> + return snprintf(page, PAGE_SIZE, "0x%llx\n", udev->features); >> +} >> +CONFIGFS_ATTR_RO(tcmu_, features); >> >> [ ... ] >> >> diff --git a/include/uapi/linux/target_core_user.h b/include/uapi/linux/target_core_user.h >> index 24a1c4e..03de6e9 100644 >> --- a/include/uapi/linux/target_core_user.h >> +++ b/include/uapi/linux/target_core_user.h >> @@ -154,4 +154,7 @@ enum tcmu_genl_attr { >> }; >> #define TCMU_ATTR_MAX (__TCMU_ATTR_MAX - 1) >> >> +#define TCMU_KERN_ALUA (1ULL << 0) >> +#define TCMU_KERN_PGR (1ULL << 1) > > > Hello Mike, > > Wouldn't it be more convenient for shell scripts if the state of these flags > would be exported through two configfs attributes instead of a single bitfield? > Wouldn't that be more consistent with other sysfs/configfs drivers that export > boolean values? > For scripts I think it would be easier. I will send a patch on top of these to fix it up. -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c index 2f1fa92..b891824 100644 --- a/drivers/target/target_core_user.c +++ b/drivers/target/target_core_user.c @@ -111,6 +111,7 @@ struct tcmu_dev { char *name; struct se_hba *hba; + uint64_t features; #define TCMU_DEV_BIT_OPEN 0 #define TCMU_DEV_BIT_BROKEN 1 @@ -1104,6 +1105,7 @@ static struct se_device *tcmu_alloc_device(struct se_hba *hba, const char *name) udev->hba = hba; udev->cmd_time_out = TCMU_TIME_OUT; + udev->features |= (TCMU_KERN_ALUA | TCMU_KERN_PGR); init_waitqueue_head(&udev->wait_cmdr); mutex_init(&udev->cmdr_lock); @@ -1882,11 +1884,22 @@ static ssize_t tcmu_emulate_write_cache_store(struct config_item *item, } CONFIGFS_ATTR(tcmu_, emulate_write_cache); +static ssize_t tcmu_features_show(struct config_item *item, char *page) +{ + struct se_dev_attrib *da = container_of(to_config_group(item), + struct se_dev_attrib, da_group); + struct tcmu_dev *udev = TCMU_DEV(da->da_dev); + + return snprintf(page, PAGE_SIZE, "0x%llx\n", udev->features); +} +CONFIGFS_ATTR_RO(tcmu_, features); + static struct configfs_attribute *tcmu_attrib_attrs[] = { &tcmu_attr_cmd_time_out, &tcmu_attr_dev_config, &tcmu_attr_dev_size, &tcmu_attr_emulate_write_cache, + &tcmu_attr_features, NULL, }; diff --git a/include/uapi/linux/target_core_user.h b/include/uapi/linux/target_core_user.h index 24a1c4e..03de6e9 100644 --- a/include/uapi/linux/target_core_user.h +++ b/include/uapi/linux/target_core_user.h @@ -154,4 +154,7 @@ enum tcmu_genl_attr { }; #define TCMU_ATTR_MAX (__TCMU_ATTR_MAX - 1) +#define TCMU_KERN_ALUA (1ULL << 0) +#define TCMU_KERN_PGR (1ULL << 1) + #endif
Userspace tools like targetcli are not able to dynamically detect if a tcmu device supports kernel features ALUA or PGRs. In some kernels it can create ALUA groups, but writing/reading ALUA files can lead to nice error codes but also crashes and hangs. This patch, made over Nicks's for next branch, adds a device configfs attribute to report the kernel features the device supports, so tools can check that before attempting to activate some of these features that were partially supported in older kernels. It can also be used for future feautes like possibly inquiry suppport if we do end up moving that to the kernel so we can properly report port info. Signed-off-by: Mike Christie <mchristi@redhat.com> --- drivers/target/target_core_user.c | 13 +++++++++++++ include/uapi/linux/target_core_user.h | 3 +++ 2 files changed, 16 insertions(+)