[Iprdd-devel] [PATCH] iprutils: Don't fail with minimum array rebuild rate.

[Gabriel Krisman Bertazi]

When user passed 0x0 to old firmwares, rebuild rate would be set to the
implementation default value, defined by SIS, and any further read to
Mode Page 0x24 would display 0x0 in the Rebuild Rate field.  Newer
firmwares, on the other hand, will correctly set the rebuild rate to the
implementation defined value, but the Mode Page field will display the
configured value, instead of 0x0.  This causes iprutils to believe the
new configuration has failed, and error out.

The Implementation defined value used by every IPR adapter out there is
the minimum rebuild rate possible, 2/16, which translates to 0x2 in the
Mode page field.  This patch gives iprutils the notion of the minimum
rebuild rate, and supports this scenario, so we don't fail when the user
requests a rebuild rate of 0.

To avoid confusing the user, we also make the values go from 10 to 100
instead of 0-100.  If the user wants to enable the default setting,
instead of typing 0, he needs to use 'default'.

Signed-off-by: Gabriel Krisman Bertazi 
---
 iprconfig.8 | 2 +-
 iprconfig.c | 5 +++--
 iprlib.c| 3 ++-
 iprlib.h| 2 ++
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/iprconfig.8 b/iprconfig.8
index 7ec3b92..82b6467 100644
--- a/iprconfig.8
+++ b/iprconfig.8
@@ -774,7 +774,7 @@ Example:
 .TP
 .B set-array-rebuild-rate [IOA] [Rebuild Rate | default]
 Used to set the rebuilt rate ratio of the IOA. [Rebuild Rate] must be in
-range 0..100. If 'default' is used, the IOA will reset to the
+range 10..100. If 'default' is used, the IOA will reset to the
 implementation default rate.  The value actually configured may not be
 the exact rate configured by the user, but an approximation to the
 closest rate supported by the IOA.
diff --git a/iprconfig.c b/iprconfig.c
index a6b1372..d071b6a 100644
--- a/iprconfig.c
+++ b/iprconfig.c
@@ -18194,10 +18194,11 @@ static int set_array_rebuild_rate(char**args, int 
num_args)
char *endptr = NULL;
rebuild_rate = strtol(args[1], , 10);
if (endptr == args[1]
-   || rebuild_rate < 0 || rebuild_rate > 100) {
+   || rebuild_rate < 10 || rebuild_rate > 100) {
scsi_err(dev,
 "'%s' is not a valid rebuild rate value.\n"
-"Supported values are in the range 0-100.\n"
+"Supported values are in the range 10-100.\n"
+"Use 'default' to reset the configuration.\n"
 "Higher values may affect performance but "
 "decrease the total rebuild time.\n", args[1]);
return -EINVAL;
diff --git a/iprlib.c b/iprlib.c
index 70e2924..1698a71 100644
--- a/iprlib.c
+++ b/iprlib.c
@@ -5596,7 +5596,8 @@ int ipr_set_array_rebuild_rate(struct ipr_ioa *ioa, u8 
rebuild_rate)
if (rc)
return rc;
 
-   if (rebuild_rate != page24->rebuild_rate)
+   if (rebuild_rate != page24->rebuild_rate &&
+   (rebuild_rate || page24->rebuild_rate != MIN_ARRAY_REBUILD_RATE))
return -EINVAL;
 
return 0;
diff --git a/iprlib.h b/iprlib.h
index b2a46c9..81d8eef 100644
--- a/iprlib.h
+++ b/iprlib.h
@@ -1748,6 +1748,8 @@ struct ipr_mode_page24 {
u8 rebuild_without_verify:1;
 #endif
 
+#define MIN_ARRAY_REBUILD_RATE 0x02
+
 #define DISABLE_DUAL_IOA_AF0x00
 #define ENABLE_DUAL_IOA_AF 0x02
 #define ENABLE_DUAL_IOA_ACTIVE_ACTIVE  0x03
-- 
2.1.0


--
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471=/4140
___
Iprdd-devel mailing list
Iprdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/iprdd-devel

Re: [Iprdd-devel] [PATCH v3] iprutils: Add maximum queue depth in GUI when creating an array

[Brian King]

Hi Gabriel,

This doesn't apply cleanly. Can you rebase the patch? Also, a couple of
minor comments below.

Thanks,

Brian

On 03/28/2016 12:15 PM, Gabriel Krisman Bertazi wrote:
> diff --git a/iprconfig.c b/iprconfig.c
> index 3189af0..3262816 100644
> --- a/iprconfig.c
> +++ b/iprconfig.c

> @@ -3665,6 +3665,7 @@ int configure_raid_parameters(i_container *i_con)
>   }
>   }
> 
> + max_qdepth = ipr_max_queue_depth(ioa, selected_count, ssd_num);
>   if (ioa->sis64)
>   qdepth = selected_count * 16;
>   else
> @@ -3783,7 +3784,7 @@ int configure_raid_parameters(i_container *i_con)
>   mvaddstr(6, 1, _("c=Change Setting"));
>   mvaddstr(8, 0,  "Protection Level . . . . . . . . . . . . :");
>   mvaddstr(9, 0,  "Stripe Size  . . . . . . . . . . . . . . :");
> - mvprintw(10, 0, "Queue Depth (default = %3d). . . . . . . :", qdepth);
> + mvprintw(10, 0, "Queue Depth (default = %3d)(max = %3d) . :", qdepth);

Aren't you missing a parameter here for the max queue depth?

>   if (ioa->vset_write_cache)
>   mvprintw(11, 0, "Array Write Cache Policy . . . . . . . . :");
>   mvaddstr(max_y - 4, 0, _("Press Enter to Continue"));

> diff --git a/iprlib.c b/iprlib.c
> index a899e1b..1e1d524 100644
> --- a/iprlib.c
> +++ b/iprlib.c
> @@ -2101,6 +2101,7 @@ static int __tool_init(int save_state)
>   DIR *dirfd, *host_dirfd;
>   struct dirent *dent, *host_dent;
>   ssize_t len;
> + char queue_str[256];
> 
>   save_old_config();
> 
> @@ -2158,7 +2159,12 @@ static int __tool_init(int save_state)
>   sscanf(fw_str, "%d", _type);
>   if (ipr_debug)
>   syslog(LOG_INFO, "tool_init: fw_type attr = 
> %d.\n", fw_type);
> - break;
> +
> + len = sysfs_read_attr(scsipath, "can_queue", queue_str, 
> 256);
> + if (len < 0)
> + ioa_dbg(ipr_ioa, "Failed to read can_queue 
> attribute");
> + sscanf(queue_str, "%d", _ioa->can_queue);
> + break;

Indentation issue with this break.

>   }
>   closedir(host_dirfd);
>   if (ipr_ioa->host_num < 0) {
> @@ -6943,6 +6949,8 @@ static void ipr_save_dev_attr(struct ipr_dev *dev, char 
> *field,
>char *value, int update)
>  {
>   char category[100];
> + struct ipr_dev *alt_dev = dev->alt_path;
> +
>   if (dev->scsi_dev_data->device_id)
>   sprintf(category,"[%s %lx]", IPR_CATEGORY_DEVICE,
>   dev->scsi_dev_data->device_id);


-- 
Brian King
Power Linux I/O
IBM Linux Technology Center


--
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471=/4140
___
Iprdd-devel mailing list
Iprdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/iprdd-devel

Re: [Iprdd-devel] [PATCH v2] iprutils: Enable time configuration for SES enclosures.

[Guilherme G. Piccoli]

On 03/29/2016 12:38 PM, Gabriel Krisman Bertazi wrote:
> Thanks for your review!
>
> yes, you are correct.  I added a return value for this function and
> changed the return type for init_ses_dev, as you suggested.

Very nice Gabriel, thank you.


>
>>> +int ipr_ses_get_time(struct ipr_dev *dev, u64* timestamp, int *origin);
>>> +int ipr_ses_set_time(struct ipr_dev *dev, u64 timestamp);
>>
>> Indentation seems weird here, but it's possibly my email client heheh
>
> hmm, indentation looks correct here.
>
> Please check the re-spin below.
>
> Thanks,

It's fine, for sure it's my email client playing tricks on me heheh
Thanks,


Guilherme


--
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471=/4140
___
Iprdd-devel mailing list
Iprdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/iprdd-devel

Re: [Iprdd-devel] [PATCH] iprutils: Enable time configuration for SES enclosures.

[Guilherme G. Piccoli]

Nice patch Gabriel! Some questions/comments below:


On 03/29/2016 11:02 AM, Gabriel Krisman Bertazi wrote:
> Some new external drawers like ESLS have an internal clock, used by the
> internal microcode.  This patch enables iprinit to configure the clock
> at boot time, and allow users to fetch and override this configuration
> in iprconfig.
>
> Signed-off-by: Gabriel Krisman Bertazi 
> ---
>   iprconfig.c | 52 +++-
>   iprlib.c| 42 ++
>   iprlib.h| 21 +
>   3 files changed, 114 insertions(+), 1 deletion(-)
>
> diff --git a/iprconfig.c b/iprconfig.c
> index a6b1372..89b6f4a 100644
> --- a/iprconfig.c
> +++ b/iprconfig.c
> @@ -27,7 +27,7 @@
>   #include 
>   #include 
>   #include 
> -
> +#include 
>   #include 
>
>   char *tool_name = "iprconfig";
> @@ -19149,6 +19149,54 @@ static int ssd_report(char **args, int num_args)
>   return 0;
>   }
>
> +static int get_ses_time(char **args, int num_args)
> +{
> + struct ipr_dev *dev = find_dev(args[0]);
> +
> + time_t timestamp;
> + int origin;
> + char* origin_str[] = { "Not Configured",
> +  "Configured" };
> + if (!dev) {
> + fprintf(stderr, "Cannot find %s\n", args[0]);
> + return -EINVAL;
> + }
> +
> + if (ipr_ses_get_time(dev, , ))
> + scsi_err(dev, "%s, Can't read enclosure time\n", dev->gen_name);
> +
> + /* ipr_ses_get_time returns its value in miliseconds, while epoch is
> +measured in seconds.  */
> + timestamp = timestamp/1000;
> +
> + printf ("%s(%s)\n", ctime(), origin_str[origin]);
> +}

Just curious here: the function is declared as int, so shouldn't it 
return an int value in case it succeeds?


> +
> +static int set_ses_time(char **args, int num_args)
> +{
> + struct ipr_dev *dev = find_dev(args[0]);
> + struct tm tm;
> + time_t timestamp;
> +
> +
> + if (!dev) {
> + fprintf(stderr, "Cannot find %s\n", args[0]);
> + return -EINVAL;
> + }
> +
> + if (!strptime (args[1], "%F", ) || !strptime (args[2], "%T", )) {
> + fprintf(stderr, "Date in the wrong format. Use \"-MM-DD 
> HH:MM:SS\"\n");
> + return -EINVAL;
> + }
> +
> + timestamp = mktime() * 1000;
> + if (ipr_ses_set_time(dev, timestamp)) {
> + scsi_err(dev, "%s, Can't set enclosure time\n", dev->gen_name);
> + return -EIO;
> + }
> + return 0;
> +}
> +
>   static const struct {
>   char *cmd;
>   int min_args;
> @@ -19270,6 +19318,8 @@ static const struct {
>   { "resume-disk-enclosure",  1, 0, 1, resume_disk_enclosure, 
> "sg8 "},
>   { "show-perf",  1, 0, 1, show_perf, "sg8"},
>   { "dump",   0, 0, 0, dump, "" },
> + { "get-ses-time",   1, 0, 1, get_ses_time, "sg8"},
> + { "set-ses-time",   3, 0, 3, set_ses_time, "sg8 
> -MM-DD HH:MM:SS"},
>   };
>
>   /**
> diff --git a/iprlib.c b/iprlib.c
> index 70e2924..8534274 100644
> --- a/iprlib.c
> +++ b/iprlib.c
> @@ -9535,6 +9535,15 @@ static void init_ioa_dev(struct ipr_dev *dev)
>   return;
>   }
>
> +int init_ses_dev(struct ipr_dev *dev)
> +{
> + time_t t = time(NULL);
> + if (t != ((time_t) -1)) {
> + t *= 1000;
> + ipr_ses_set_time(dev, t);
> + }
> +}

Same question as above: why not returning? Perhaps worth to declare it 
as void?


> +
>   /**
>* ipr_init_dev -
>* @dev:ipr dev struct
> @@ -9561,6 +9570,9 @@ int ipr_init_dev(struct ipr_dev *dev)
>   if (>ioa->ioa == dev)
>   init_ioa_dev(dev);
>   break;
> + case TYPE_ENCLOSURE:
> + init_ses_dev(dev);
> + break;
>   default:
>   break;
>   };
> @@ -10076,3 +10088,33 @@ int ipr_jbod_sysfs_bind(struct ipr_dev *dev, u8 op)
>   close(fd);
>   return 0;
>   }
> +
> +int ipr_ses_get_time(struct ipr_dev *dev, u64* timestamp, int *origin)
> +{
> + struct ipr_ses_diag_page12 get_time;
> + int err;
> +
> + err = ipr_receive_diagnostics(dev, 0x12, _time, sizeof(get_time));
> + if (err)
> + return -EIO;
> +
> + *origin = !!get_time.timestamp_origin;
> + *timestamp = be64toh(*((u64*) get_time.timestamp)) >> 16;
> + return 0;
> +}
> +
> +int ipr_ses_set_time(struct ipr_dev *dev, u64 timestamp)
> +{
> + struct ipr_ses_diag_ctrl_page13 set_time;
> +
> + memset(_time, '\0', sizeof(set_time));
> +
> + set_time.page_code = 0x13;
> + set_time.page_length[1] = 8;
> +
> + timestamp = htobe64(timestamp << 16);
> + memcpy(set_time.timestamp, (char*) &
> +timestamp, 6);
> +
> + return ipr_send_diagnostics(dev, _time, sizeof(set_time));
> +}
> diff --git a/iprlib.h 

[Iprdd-devel] [PATCH] iprutils: Enable time configuration for SES enclosures.

[Gabriel Krisman Bertazi]

Some new external drawers like ESLS have an internal clock, used by the
internal microcode.  This patch enables iprinit to configure the clock
at boot time, and allow users to fetch and override this configuration
in iprconfig.

Signed-off-by: Gabriel Krisman Bertazi 
---
 iprconfig.c | 52 +++-
 iprlib.c| 42 ++
 iprlib.h| 21 +
 3 files changed, 114 insertions(+), 1 deletion(-)

diff --git a/iprconfig.c b/iprconfig.c
index a6b1372..89b6f4a 100644
--- a/iprconfig.c
+++ b/iprconfig.c
@@ -27,7 +27,7 @@
 #include 
 #include 
 #include 
-
+#include 
 #include 
 
 char *tool_name = "iprconfig";
@@ -19149,6 +19149,54 @@ static int ssd_report(char **args, int num_args)
return 0;
 }
 
+static int get_ses_time(char **args, int num_args)
+{
+   struct ipr_dev *dev = find_dev(args[0]);
+
+   time_t timestamp;
+   int origin;
+   char* origin_str[] = { "Not Configured",
+"Configured" };
+   if (!dev) {
+   fprintf(stderr, "Cannot find %s\n", args[0]);
+   return -EINVAL;
+   }
+
+   if (ipr_ses_get_time(dev, , ))
+   scsi_err(dev, "%s, Can't read enclosure time\n", dev->gen_name);
+
+   /* ipr_ses_get_time returns its value in miliseconds, while epoch is
+  measured in seconds.  */
+   timestamp = timestamp/1000;
+
+   printf ("%s(%s)\n", ctime(), origin_str[origin]);
+}
+
+static int set_ses_time(char **args, int num_args)
+{
+   struct ipr_dev *dev = find_dev(args[0]);
+   struct tm tm;
+   time_t timestamp;
+
+
+   if (!dev) {
+   fprintf(stderr, "Cannot find %s\n", args[0]);
+   return -EINVAL;
+   }
+
+   if (!strptime (args[1], "%F", ) || !strptime (args[2], "%T", )) {
+   fprintf(stderr, "Date in the wrong format. Use \"-MM-DD 
HH:MM:SS\"\n");
+   return -EINVAL;
+   }
+
+   timestamp = mktime() * 1000;
+   if (ipr_ses_set_time(dev, timestamp)) {
+   scsi_err(dev, "%s, Can't set enclosure time\n", dev->gen_name);
+   return -EIO;
+   }
+   return 0;
+}
+
 static const struct {
char *cmd;
int min_args;
@@ -19270,6 +19318,8 @@ static const struct {
{ "resume-disk-enclosure",  1, 0, 1, resume_disk_enclosure, 
"sg8 "},
{ "show-perf",  1, 0, 1, show_perf, "sg8"},
{ "dump",   0, 0, 0, dump, "" },
+   { "get-ses-time",   1, 0, 1, get_ses_time, "sg8"},
+   { "set-ses-time",   3, 0, 3, set_ses_time, "sg8 
-MM-DD HH:MM:SS"},
 };
 
 /**
diff --git a/iprlib.c b/iprlib.c
index 70e2924..8534274 100644
--- a/iprlib.c
+++ b/iprlib.c
@@ -9535,6 +9535,15 @@ static void init_ioa_dev(struct ipr_dev *dev)
return;
 }
 
+int init_ses_dev(struct ipr_dev *dev)
+{
+   time_t t = time(NULL);
+   if (t != ((time_t) -1)) {
+   t *= 1000;
+   ipr_ses_set_time(dev, t);
+   }
+}
+
 /**
  * ipr_init_dev - 
  * @dev:   ipr dev struct
@@ -9561,6 +9570,9 @@ int ipr_init_dev(struct ipr_dev *dev)
if (>ioa->ioa == dev)
init_ioa_dev(dev);
break;
+   case TYPE_ENCLOSURE:
+   init_ses_dev(dev);
+   break;
default:
break;
};
@@ -10076,3 +10088,33 @@ int ipr_jbod_sysfs_bind(struct ipr_dev *dev, u8 op)
close(fd);
return 0;
 }
+
+int ipr_ses_get_time(struct ipr_dev *dev, u64* timestamp, int *origin)
+{
+   struct ipr_ses_diag_page12 get_time;
+   int err;
+
+   err = ipr_receive_diagnostics(dev, 0x12, _time, sizeof(get_time));
+   if (err)
+   return -EIO;
+
+   *origin = !!get_time.timestamp_origin;
+   *timestamp = be64toh(*((u64*) get_time.timestamp)) >> 16;
+   return 0;
+}
+
+int ipr_ses_set_time(struct ipr_dev *dev, u64 timestamp)
+{
+   struct ipr_ses_diag_ctrl_page13 set_time;
+
+   memset(_time, '\0', sizeof(set_time));
+
+   set_time.page_code = 0x13;
+   set_time.page_length[1] = 8;
+
+   timestamp = htobe64(timestamp << 16);
+   memcpy(set_time.timestamp, (char*) &
+  timestamp, 6);
+
+   return ipr_send_diagnostics(dev, _time, sizeof(set_time));
+}
diff --git a/iprlib.h b/iprlib.h
index b2a46c9..eccd116 100644
--- a/iprlib.h
+++ b/iprlib.h
@@ -2646,6 +2646,25 @@ struct ipr_ses_inquiry_pageC3 {
u8 reserved2;
 };
 
+/* Page 12h - Get Time */
+struct ipr_ses_diag_page12 {
+   u8 page_code;
+   u8 reserved1;
+   u8 page_length[2];
+   u8 timestamp_origin;
+   u8 reserved2;
+   u8 timestamp[6];
+};
+
+/* Page 13h - Set Time */
+struct ipr_ses_diag_ctrl_page13 {
+   u8 page_code;
+   u8 reserved1;
+   u8 

Re: [Iprdd-devel] [PATCH] iprutils: Fixup Undefined reference causes build to fail.

[Gabriel Krisman Bertazi]

Brian King  writes:

> On 03/28/2016 03:05 PM, Gabriel Krisman Bertazi wrote:
>> Commit 855e7fd ("iprutils: Enable editor choice for new log system")
>> introduced a new error code but the patch missed the symbol declaration,
>> causing the build to fail with an Undefined Reference error, like below.
>> 
>> /home/krisman/work/iprdd-iprutils/iprconfig.c:12141:10: error:
>> ‘RC_94_Tmp_Log_Fail’ undeclared (first ue in this function)
>>return RC_94_Tmp_Log_Fail;
>
> I had to manually merge Heitor's patches, and it looks like I left a couple
> unstaged changes in my local git repo. I've pushed these now. Please let me
> know if you still see issues.
>

thanks for the quick response!  The build works fine with the latest commit.

-- 
Gabriel Krisman Bertazi


--
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471=/4140
___
Iprdd-devel mailing list
Iprdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/iprdd-devel