powerpc-utils-devel — powerpc-utils development

[Powerpc-utils-devel] [PATCH 2/2] update_flash: Use device tree to get firmware version details

[Vasant H. <heg...@li...>]

On P9, OPAL exports firmware information via device tree
(/ibm,firmware-versions node). Even recent hostboot firmware on P8 BMC system
exports these information via mini device tree.

Lets add support to parse device tree to get firmware information. If firmware
information is not present in device tree then fall back to existing ipmi
method.

Signed-off-by: Vasant Hegde <heg...@li...>
---
 scripts/update_flash_nv |   50 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/scripts/update_flash_nv b/scripts/update_flash_nv
index 0ef765c..cb51a8d 100755
--- a/scripts/update_flash_nv
+++ b/scripts/update_flash_nv
@@ -58,6 +58,9 @@ DT_PATH=/proc/device-tree
 DT_FW_MI_FILE=${DT_PATH}/ibm,opal/firmware/mi-version
 DT_FW_ML_FILE=${DT_PATH}/ibm,opal/firmware/ml-version
 
+# Firmware versions device tree node
+DT_PATH_FW_NODE=${DT_PATH}/ibm,firmware-versions
+
 # Code update status values
 FLASH_SUCCESS=0			# Success
 FLASH_PARAM_ERR=-1		# Parameter error
@@ -587,7 +590,44 @@ opp_manage_flash() {
 	exit $E_SUCCESS
 }
 
-opp_display_current_fw_version() {
+# Use device tree to get firmware version
+opp_display_dt_firmware_version() {
+	local prop_prod=""
+
+	echo
+	echo "Firmware version:"
+
+	# Different BMCs uses different device tree property to display
+	# product name.
+	if [ -f $DT_PATH_FW_NODE/IBM ]; then
+		prop_prod=IBM
+	elif [ -f $DT_PATH_FW_NODE/open-power ]; then
+		prop_prod=open-power
+	elif [ -f $DT_PATH_FW_NODE/buildroot ]; then
+		prop_prod=buildroot
+	fi
+
+	if [ "$prop_prod" != "" ]; then
+		echo " Product Version       : $prop_prod-$(tr -d '\0' < $DT_PATH_FW_NODE/$prop_prod)"
+	fi
+
+	for i in `ls $DT_PATH_FW_NODE`
+	do
+		if [ "$i" = "$prop_prod" ]; then
+			continue
+		fi
+		if [ "$i" = "name" ]; then
+			continue
+		fi
+		if [ "$i" = "phandle" ] || [ "$i" = "linux,phandle" ]; then
+			continue
+		fi
+		echo " Product Extra         : $i-$(tr -d '\0' < $DT_PATH_FW_NODE/$i)"
+	done
+}
+
+# Use inband ipmi interface to get firmware version
+opp_display_ipmi_fw_version() {
 	which ipmitool >/dev/null 2>&1
 	if [ $? -ne 0 ]; then
 		echo "update_flash: ipmitool command not found."
@@ -609,6 +649,14 @@ opp_display_current_fw_version() {
 
 	echo "Firmware version:"
 	ipmitool fru print $id
+}
+
+opp_display_current_fw_version() {
+	if [ -d $DT_PATH_FW_NODE ]; then
+		opp_display_dt_firmware_version
+	else
+		opp_display_ipmi_fw_version
+	fi
 
 	exit $E_SUCCESS
 }

[Powerpc-utils-devel] [PATCH 1/2] update_flash: Use inband ipmi path to get firmware version

[Vasant H. <heg...@li...>]

update_flash_nv uses 'ipmitool -I usb' interface to get firmware version
details. This is AMI BMC OEM feature and not available on other BMC systems.
Hence lets use inband interface to get firmware version details.

Signed-off-by: Vasant Hegde <heg...@li...>
---
 scripts/update_flash_nv |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/scripts/update_flash_nv b/scripts/update_flash_nv
index 0cec2eb..0ef765c 100755
--- a/scripts/update_flash_nv
+++ b/scripts/update_flash_nv
@@ -588,16 +588,27 @@ opp_manage_flash() {
 }
 
 opp_display_current_fw_version() {
-	check_ipmitool
+	which ipmitool >/dev/null 2>&1
+	if [ $? -ne 0 ]; then
+		echo "update_flash: ipmitool command not found."
+		error $E_IPMI "Please install ipmitool and retry."
+	fi
+
+	# Use inband interface to get firmware version
+	ipmitool mc info >/dev/null 2>&1 && break;
+	if [ $? -ne 0 ]; then
+		echo "update_flash: ipmi inband interface is not working."
+		error $E_IPMI "Check 'ipmi_devintf' kernel module loaded or not."
+	fi
 
 	echo
-	id=`ipmitool -I usb fru 2>/dev/null |grep "System Firmware" | cut -d " " -f8 | cut -d ")" -f1`
+	id=`ipmitool fru 2>/dev/null |grep "System Firmware" | cut -d " " -f8 | cut -d ")" -f1`
 	if [ "x$id" = "x" ]; then
 		error $E_IPMI "Failed to get firmware version."
 	fi
 
 	echo "Firmware version:"
-	ipmitool -I usb fru print $id
+	ipmitool fru print $id
 
 	exit $E_SUCCESS
 }

[Powerpc-utils-devel] [PATCH] Handle --help option for bootlist

[Harish <ha...@li...>]

This patch handles --help option for bootlist command and returns
with 0

Signed-off-by: Harish <ha...@li...>
---
 scripts/bootlist | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/bootlist b/scripts/bootlist
index 771389a..187b08c 100755
--- a/scripts/bootlist
+++ b/scripts/bootlist
@@ -422,6 +422,10 @@ while [[ -n $1 ]]; do
 	    add_logical $i
 	done
 	shift
+    elif [[ $1 = "--help" ]]; then
+       # display bootlist command help message
+       usage
+       exit 0
     elif [[ $1 = -* ]]; then
     	# catch any illegal flags here
 	usage
-- 
2.7.4

Re: [Powerpc-utils-devel] Changes to lparstat (1.2.26)

[Nathan F. <nf...@li...>]

On 01/06/2016 01:40 AM, Sanders, Miguel wrote:
> Hi guys
> 
> How are you?
> We're currently deploying SAP HANA on POWER and I found some defects in the lparstat (1.2.26) provided by the powerpc-utils package.
> Can you please revise the attached patch files and push the code.
> 
> 1) If no count is provided, lparstat should run indefinitely given the specified interval
> 2) physc calculation has been corrected. get_time() function didn't take t.tv_usec into account properly.
> 3) %entc calculation has been corrected.
> 4) smt has been corrected. Information is now gathered from the --threads-per-core option of ppc64_cpu
> 5) user/sys/wait/idle time has been corrected. get_cpu_stat() function didn't properly calculate the percentage
> 6) shared/dedicated memory is now derived from entitled_memory_weight. entitled_memory_weight=0 indicates dedicated memory.
> 7) memory is measured in MB (like lparstat in AIX)
> 8) added function get_cpu_dispatches() (in order to calculate the diff)
> 9) added function get_cpu_interrupts() (in order to calculate the diff)
> 10) lcpu calculation has been corrected. added function get_cpu_lcpu() function.
> 11) similar to lparstat on AIX, every 21 iterations, the header is shown.

Could you submit this as a series of patches, one patch for each of the
issues you have listed. This makes it much easier to determine which changes
go with what issue that you are trying to address.

Also, please put your patches inline instead of as attachments.

-Nathan
> 
> Many thanks
> 
> Sincerely
> 
> -- 
> Met vriendelijke groeten
> Best regards
> 
> *Miguel Sanders*
> ArcelorMittal Europe – Flat Products – Business Division North
> 
> External collaborator | Midrange UNIX
> John Kennedylaan 51 B-9042 Gent
> *T* +32 9 347 52 78
> *E* gen...@ar...
> *E* mig...@ar...
> 
> 
> 
> 
> ------------------------------------------------------------------------------
> 
> 
> 
> _______________________________________________
> Powerpc-utils-devel mailing list
> Pow...@li...
> https://lists.sourceforge.net/lists/listinfo/powerpc-utils-devel
> 

[Powerpc-utils-devel] Changes to lparstat (1.2.26)

[Sanders, M. <mig...@ar...>]

Hi guys

How are you?
We're currently deploying SAP HANA on POWER and I found some defects in the lparstat (1.2.26) provided by the powerpc-utils package.
Can you please revise the attached patch files and push the code.

1) If no count is provided, lparstat should run indefinitely given the specified interval
2) physc calculation has been corrected. get_time() function didn't take t.tv_usec into account properly.
3) %entc calculation has been corrected.
4) smt has been corrected. Information is now gathered from the --threads-per-core option of ppc64_cpu
5) user/sys/wait/idle time has been corrected. get_cpu_stat() function didn't properly calculate the percentage
6) shared/dedicated memory is now derived from entitled_memory_weight. entitled_memory_weight=0 indicates dedicated memory.
7) memory is measured in MB (like lparstat in AIX)
8) added function get_cpu_dispatches() (in order to calculate the diff)
9) added function get_cpu_interrupts() (in order to calculate the diff)
10) lcpu calculation has been corrected. added function get_cpu_lcpu() function.
11) similar to lparstat on AIX, every 21 iterations, the header is shown.

Many thanks

Sincerely

--
Met vriendelijke groeten
Best regards

Miguel Sanders
ArcelorMittal Europe – Flat Products – Business Division North

External collaborator | Midrange UNIX
John Kennedylaan 51 B-9042 Gent
T +32 9 347 52 78
E gen...@ar...<mailto:gen...@ar...>
E mig...@ar...<mailto:mig...@ar...>

[Powerpc-utils-devel] Ubuntu 14.04.4 beta images available

[Breno L. <br...@br...>]

Ubuntu 14.04.4 beta images were made available today by 
Canonical. Ubuntu 14.04.4 ships with Wily kernel, version 4.2.

ISO: http://cdimage.ubuntu.com/ubuntu-server/trusty/daily/current/

Netboot: http://ports.ubuntu.com/ubuntu-ports/dists/trusty-proposed/main/installer-ppc64el/current/images/wily-netboot/

Re: [Powerpc-utils-devel] Migration to github

[Michael E. <mi...@el...>]

On Tue, 2015-07-28 at 11:56 -0500, Nathan Fontenot wrote:
> The powerpc-utils and powerpc-utils-python projects are moving to
> github. Due to the outages on sourceforge and other issues this is
> a move I feel should be done. I don't do this lightly since I know
> any move will cause disruptions, hopefully there will be none.
> 
> The new git repos for the projects are:
> https://github.com/nfont/powerpc-utils
> https://github.com/nfont/powerpc-utils-python
> 
> >From this point forward I plan to only push patches to the github repos.

There is no indication (that I can see) in the sourceforge repo that the
migration has happened.

Which means if you have an existing clone you can do a "git pull" and see no
updates and just think there are no updates.

What some other projects have done is push a commit to the sourceforge
repository only which says "We've moved to github", so at least folks will see
that when they pull and know that they need to update their remote.

eg. http://sourceforge.net/p/libhugetlbfs/code/ci/680e65dccaee1875cea5004815e7c0790d0b92b8/

In fact they have deleted everything except the README, which would also help
to get folks attention.


And they also have a nice banner across the top of the sourceforge page
mentioning the move has happened, eg:

http://sourceforge.net/projects/libhugetlbfs/

cheers

[Powerpc-utils-devel] [PATCH] drmgr: Combine the init_node() and examine_child() routines

[Nathan F. <nf...@li...>]

The complexity of the init_node() and examine_child() routines that call
each other recursively is causing drmgr to segfault in odd and sometimes
slightly differing ways. Each instance does involve drmgr segfaulting in
libc malloc code.

This patch re-writes these two routines to combine them into a single
routine that is called recursively. The result is cleaner code that no
longer segfaults.

Signed-off-by: Nathan Fontenot <nf...@li...>
---
 src/drmgr/common_pci.c |  100 ++++++++++++++++--------------------------------
 1 file changed, 34 insertions(+), 66 deletions(-)

diff --git a/src/drmgr/common_pci.c b/src/drmgr/common_pci.c
index 04a8a35..590c9b2 100644
--- a/src/drmgr/common_pci.c
+++ b/src/drmgr/common_pci.c
@@ -304,11 +304,6 @@ add_child_node(struct dr_node *parent, char *child_path)
 	parent->children = child;
 }
 
-/* This forward declaration is needed because init_node and examine_child
- * call each other.
- */
-static int examine_child(struct dr_node *, char *);
-
 /**
  * init_node
  *
@@ -318,82 +313,55 @@ static int examine_child(struct dr_node *, char *);
 static int
 init_node(struct dr_node *node)
 {
-	struct dirent **de_list, *de;
-	char *newpath;
-	int count;
-	int rc, i;
+	DIR *d;
+	struct dirent *de;
+	char child_path[DR_PATH_MAX];
+	uint32_t my_drc_index;
+	int rc;
 
 	if (node->is_owned)
 		find_ofdt_dname(node, node->ofdt_path);
 
-	count = scandir(node->ofdt_path, &de_list, 0, alphasort);
-	for (i = 0; i < count; i++) {
-		de = de_list[i];
+	d = opendir(node->ofdt_path);
+	if (!d)
+		return -1;
+
+	rc = 0;
+	while ((de = readdir(d)) != NULL) {
 		if ((de->d_type != DT_DIR) || is_dot_dir(de->d_name))
 			continue;
 
-		newpath = zalloc(strlen(node->ofdt_path) +
-				 strlen(de->d_name) + 2);
-		if (newpath == NULL) {
-			say(ERROR, "Could not allocate path for node at "
-			    "%s/%s\n", node->ofdt_path, de->d_name);
-			return 1;
-		}
-
-		sprintf(newpath, "%s/%s", node->ofdt_path, de->d_name);
-		rc = examine_child(node, newpath);
-		if (rc)
-			return rc;
-	}
-
-	return 0;
-}
-
-/**
- * examine_children
- *
- * @param node
- * @param child_path
- * @returns 0 on success, !0 otherwise
- */
-static int
-examine_child(struct dr_node *node, char *child_path)
-{
-	uint32_t my_drc_index;
-	int used = 0;
-	int rc = 0;
+		sprintf(child_path, "%s/%s", node->ofdt_path, de->d_name);
 
-	if (get_my_drc_index(child_path, &my_drc_index))
-		goto done;
+		if (get_my_drc_index(child_path, &my_drc_index))
+			continue;
 
-	if (node->dev_type == PCI_HP_DEV) {
-		if (node->drc_index == my_drc_index) {
-			/* Add hotplug children */
-			add_child_node(node, child_path);
-			used = 1;
-		}
-	} else {
-		if (! node->is_owned) {
+		if (node->dev_type == PCI_HP_DEV) {
 			if (node->drc_index == my_drc_index) {
-				/* Update node path */
-				snprintf(node->ofdt_path, DR_PATH_MAX, "%s",
-					 child_path);
-				node->is_owned = 1;
-				used = 1;
-
-				/* Populate w/ children */
-				rc = init_node(node);
+				/* Add hotplug children */
+				add_child_node(node, child_path);
 			}
 		} else {
-			/* Add all DR-capable children */
-			add_child_node(node, child_path);
-			used = 1;
+			if (!node->is_owned) {
+				if (node->drc_index == my_drc_index) {
+					/* Update node path */
+					snprintf(node->ofdt_path, DR_PATH_MAX,
+						 "%s", child_path);
+					node->is_owned = 1;
+
+					/* Populate w/ children */
+					rc = init_node(node);
+					if (rc)
+						break;
+				}
+			} else {
+				/* Add all DR-capable children */
+				add_child_node(node, child_path);
+			}
 		}
 	}
-done:
-	if (! used)
-		free(child_path);
 
+	closedir(d);
 	return rc;
 }
 

[Powerpc-utils-devel] Migration to github

[Nathan F. <nf...@li...>]

The powerpc-utils and powerpc-utils-python projects are moving to
github. Due to the outages on sourceforge and other issues this is
a move I feel should be done. I don't do this lightly since I know
any move will cause disruptions, hopefully there will be none.

The new git repos for the projects are:
https://github.com/nfont/powerpc-utils
https://github.com/nfont/powerpc-utils-python

>From this point forward I plan to only push patches to the github repos.

This move will also result in moving the mailing list to google groups.
Unless anyone specifically requests not to be added to the new mailing
list I will be adding everyone that is currently subscribed to this
mailing list to the google groups mailing list. If you do not
want to be added let me know (nf...@li...).

The new mailing list is https://groups.google.com/forum/#!forum/powerpc-utils-devel

If anyone has any questions/thoughts/concerns? let me know.

Thanks,
-Nathan

Re: [Powerpc-utils-devel] Result of static analysis tools

[Nathan F. <nf...@li...>]

Hi Rafael,

I saw this note come through a week or do ago but because of sourceforge
being down it is just now coming through on the mailing list.

Thanks for doing the analysis, I'll take a look at the results and start
working on some updates.

-Nathan

On 07/27/2015 08:36 AM, Rafael dos Santos wrote:
> Hi,
> 
> I ran some static analysis tools (clang, cppcheck) on the latest tag of powerpc-utils codebase. You can find the results attached. I hope they can be useful.
> 
> 
> Att
> --
> Rafael Fonseca
> 
> 
> 
> ------------------------------------------------------------------------------
> 
> 
> 
> _______________________________________________
> Powerpc-utils-devel mailing list
> Pow...@li...
> https://lists.sourceforge.net/lists/listinfo/powerpc-utils-devel
> 

[Powerpc-utils-devel] Result of static analysis tools

[Rafael d. S. <rdo...@re...>]

Hi,

I ran some static analysis tools (clang, cppcheck) on the latest tag of powerpc-utils codebase. You can find the results attached. I hope they can be useful.


Att
--
Rafael Fonseca

[Powerpc-utils-devel] Test, please ignore (eom)

[Nathan F. <nf...@li...>]

[Powerpc-utils-devel] [TRIVIAL PATCH] Fix typo(s) across the file.

[Kamalesh B. <kam...@li...>]

Fix trivial spelling mistakes and trailing white spaces.
'avaiable' -> 'available'
'parmaeter' -> 'parameter'
'intialize' -> 'initialize'

Signed-off-by: Kamalesh Babulal <kam...@li...>
---
 src/drmgr/drslot_chrp_cpu.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/drmgr/drslot_chrp_cpu.c b/src/drmgr/drslot_chrp_cpu.c
index 54fc576..4c193fa 100644
--- a/src/drmgr/drslot_chrp_cpu.c
+++ b/src/drmgr/drslot_chrp_cpu.c
@@ -150,7 +150,7 @@ get_available_cpu(struct options *opts, struct dr_info *dr_info)
 	}
 
 	if (!cpu)
-		say(ERROR, "Could not find avaiable cpu.\n");
+		say(ERROR, "Could not find available cpu.\n");
 
 	return cpu;
 }
@@ -235,7 +235,7 @@ remove_cpus(struct options *opts, struct dr_info *dr_info)
 	while (count < opts->quantity) {
 		if (drmgr_timed_out())
 			break;
-		
+
 		if (cpu_count(dr_info) == 1) {
 			say(WARN, "Cannot remove the last CPU\n");
 			rc = -1;
@@ -365,14 +365,14 @@ drslot_chrp_cpu(struct options *opts)
 		    (strcmp(opts->p_option, "variable_weight") == 0)) {
 			rc = update_sysparm(opts);
 			if (rc)
-				say(ERROR, "Could not update system parmaeter "
+				say(ERROR, "Could not update system parameter "
 				    "%s\n", opts->p_option);
 			return rc;
 		}
 	}
 
 	if (init_cpu_drc_info(&dr_info)) {
-		say(ERROR, "Could not intialize Dynamic Reconfiguration "
+		say(ERROR, "Could not initialize Dynamic Reconfiguration "
 		    "information.\n");
 		return -1;
 	}
-- 
2.1.2

Re: [Powerpc-utils-devel] [PATCH] Cleanup build warnings

[Kamalesh B. <kam...@li...>]

On 06/30/2015 10:42 PM, Nathan Fontenot wrote:
> Code cleanup to get rid of warnings generated when building powerpc-utils.
>
> For the most part this involves simple code changes. For the serv_config
> the code updates needed to get rid of the strict-aliasing warnings I thought
> to be more work than was worth the effort so I simply disabled strict-aliasing
> for this command.
>
> Signed-off-by: Nathan Fontenot <nf...@li...>
> ---
[...]

This patch fixes the lparstat.c. 'unsued' variable warning.

>  /**
> diff --git a/src/drmgr/common_cpu.c b/src/drmgr/common_cpu.c
> index e4c8082..c36f669 100644
> --- a/src/drmgr/common_cpu.c
> +++ b/src/drmgr/common_cpu.c
> @@ -194,8 +194,12 @@ cpu_index_to_path(struct dr_node *cpu)
>
>  	closedir(d);
>
> -	if (found)
> +	if (found) {
>  		snprintf(cpu->ofdt_path, DR_PATH_MAX, "%s", path);
> +		rc = 0;
> +	} else {
> +		rc = -1;
> +	}

We can avoid this else block by declaring  = -1;

>  	return rc;
>  }
[...]

> diff --git a/src/drmgr/drslot_chrp_cpu.c b/src/drmgr/drslot_chrp_cpu.c
> index 54fc576..f34854c 100644
> --- a/src/drmgr/drslot_chrp_cpu.c
> +++ b/src/drmgr/drslot_chrp_cpu.c
> @@ -352,8 +352,8 @@ int
>  drslot_chrp_cpu(struct options *opts)
>  {
>  	struct dr_info dr_info;
> -	int rc = -1;
> -
> +	int rc;
> +	

Above line introduces trailing white spaces.

>  	if (! cpu_dlpar_capable()) {
>  		say(ERROR, "CPU DLPAR capability is not enabled on this "
>  		    "platform.\n");
> @@ -386,15 +386,20 @@ drslot_chrp_cpu(struct options *opts)
>
>  	if (opts->p_option && (strcmp(opts->p_option, "smt_threads") == 0)) {
>  		rc = smt_threads_func(opts, &dr_info);
> -	} else {
> -		switch (opts->action) {
> -		    case ADD:
> -			rc = add_cpus(opts, &dr_info);
> -			break;
> -		    case REMOVE:
> -			rc = remove_cpus(opts, &dr_info);
> -			break;
> -		}
> +		free_cpu_drc_info(&dr_info);
> +		return rc;
> +	} 
> +	

Above line introduces trailing white spaces.
> +	switch (opts->action) {
> +	case ADD:
> +		rc = add_cpus(opts, &dr_info);
> +		break;
> +	case REMOVE:
> +		rc = remove_cpus(opts, &dr_info);
> +		break;
> +	default:
> +		rc = -1;
> +		break;
>  	}
>
>  	free_cpu_drc_info(&dr_info);
[...]

> diff --git a/src/serv_config.c b/src/serv_config.c
> index 46e02f3..900f0f3 100644
> --- a/src/serv_config.c
> +++ b/src/serv_config.c
> @@ -463,7 +463,7 @@ int
>  update_nvram(char *var, char *val, char *partition) {
>  	char buf[256];
>  	pid_t child;
> -	int status, rc;
> +	int status;
>  	char *nvram_args[] = { "nvram", "--update-config",
>  			buf, "-p", partition, NULL };
>
> @@ -482,12 +482,12 @@ update_nvram(char *var, char *val, char *partition) {
>  	}
>  	else if (child == 0) {
>  		/* child process */
> -		rc = execv(NVRAM_PROGRAM, nvram_args);
> -
> -		/* shouldn't get here */
> -		err_msg(ERR_MSG, "Could not exec %s to update NVRAM\n",
> +		if (execv(NVRAM_PROGRAM, nvram_args)) {
> +			/* shouldn't get here */
> +			err_msg(ERR_MSG, "Could not exec %s to update NVRAM\n",
>  				NVRAM_PROGRAM);
> -		exit(1);
> +			exit(1);
> +		}
>  	}
>  	else {
>  		/* parent process */
> @@ -609,12 +609,11 @@ byte_to_string(uint8_t num, char *buf, size_t size) {
>   */
>  int
>  parse_call_home_buffer(char *var, char *buf, size_t size) {
> -	int buf_size;
>  	char *loc;
>
>  	if (!call_home_buffer) return RC_OTHER;	/* should never happen */
>
> -	buf_size = be16toh(*(uint16_t *)call_home_buffer);
> +	/* The first 16 bits is the call home bufer size, skip past this. */

Spelling of 'buffer' is misspelled.

>  	loc = call_home_buffer + sizeof(uint16_t);
>
>  	while (loc[0] != '\0') {
>

-- 
Cheers,
Kamalesh.

[Powerpc-utils-devel] [PATCH] Cleanup build warnings

[Nathan F. <nf...@li...>]

Code cleanup to get rid of warnings generated when building powerpc-utils.

For the most part this involves simple code changes. For the serv_config
the code updates needed to get rid of the strict-aliasing warnings I thought
to be more work than was worth the effort so I simply disabled strict-aliasing
for this command.

Signed-off-by: Nathan Fontenot <nf...@li...>
---
 Makefile.am                 |    1 +
 src/drmgr/common.c          |   20 +++++++++++++-------
 src/drmgr/common_cpu.c      |    6 +++++-
 src/drmgr/common_pci.c      |    7 ++++++-
 src/drmgr/drmig_chrp_pmig.c |    5 +++--
 src/drmgr/drslot_chrp_cpu.c |   27 ++++++++++++++++-----------
 src/drmgr/drslot_chrp_mem.c |   11 ++++++++---
 src/lparstat.c              |   40 +++++++++++++++++++++++++++-------------
 src/nvram.c                 |    7 +++++--
 src/ppc64_cpu.c             |   20 +++++++++++---------
 src/rtas_ibm_get_vpd.c      |    3 +--
 src/serv_config.c           |   17 ++++++++---------
 12 files changed, 104 insertions(+), 60 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index aa2befb..9a00d92 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -93,6 +93,7 @@ src_rtas_ibm_get_vpd_SOURCES = src/rtas_ibm_get_vpd.c $(librtas_error_SOURCES) $
 src_rtas_ibm_get_vpd_LDADD = -lrtas
 
 src_serv_config_SOURCES = src/serv_config.c $(librtas_error_SOURCES) $(pseries_platform_SOURCES)
+src_serv_config_CFLAGS = $(AM_CFLAGS) -fno-strict-aliasing
 src_serv_config_LDADD = -lrtas
 
 src_uesensor_SOURCES = src/uesensor.c $(librtas_error_SOURCES) $(pseries_platform_SOURCES)
diff --git a/src/drmgr/common.c b/src/drmgr/common.c
index 0cb621d..0452168 100644
--- a/src/drmgr/common.c
+++ b/src/drmgr/common.c
@@ -53,7 +53,7 @@ int say(enum say_level lvl, char *fmt, ...)
 {
 	va_list ap;
 	char buf[256];
-	int len, rc;
+	int len;
 
 	va_start(ap, fmt);
 	memset(buf, 0, 256);
@@ -66,7 +66,7 @@ int say(enum say_level lvl, char *fmt, ...)
 	}
 
 	if (log_fd)
-		rc = write(log_fd, buf, len);
+		len = write(log_fd, buf, len);
 
 	if (lvl <= output_level)
 		fprintf(stderr, "%s", buf);
@@ -627,7 +627,7 @@ get_att_prop(const char *path, const char *name, char *buf, size_t buf_sz,
 	     const char *attr_type)
 {
 	FILE *fp;
-	int rc;
+	int rc = 0;
 	char dir[DR_PATH_MAX];
 
 	if (buf == NULL)
@@ -648,17 +648,23 @@ get_att_prop(const char *path, const char *name, char *buf, size_t buf_sz,
 	 * either /proc or sysfs so it works and is cheaper than a strcmp()
 	 */
 	switch (dir[1]) {
-	    case 'p':	/* /proc */
-		rc = fread(buf, buf_sz, 1, fp);
+	case 'p':	/* /proc */
+		if (fread(buf, buf_sz, 1, fp) != 1)
+			rc = -1;
 		break;
 
-	    case 's':	/* sysfs */
+	case 's':	/* sysfs */
 		rc = fscanf(fp, attr_type, (int *)buf);
+		if (rc == EOF)
+			rc = ferror(fp);
 		break;
+
+	default:
+		rc = -1;
 	}
 
 	fclose(fp);
-	return 0;
+	return rc;
 }
 
 /**
diff --git a/src/drmgr/common_cpu.c b/src/drmgr/common_cpu.c
index e4c8082..c36f669 100644
--- a/src/drmgr/common_cpu.c
+++ b/src/drmgr/common_cpu.c
@@ -194,8 +194,12 @@ cpu_index_to_path(struct dr_node *cpu)
 
 	closedir(d);
 
-	if (found)
+	if (found) {
 		snprintf(cpu->ofdt_path, DR_PATH_MAX, "%s", path);
+		rc = 0;
+	} else {
+		rc = -1;
+	}
 
 	return rc;
 }
diff --git a/src/drmgr/common_pci.c b/src/drmgr/common_pci.c
index 04a8a35..d646a94 100644
--- a/src/drmgr/common_pci.c
+++ b/src/drmgr/common_pci.c
@@ -1037,7 +1037,12 @@ get_bus_id(char *loc_code)
 		if (f == NULL)
 			continue;
 
-		fread(location, sizeof(location), 1, f);
+		if (fread(location, sizeof(location), 1, f) != 1) {
+			say(ERROR, "Could not read phys_location\n");
+			fclose(f);
+			return NULL;
+		}
+
 		fclose(f);
 
 		/* Strip any newline from the location to compare */
diff --git a/src/drmgr/drmig_chrp_pmig.c b/src/drmgr/drmig_chrp_pmig.c
index 8e8f9d0..68b20b8 100644
--- a/src/drmgr/drmig_chrp_pmig.c
+++ b/src/drmgr/drmig_chrp_pmig.c
@@ -15,6 +15,7 @@
 #include <sys/types.h>
 #include <dirent.h>
 #include <time.h>
+#include <inttypes.h>
 #include <librtas.h>
 #include "dr.h"
 #include "ofdt.h"
@@ -582,7 +583,7 @@ int do_migration(uint64_t stream_val)
 		rc = rtas_suspend_me(stream_val);
 		say(DEBUG, "ibm,suspend-me() returned %d\n", rc);
 	} else if (api_level == MIGRATION_API_V1) {
-		sprintf(buf, "0x%llx\n", stream_val);
+		sprintf(buf, "0x%" PRIx64 "\n", stream_val);
 
 		fd = open(SYSFS_MIGRATION_FILE, O_WRONLY);
 		if (fd == -1) {
@@ -616,7 +617,7 @@ int do_hibernation(uint64_t stream_val)
 	int rc, fd;
 	char buf[64];
 
-	sprintf(buf, "0x%llx\n", stream_val);
+	sprintf(buf, "0x%" PRIx64 "\n", stream_val);
 
 	fd = open(SYSFS_HIBERNATION_FILE, O_WRONLY);
 	if (fd == -1) {
diff --git a/src/drmgr/drslot_chrp_cpu.c b/src/drmgr/drslot_chrp_cpu.c
index 54fc576..f34854c 100644
--- a/src/drmgr/drslot_chrp_cpu.c
+++ b/src/drmgr/drslot_chrp_cpu.c
@@ -352,8 +352,8 @@ int
 drslot_chrp_cpu(struct options *opts)
 {
 	struct dr_info dr_info;
-	int rc = -1;
-
+	int rc;
+	
 	if (! cpu_dlpar_capable()) {
 		say(ERROR, "CPU DLPAR capability is not enabled on this "
 		    "platform.\n");
@@ -386,15 +386,20 @@ drslot_chrp_cpu(struct options *opts)
 
 	if (opts->p_option && (strcmp(opts->p_option, "smt_threads") == 0)) {
 		rc = smt_threads_func(opts, &dr_info);
-	} else {
-		switch (opts->action) {
-		    case ADD:
-			rc = add_cpus(opts, &dr_info);
-			break;
-		    case REMOVE:
-			rc = remove_cpus(opts, &dr_info);
-			break;
-		}
+		free_cpu_drc_info(&dr_info);
+		return rc;
+	} 
+	
+	switch (opts->action) {
+	case ADD:
+		rc = add_cpus(opts, &dr_info);
+		break;
+	case REMOVE:
+		rc = remove_cpus(opts, &dr_info);
+		break;
+	default:
+		rc = -1;
+		break;
 	}
 
 	free_cpu_drc_info(&dr_info);
diff --git a/src/drmgr/drslot_chrp_mem.c b/src/drmgr/drslot_chrp_mem.c
index ce58c42..dc4c50c 100644
--- a/src/drmgr/drslot_chrp_mem.c
+++ b/src/drmgr/drslot_chrp_mem.c
@@ -671,7 +671,7 @@ update_drconf_node(struct dr_node *lmb, struct lmb_list_head *lmb_list,
 	memcpy(tmp, lmb_list->drconf_buf, lmb_list->drconf_buf_sz);
 	tmp += lmb_list->drconf_buf_sz;
 
-	tmp += sprintf(tmp, " %s %d ", (action == ADD ? "add" : "remove"),
+	tmp += sprintf(tmp, " %s %ld ", (action == ADD ? "add" : "remove"),
 		       sizeof(lmb->lmb_address));
 	memcpy(tmp, &lmb->lmb_address, sizeof(lmb->lmb_address));
 	tmp += sizeof(lmb->lmb_address);
@@ -829,7 +829,6 @@ set_mem_scn_state(struct mem_scn *mem_scn, int state)
 	int file;
 	char path[DR_PATH_MAX];
 	int rc = 0;
-	int unused;
 	time_t t;
 	char tbuf[128];
 
@@ -848,9 +847,15 @@ set_mem_scn_state(struct mem_scn *mem_scn, int state)
 		return -1;
 	}
 
-	unused = write(file, state_strs[state], strlen(state_strs[state]));
+	rc = write(file, state_strs[state], strlen(state_strs[state]));
 	close(file);
 
+	if (rc == -1) {
+		say(ERROR, "Failed to mark %s %s\n", mem_scn->sysfs_path,
+		    state_strs[state]);
+		return rc;
+	}
+
 	if (get_mem_scn_state(mem_scn) != state) {
 		time(&t);
 		strftime(tbuf, 128, "%T", localtime(&t));
diff --git a/src/lparstat.c b/src/lparstat.c
index 8c4c71d..2100075 100644
--- a/src/lparstat.c
+++ b/src/lparstat.c
@@ -140,7 +140,7 @@ int parse_lparcfg()
 {
 	FILE *f;
 	char line[128];
-	char *unused;
+	int first_line = 1;
 
 	f = fopen(LPARCFG_FILE, "r");
 	if (!f) {
@@ -149,11 +149,15 @@ int parse_lparcfg()
 	}
 
 	/* parse the file skipping the first line */
-	unused = fgets(line, 128, f);
 	while (fgets(line, 128, f) != NULL) {
 		char *name, *value, *nl;
 		struct sysentry *se;
 
+		if (first_line) {
+			first_line = 0;
+			continue;
+		}
+
 		if (line[0] == '\n')
 			continue;
 
@@ -211,13 +215,16 @@ int parse_proc_stat()
 	int i, entries = 6;
 	long long statvals[entries];
 	struct sysentry *se;
-	char *unused;
 	char *names[] = {"cpu_total", "cpu_user", "cpu_nice", "cpu_sys",
 			 "cpu_idle", "cpu_iowait"};
 
 	/* we just need the first line */
 	f = fopen("/proc/stat", "r");
-	unused = fgets(line, 128, f);
+	if (fgets(line, 128, f) == NULL) {
+		fclose(f);
+		return -1;
+	}
+
 	fclose(f);
 
 	statvals[0] = 0;
@@ -312,17 +319,17 @@ void get_name(const char *file, char *buf)
 {
 	FILE *f;
 	char tmpbuf[64];
-	int rc;
 
 	f = fopen(file, "r");
-	if(!f) {
+	if (!f) {
 		sprintf(buf, "%c", '\0');
 		return;
 	}
-	rc = fread(tmpbuf, 64, 1, f);
-	fclose(f);
 
-	sprintf(buf, "%s", tmpbuf);
+	if (fread(tmpbuf, 64, 1, f) == 1)
+		sprintf(buf, "%s", tmpbuf);
+
+	fclose(f);
 }
 
 void get_node_name(struct sysentry *se, char *buf)
@@ -347,10 +354,14 @@ void get_mem_total(struct sysentry *se, char *buf)
 {
 	FILE *f;
 	char line[128];
-	char *mem, *nl, *unused;
+	char *mem, *nl;
 
 	f = fopen("/proc/meminfo", "r");
-	unused = fgets(line, 128, f);
+	if (fgets(line, 128, f) == NULL) {
+		fclose(f);
+		return;
+	}
+
 	fclose(f);
 
 	mem = strchr(line, ':');
@@ -369,10 +380,13 @@ void get_smt_mode(struct sysentry *se, char *buf)
 	FILE *f;
 	char line[128];
 	char *cmd = "/usr/sbin/ppc64_cpu --smt";
-	char *unused;
 
 	f = popen(cmd, "r");
-	unused = fgets(line, 128, f);
+	if (fgets(line, 128, f) == NULL) {
+		pclose(f);
+		return;
+	}
+
 	pclose(f);
 
 	/* The output is either "SMT=x" or "SMT is off", we can cheat
diff --git a/src/nvram.c b/src/nvram.c
index 4d968d0..09c9887 100644
--- a/src/nvram.c
+++ b/src/nvram.c
@@ -769,7 +769,7 @@ dump_errlog(struct nvram *nvram)
     uint16_t *sys_regs;	    /* System specific registers 
                              * (e.g. bus arbitration chips, etc */
     uint16_t *cpu_regs[MAX_CPUS+1];
-    uint16_t *memctrl_data;
+    /* uint16_t *memctrl_data; << See memctrl_data section below */
     uint16_t *ioctrl_data;
 
     phead = nvram_find_partition(nvram, NVRAM_SIG_SP, "ibm,err-log", NULL);
@@ -821,11 +821,14 @@ dump_errlog(struct nvram *nvram)
     num_memctrls = ntohs(p[i]);
     printf("Memory Controllers: %d\n", num_memctrls);
 
-    /* next index is offset of memory controller data */
+    /* next index is offset of memory controller data, this is not used
+       so just skip over it. */
     i++;	/* ToDo: this may be a list of offsets...manual doesn't show 
 		   that but only 1 seems odd */
+#if 0
     offset = ntohs(p[i])/2+1;
     memctrl_data = offset + i < p_max ? p + offset + i : 0;
+#endif
 
     /* next index is number of I/O Subsystem controllers */
     i++;
diff --git a/src/ppc64_cpu.c b/src/ppc64_cpu.c
index 42994ca..46aed33 100644
--- a/src/ppc64_cpu.c
+++ b/src/ppc64_cpu.c
@@ -694,7 +694,10 @@ static int do_smt_snooze_delay(char *state)
 
 static int do_run_mode(char *run_mode)
 {
-	char mode[3];
+	struct {
+		uint16_t	len;
+		uint8_t		val;
+	} rmode;
 	int rc;
 
 	if (getuid() != 0) {
@@ -704,7 +707,7 @@ static int do_run_mode(char *run_mode)
 	}
 
 	if (!run_mode) {
-		rc = rtas_get_sysparm(DIAGNOSTICS_RUN_MODE, 3, mode);
+		rc = rtas_get_sysparm(DIAGNOSTICS_RUN_MODE, 3, (char *)&rmode);
 		if (rc) {
 			if (rc == -3) {
 				printf("Machine does not support diagnostic "
@@ -720,19 +723,18 @@ static int do_run_mode(char *run_mode)
 				       "mode\n");
 			}
 		} else
-			printf("run-mode=%d\n", mode[2]);
+			printf("run-mode=%d\n", rmode.val);
 	} else {
-		short rmode = atoi(run_mode);
+		rmode.val = atoi(run_mode);
 
-		if (rmode < 0 || rmode > 3) {
-			printf("Invalid run-mode=%d\n", rmode);
+		if (rmode.val < 0 || rmode.val > 3) {
+			printf("Invalid run-mode=%d\n", rmode.val);
 			return -1;
 		}
 
-		*(short *)mode = htobe16(1);
-		mode[2] = rmode;
+		rmode.len = 1;
 
-		rc = rtas_set_sysparm(DIAGNOSTICS_RUN_MODE, mode);
+		rc = rtas_set_sysparm(DIAGNOSTICS_RUN_MODE, (char *)&rmode);
 		if (rc) {
 			if (rc == -3) {
 				printf("Machine does not support diagnostic "
diff --git a/src/rtas_ibm_get_vpd.c b/src/rtas_ibm_get_vpd.c
index b2d94ce..c2ffd9a 100644
--- a/src/rtas_ibm_get_vpd.c
+++ b/src/rtas_ibm_get_vpd.c
@@ -111,7 +111,7 @@ int main(int argc, char **argv)
 {
 	char *loc_code = "";
 	char err_buf[ERR_BUF_SIZE];
-	int lflag = 0, rc, c;
+	int rc, c;
 	unsigned int seq = 1, next_seq;
 	struct buf_element *list, *current;
 
@@ -133,7 +133,6 @@ int main(int argc, char **argv)
 		switch (c) {
 		case 'l':
 			loc_code = optarg;
-			lflag = 1;
 			break;
 		case 'h':
 			print_help(argv[0]);
diff --git a/src/serv_config.c b/src/serv_config.c
index 46e02f3..900f0f3 100644
--- a/src/serv_config.c
+++ b/src/serv_config.c
@@ -463,7 +463,7 @@ int
 update_nvram(char *var, char *val, char *partition) {
 	char buf[256];
 	pid_t child;
-	int status, rc;
+	int status;
 	char *nvram_args[] = { "nvram", "--update-config",
 			buf, "-p", partition, NULL };
 
@@ -482,12 +482,12 @@ update_nvram(char *var, char *val, char *partition) {
 	}
 	else if (child == 0) {
 		/* child process */
-		rc = execv(NVRAM_PROGRAM, nvram_args);
-
-		/* shouldn't get here */
-		err_msg(ERR_MSG, "Could not exec %s to update NVRAM\n",
+		if (execv(NVRAM_PROGRAM, nvram_args)) {
+			/* shouldn't get here */
+			err_msg(ERR_MSG, "Could not exec %s to update NVRAM\n",
 				NVRAM_PROGRAM);
-		exit(1);
+			exit(1);
+		}
 	}
 	else {
 		/* parent process */
@@ -609,12 +609,11 @@ byte_to_string(uint8_t num, char *buf, size_t size) {
  */
 int
 parse_call_home_buffer(char *var, char *buf, size_t size) {
-	int buf_size;
 	char *loc;
 
 	if (!call_home_buffer) return RC_OTHER;	/* should never happen */
 
-	buf_size = be16toh(*(uint16_t *)call_home_buffer);
+	/* The first 16 bits is the call home bufer size, skip past this. */
 	loc = call_home_buffer + sizeof(uint16_t);
 
 	while (loc[0] != '\0') {
@@ -674,7 +673,7 @@ retrieve_value(struct service_var *var, char *buf, size_t size) {
 			if (var->sysparm_num == USE_CALL_HOME_SYSPARM)
 				break;
 
-			ret_size = be16toh(*(uint16_t *)param);
+			ret_size = be16toh(*param);
 			if (!strcmp(var->nvram_var, "sp-ri-pon") ||
 				!strcmp(var->nvram_var, "sp-remote-pon") ||
 				!strcmp(var->nvram_var, "sp-sen")) {

[Powerpc-utils-devel] [RFC PATCH v2] snap: Warn users about deprecated support for SLES 12 onwards

[Kamalesh B. <kam...@li...>]

This patch introduce deprecated warning on SLES 12 onwards
to use supportconfig. supportconfig captures all the information
collected by snap, so it safe to deprecate snap.

It also rearranges check for distribution (RHEL/SLES/Ubuntu) into
common function check_distro_support().

Signed-off-by: Kamalesh Babulal <kam...@li...>
Cc: Vasant Hegde <heg...@li...>
Cc: Nathan Fontenot <nf...@li...>
---
Changes from v1:
 - Moved the check for ubuntu into check_distro_support().

 scripts/snap | 61 +++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 42 insertions(+), 19 deletions(-)

diff --git a/scripts/snap b/scripts/snap
index f7bc72a..613f504 100755
--- a/scripts/snap
+++ b/scripts/snap
@@ -34,18 +34,45 @@ my $outfile = "snap.tar.gz";		# in the working dir.
 my $cmddir = "snap_commands";		# cmd output dir.
 my $cmdoutdir = "$outdir/$cmddir";	# in outdir dir.
 my $rsxx_exists = 0;			# Does an IBM Flash Adapter exist?
-my $distro_file = "/etc/issue";
-my $redhat_release_file = "/etc/redhat-release";
-
-if (-e $redhat_release_file) {
-	open(RELEASE, "< $redhat_release_file") or die "open: $!\n";
-	$_ = <RELEASE>;
-	my $redhat_version = (split / /, $_)[6];
-	if ($redhat_version >= 7.0) {
-		print "snap is not supported on the RHEL 7 onwards..!\n";
-		print "Please use sosreport to collect log data..!! \n";
-		exit 1;
-	}
+
+sub check_distro_support {
+	my $redhat_release_file = "/etc/redhat-release";
+	my $suse_release_file = "/etc/SuSE-release";
+	my $distro_file = "/etc/issue";
+
+	if (-e $redhat_release_file) {
+		open(RELEASE, "< $redhat_release_file") or die "open: $!\n";
+		$_ = <RELEASE>;
+		my $redhat_version = (split / /, $_)[6];
+		if ($redhat_version >= 7.0) {
+			print "snap is not supported on the RHEL 7 onwards..!\n";
+			print "Please use sosreport to collect log data..!! \n";
+			close(RELEASE);
+			exit 1;
+		}
+		close(RELEASE);
+	} elsif (-e $suse_release_file) {
+		open(RELEASE, "< $suse_release_file") or die "open: $!\n";
+		while(<RELEASE>) {
+			if ($_ =~ /VERSION/) {
+				my $suse_version = (split /=/, $_)[1];
+				if ($suse_version >= 12) {
+					print "snap is deprecated from SLES 12 onwards..!\n";
+					print "Please use supportconfig to collect log data..!! \n";
+					close(RELEASE);
+					exit 1;
+				}
+			} # if
+		} # while
+		close(RELEASE);
+	} else {
+		open(RELEASE, "< $distro_file") or die "open: $!\n";
+		if (<RELEASE> =~ /Ubuntu/) {
+			print "snap: is not supported on the Ubuntu platform\n";
+			close(RELEASE);
+			exit 1;
+		} #if
+	} #else
 }
 
 our($opt_a, $opt_d, $opt_h, $opt_o, $opt_t, $opt_v);
@@ -333,6 +360,9 @@ sub snap_commands {
 
 $< == 0 or error(1, "Must be executed as root");
 
+#check for the distro version
+check_distro_support();
+
 my $perldumpenv='perl -MData::Dumper -e '."'".
     '\$Data::Dumper::Terse=1;print Dumper(\%ENV);'."'";
 
@@ -341,13 +371,6 @@ eval '%ENV=('.$1.')' if `bash -c "
         $perldumpenv"`
     =~ /^\s*\{(.*)\}\s*$/mxs;
 
-open(my $input, "<", $distro_file);
-
-if (<$input> =~ /Ubuntu/) {
-	print "snap: is not supported on the Ubuntu platform\n";
-	exit 1;
-}
-
 if ($ENV{'platform'} == $ENV{'PLATFORM_UNKNOWN'} || $ENV{'platform'} == $ENV{'PLATFORM_POWERNV_HOST'}) {
         print "snap: is not supported on the $ENV{'platform_name'} platform\n";
         exit 1;
-- 
2.1.2

[Powerpc-utils-devel] [RFC PATCH] snap: Warn users about deprecated support for SLES 12 onwards

[Kamalesh B. <kam...@li...>]

This patch introduce deprecated warning on SLES 12 onwards
to use supportconfig. supportconfig captures all the information
collected by snap, so it safe to deprecate snap.

It also rearranges check for distribution (RHEL/SLES) into
common function check_distro_support().

Signed-off-by: Kamalesh Babulal <kam...@li...>
Cc: Vasant Hegde <heg...@li...>
Cc: Nathan Fontenot <nf...@li...>
---
 scripts/snap | 43 +++++++++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 10 deletions(-)

diff --git a/scripts/snap b/scripts/snap
index f7bc72a..879316c 100755
--- a/scripts/snap
+++ b/scripts/snap
@@ -35,16 +35,36 @@ my $cmddir = "snap_commands";		# cmd output dir.
 my $cmdoutdir = "$outdir/$cmddir";	# in outdir dir.
 my $rsxx_exists = 0;			# Does an IBM Flash Adapter exist?
 my $distro_file = "/etc/issue";
-my $redhat_release_file = "/etc/redhat-release";
-
-if (-e $redhat_release_file) {
-	open(RELEASE, "< $redhat_release_file") or die "open: $!\n";
-	$_ = <RELEASE>;
-	my $redhat_version = (split / /, $_)[6];
-	if ($redhat_version >= 7.0) {
-		print "snap is not supported on the RHEL 7 onwards..!\n";
-		print "Please use sosreport to collect log data..!! \n";
-		exit 1;
+
+sub check_distro_support {
+	my $redhat_release_file = "/etc/redhat-release";
+	my $suse_release_file = "/etc/SuSE-release";
+
+	if (-e $redhat_release_file) {
+		open(RELEASE, "< $redhat_release_file") or die "open: $!\n";
+		$_ = <RELEASE>;
+		my $redhat_version = (split / /, $_)[6];
+		if ($redhat_version >= 7.0) {
+			print "snap is not supported on the RHEL 7 onwards..!\n";
+			print "Please use sosreport to collect log data..!! \n";
+			close(RELEASE);
+			exit 1;
+		}
+		close(RELEASE);
+	} elsif (-e $suse_release_file) {
+		open(RELEASE, "< $suse_release_file") or die "open: $!\n";
+		while(<RELEASE>) {
+			if ($_ =~ /VERSION/) {
+				my $suse_version = (split /=/, $_)[1];
+				if ($suse_version >= 12) {
+					print "snap is deprecated from SLES 12 onwards..!\n";
+					print "Please use supportconfig to collect log data..!! \n";
+					close(RELEASE);
+					exit 1;
+				}
+			} # if
+		} # while
+		close(RELEASE);
 	}
 }
 
@@ -333,6 +353,9 @@ sub snap_commands {
 
 $< == 0 or error(1, "Must be executed as root");
 
+#check for the distro version
+check_distro_support();
+
 my $perldumpenv='perl -MData::Dumper -e '."'".
     '\$Data::Dumper::Terse=1;print Dumper(\%ENV);'."'";
 
-- 
2.1.2

[Powerpc-utils-devel] [PATCH] lsslot: Fix missing header for __be32 types

[Joel S. <jo...@jm...>]

On my system (Ubuntu 15.04) lsslot does not build as it's missing the
__be32 types.

 src/drmgr/lsslot.c: In function ‘print_drconf_mem’:
 src/drmgr/lsslot.c:712:2: error: unknown type name ‘__be32’
   __be32 *aa;

Signed-off-by: Joel Stanley <jo...@jm...>
---

I assumed this tool was Linux only. If not, we will need to find a different
header for the types.

 src/drmgr/lsslot.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/drmgr/lsslot.c b/src/drmgr/lsslot.c
index 0537dde..bbb3abd 100644
--- a/src/drmgr/lsslot.c
+++ b/src/drmgr/lsslot.c
@@ -9,6 +9,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <locale.h>
+#include <linux/types.h>
 #include "rtas_calls.h"
 #include "drpci.h"
 #include "dr.h"
-- 
2.1.4

[Powerpc-utils-devel] [PATCH v2 2/2] nvram: don't allow long names

[Joel S. <jo...@jm...>]

The maximum partition name length is 12 characters but nvram accepts
strings of any length. This change will enforce the maximum length
passed to options that take a partition name to avoid confusion.

Signed-off-by: Joel Stanley <jo...@jm...>
---
v2: put check in a function as suggested by Nathan

 src/nvram.c | 24 +++++++++++++++++++++++-
 src/nvram.h | 14 ++++++++++----
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/src/nvram.c b/src/nvram.c
index c3f4ebc..4d968d0 100644
--- a/src/nvram.c
+++ b/src/nvram.c
@@ -45,6 +45,7 @@
 #include <inttypes.h>
 #include <zlib.h>
 #include <endian.h>
+#include <stdbool.h>
 
 #include "nvram.h"
 
@@ -452,6 +453,16 @@ nvram_parse_partitions(struct nvram *nvram)
     return 0;
 }
 
+bool part_name_valid(const char *name)
+{
+    if (strlen(name) > MAX_PART_NAME) {
+        err_msg("partition name maximum length is %d\n", MAX_PART_NAME);
+        return false;
+    }
+
+    return true;
+}
+
 /**
  * nvram_find_fd_partition
  * @brief Find a particular nvram partition using a file descriptor
@@ -467,6 +478,9 @@ nvram_find_fd_partition(struct nvram *nvram, char *name)
     int				len;
     int				found = 0;
 
+    if (part_name_valid(name))
+	    return -1;
+
     if (lseek(nvram->fd, SEEK_SET, 0) == -1) {
         err_msg("could not seek to beginning of file %s\n", nvram->filename);
 	return -1;
@@ -1461,12 +1475,18 @@ main (int argc, char *argv[])
 		break;
 	    case 'd':	/* dump */
 		dump_name = optarg;
+                if (!part_name_valid(dump_name))
+                    exit(1);
 		break;
 	    case 'a':	/* ASCII dump */
 	    	ascii_name = optarg;
+                if (!part_name_valid(ascii_name))
+                    exit(1);
 		break;
 	    case 'z':	/* dump compressed data */
-		zip_name = optarg;
+                zip_name = optarg;
+                if (!part_name_valid(zip_name))
+                    exit(1);
 		break;
 	    case 'n':	/* nvram-file */
 		nvram.filename = optarg;
@@ -1509,6 +1529,8 @@ main (int argc, char *argv[])
 		break;
 	    case 'p':	/* update-config partition name */
 	        config_pname = optarg;
+                if (!part_name_valid(config_pname))
+                    exit(1);
 		break;
 	    case '?':
 		exit(1);
diff --git a/src/nvram.h b/src/nvram.h
index b4961fe..b78e793 100644
--- a/src/nvram.h
+++ b/src/nvram.h
@@ -45,14 +45,20 @@
 #define MAX_CPUS 128
 
 /**
+ * @def MAX_PART_NAME
+ * @brief maximum number of bytes in partition name
+ */
+#define MAX_PART_NAME 12
+
+/**
  * @struct partition_header
  * @brief nvram partition header data
  */
 struct partition_header {
-    unsigned char	signature;  /**< partition signature */
-    unsigned char 	checksum;   /**< partition checksum */
-    unsigned short 	length;     /**< partition length */
-    char 		name[12];   /**< partition name */
+    unsigned char	signature;		/**< partition signature */
+    unsigned char 	checksum;		/**< partition checksum */
+    unsigned short 	length;			/**< partition length */
+    char 		name[MAX_PART_NAME];	/**< partition name */
 };
 
 /* sub-header for error-log partitions */
-- 
2.1.4

[Powerpc-utils-devel] [PATCH v2 1/2] nvram: fix stack overflow in logging function

[Joel S. <jo...@jm...>]

Signed-off-by: Joel Stanley <jo...@jm...>
---
 src/nvram.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/nvram.c b/src/nvram.c
index ce3f7e1..c3f4ebc 100644
--- a/src/nvram.c
+++ b/src/nvram.c
@@ -136,7 +136,7 @@ help(void)
  * @def MAXLINE
  * @brief maximum line length
  */
-#define MAXLINE		4096
+#define MAXLINE		512
 
 /**
  * _msg
@@ -152,10 +152,10 @@ _msg(int msg_type, const char *fmt, va_list ap)
     int	n;
     char buf[MAXLINE];
 
-    n = sprintf(buf, "%s: %s", nvram_cmdname, 
+    n = snprintf(buf, sizeof(buf), "%s: %s", nvram_cmdname,
 		(msg_type == WARN_MSG ? "WARNING: " : "ERROR: "));
 
-    vsprintf(buf + n, fmt, ap);
+    vsnprintf(buf + n, sizeof(buf) - n, fmt, ap);
 
     fflush(stderr);
     fputs(buf, stderr);
-- 
2.1.4

Re: [Powerpc-utils-devel] [PATCH 2/2] nvram: don't allow long names

[Joel S. <jo...@jm...>]

On Thu, Jun 18, 2015 at 5:54 AM, Nathan Fontenot
<nf...@li...> wrote:
>>
>> +    if (strlen(name) > MAX_PART_NAME) {
>> +        err_msg("partition name too long\n");
>> +        return -1;
>> +    }
>> +
>
> Any reason for not making this a function for here (and below) instead of
> copying the code in every place?

Not really. I will send a v2 tomorrow.

Thanks for the review.

Cheers,

Joel

Re: [Powerpc-utils-devel] [PATCH] ppc64_cpu: Use calloc for array allocation

[Vasant H. <heg...@li...>]

On 06/23/2015 07:05 PM, Nathan Fontenot wrote:
> Use calloc instead of malloc + memset for array allocation.
> 
> Signed-off-by: Nathan Fontenot <nf...@li...>

Looks good.

Reviewed-by: Vasant Hegde <heg...@li...>

-Vasant

[Powerpc-utils-devel] [PATCH] ppc64_cpu: Use calloc for array allocation

[Nathan F. <nf...@li...>]

Use calloc instead of malloc + memset for array allocation.

Signed-off-by: Nathan Fontenot <nf...@li...>
---
 src/ppc64_cpu.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/ppc64_cpu.c b/src/ppc64_cpu.c
index 6ecfe7f..42994ca 100644
--- a/src/ppc64_cpu.c
+++ b/src/ppc64_cpu.c
@@ -980,8 +980,9 @@ static int do_cpu_frequency(int sleep_time)
 
 	setrlimit_open_files();
 
-	cpu_freqs = malloc(sizeof(*cpu_freqs) * threads_in_system);
-	memset(cpu_freqs, 0, sizeof(*cpu_freqs) * threads_in_system);
+	cpu_freqs = calloc(threads_in_system, sizeof(*cpu_freqs));
+	if (!cpu_freqs)
+		return -ENOMEM;
 
 	rc = setup_counters(cpu_freqs);
 	if (rc) {
@@ -1123,8 +1124,10 @@ static int do_cores_online(char *state)
 		return -1;
 	}
 
-	core_state = malloc(sizeof(int) * cpus_in_system);
-	memset(core_state, 0, sizeof(int) * cpus_in_system);
+	core_state = calloc(cpus_in_system, sizeof(int));
+	if (!core_state)
+		return -ENOMEM;
+
 	for (i = 0; i < cpus_in_system ; i++) {
 		core_state[i] = cpu_online(i * threads_per_cpu);
 		if (core_state[i])

[Powerpc-utils-devel] [PATCH] ppc64_cpu: Correct pthread handling for exiting threads

[Nathan F. <nf...@li...>]

The pthread handling for checking cpu frequency in the ppc64_cpu
command does not properly handle 'soak' threads exiting from a failed
sched_setaffinity call.

We need to update the number of threads handled to be dynamically set
instead of defining MAX_NR_CPUS, which is out of date. We also need to
check for 'soak' threads that have exited before reading the perf counters

Signed-off-by: Nathan Fontenot <nf...@li...>
---
 src/ppc64_cpu.c |  115 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 73 insertions(+), 42 deletions(-)

diff --git a/src/ppc64_cpu.c b/src/ppc64_cpu.c
index 376c20f..6ecfe7f 100644
--- a/src/ppc64_cpu.c
+++ b/src/ppc64_cpu.c
@@ -50,8 +50,12 @@
 #define CPU_OFFLINE		-1
 
 #ifdef HAVE_LINUX_PERF_EVENT_H
-static unsigned long long cpu_freq[MAX_NR_CPUS];
-static int counters[MAX_NR_CPUS];
+struct cpu_freq {
+	int offline;
+	int counter;
+	pthread_t tid;
+	unsigned long long freq;
+};
 
 #ifndef __NR_perf_event_open
 #define __NR_perf_event_open	319
@@ -762,7 +766,7 @@ static int do_run_mode(char *run_mode)
 
 #ifdef HAVE_LINUX_PERF_EVENT_H
 
-static int setup_counters(void)
+static int setup_counters(struct cpu_freq *cpu_freqs)
 {
 	int i;
 	struct perf_event_attr attr;
@@ -774,13 +778,15 @@ static int setup_counters(void)
 	attr.size = sizeof(attr);
 
 	for (i = 0; i < threads_in_system; i++) {
-		if (!cpu_online(i))
+		if (!cpu_online(i)) {
+			cpu_freqs[i].offline = 1;
 			continue;
+		}
 
-		counters[i] = syscall(__NR_perf_event_open, &attr, -1,
-				      i, -1, 0);
+		cpu_freqs[i].counter = syscall(__NR_perf_event_open, &attr,
+					       -1, i, -1, 0);
 
-		if (counters[i] < 0) {
+		if (cpu_freqs[i].counter < 0) {
 			if (errno == ENOSYS)
 				fprintf(stderr, "frequency determination "
 					"not supported with this kernel.\n");
@@ -794,45 +800,68 @@ static int setup_counters(void)
 	return 0;
 }
 
-static void start_counters(void)
+static void start_counters(struct cpu_freq *cpu_freqs)
 {
 	int i;
 
 	for (i = 0; i < threads_in_system; i++) {
-		if (cpu_freq[i] == CPU_OFFLINE)
+		if (cpu_freqs[i].offline)
 			continue;
 
-		ioctl(counters[i], PERF_EVENT_IOC_ENABLE);
+		ioctl(cpu_freqs[i].counter, PERF_EVENT_IOC_ENABLE);
 	}
 }
 
-static void stop_counters(void)
+static void stop_counters(struct cpu_freq *cpu_freqs)
 {
 	int i;
 
 	for (i = 0; i < threads_in_system; i++) {
-		if (cpu_freq[i] == CPU_OFFLINE)
+		if (cpu_freqs[i].offline)
 			continue;
 
-		ioctl(counters[i], PERF_EVENT_IOC_DISABLE);
+		ioctl(cpu_freqs[i].counter, PERF_EVENT_IOC_DISABLE);
 	}
 }
 
-static void read_counters(void)
+static void read_counters(struct cpu_freq *cpu_freqs)
 {
 	int i;
 
 	for (i = 0; i < threads_in_system; i++) {
 		size_t res;
 
-		if (cpu_freq[i] == CPU_OFFLINE)
+		if (cpu_freqs[i].offline)
 			continue;
 
-		res = read(counters[i], &cpu_freq[i],
+		res = read(cpu_freqs[i].counter, &cpu_freqs[i].freq,
 			   sizeof(unsigned long long));
 		assert(res == sizeof(unsigned long long));
 
-		close(counters[i]);
+		close(cpu_freqs[i].counter);
+	}
+}
+
+static void check_threads(struct cpu_freq *cpu_freqs)
+{
+	int i;
+
+	for (i = 0; i < threads_in_system; i++) {
+		if (cpu_freqs[i].offline)
+			continue;
+
+		/* Sending signal 0 with pthread_kill will just check for
+		 * the existance of the thread without actually sending a
+		 * signal, we use this to see if the thread exited.
+		 */
+		if (pthread_kill(cpu_freqs[i].tid, 0)) {
+			/* pthread exited, mark it offline iso we don't use
+			 * it in our calculations and close its perf
+			 * counter.
+			 */
+			cpu_freqs[i].offline = 1;
+			close(cpu_freqs[i].counter);
+		}
 	}
 }
 
@@ -846,7 +875,7 @@ static void *soak(void *arg)
 
 	if (sched_setaffinity(0, sizeof(cpumask), &cpumask)) {
 		perror("sched_setaffinity");
-		exit(1);
+		pthread_exit(NULL);
 	}
 
 	while (1)
@@ -947,27 +976,28 @@ static int do_cpu_frequency(int sleep_time)
 	unsigned long max_cpu = -1UL;
 	unsigned long long sum = 0;
 	unsigned long count = 0;
+	struct cpu_freq *cpu_freqs;
 
 	setrlimit_open_files();
 
-	memset(cpu_freq, 0, sizeof(cpu_freq));
-	memset(counters, 0, sizeof(counters));
+	cpu_freqs = malloc(sizeof(*cpu_freqs) * threads_in_system);
+	memset(cpu_freqs, 0, sizeof(*cpu_freqs) * threads_in_system);
 
-	rc = setup_counters();
-	if (rc)
+	rc = setup_counters(cpu_freqs);
+	if (rc) {
+		free(cpu_freqs);
 		return rc;
+	}
 
 	/* Start a soak thread on each CPU */
 	for (i = 0; i < threads_in_system; i++) {
-		pthread_t tid;
-
-		if (!cpu_online(i)) {
-			cpu_freq[i] = CPU_OFFLINE;
+		if (cpu_freqs[i].offline)
 			continue;
-		}
 
-		if (pthread_create(&tid, NULL, soak, (void *)(long)i)) {
+		if (pthread_create(&cpu_freqs[i].tid, NULL, soak,
+				   (void *)(long)i)) {
 			perror("pthread_create");
+			free(cpu_freqs);
 			return -1;
 		}
 	}
@@ -975,32 +1005,31 @@ static int do_cpu_frequency(int sleep_time)
 	/* Wait for soak threads to start */
 	usleep(1000000);
 
-	start_counters();
+	start_counters(cpu_freqs);
 	/* Count for specified timeout in seconds */
 	usleep(sleep_time * 1000000);
-	stop_counters();
 
-	read_counters();
+	stop_counters(cpu_freqs);
+	check_threads(cpu_freqs);
+	read_counters(cpu_freqs);
 
 	for (i = 0; i < threads_in_system; i++) {
-		if (cpu_freq[i] == CPU_OFFLINE)
-			continue;
+		unsigned long long frequency;
 
-		/* No result - Couldn't schedule on that cpu */
-		if (cpu_freq[i] == 0) {
-			printf("WARNING: couldn't run on cpu %d\n", i);
+		if (cpu_freqs[i].offline)
 			continue;
-		}
 
-		if (cpu_freq[i] < min) {
-			min = cpu_freq[i];
+		frequency = cpu_freqs[i].freq;
+
+		if (frequency < min) {
+			min = frequency;
 			min_cpu = i;
 		}
-		if (cpu_freq[i] > max) {
-			max = cpu_freq[i];
+		if (frequency > max) {
+			max = frequency;
 			max_cpu = i;
 		}
-		sum += cpu_freq[i];
+		sum += frequency;
 		count++;
 	}
 
@@ -1010,6 +1039,8 @@ static int do_cpu_frequency(int sleep_time)
 	printf("max:\t%.3f GHz (cpu %ld)\n", freq_calc(max, sleep_time),
 	       max_cpu);
 	printf("avg:\t%.3f GHz\n\n", freq_calc((sum / count), sleep_time));
+
+	free(cpu_freqs);
 	return 0;
 }
 

Re: [Powerpc-utils-devel] [PATCH 2/2] nvram: don't allow long names

[Nathan F. <nf...@li...>]

On 06/09/2015 10:43 PM, Joel Stanley wrote:
> The maximum partition name length is 12 characters but nvram accepts
> strings of any length. This change will enforce the maximum length
> passed to options that take a partition name to avoid confusion.
> 
> Signed-off-by: Joel Stanley <jo...@jm...>
> ---
>  src/nvram.c | 25 +++++++++++++++++++++++++
>  src/nvram.h | 14 ++++++++++----
>  2 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/src/nvram.c b/src/nvram.c
> index c3f4ebc..b2f5c8c 100644
> --- a/src/nvram.c
> +++ b/src/nvram.c
> @@ -467,6 +467,11 @@ nvram_find_fd_partition(struct nvram *nvram, char *name)
>      int				len;
>      int				found = 0;
> 
> +    if (strlen(name) > MAX_PART_NAME) {
> +        err_msg("partition name too long\n");
> +        return -1;
> +    }
> +

Any reason for not making this a function for here (and below) instead of
copying the code in every place?

-Nathan

>      if (lseek(nvram->fd, SEEK_SET, 0) == -1) {
>          err_msg("could not seek to beginning of file %s\n", nvram->filename);
>  	return -1;
> @@ -1461,12 +1466,27 @@ main (int argc, char *argv[])
>  		break;
>  	    case 'd':	/* dump */
>  		dump_name = optarg;
> +		if (strlen(dump_name) > MAX_PART_NAME) {
> +			err_msg("partition name maximum length is %d\n",
> +					MAX_PART_NAME);
> +			exit(1);
> +		}
>  		break;
>  	    case 'a':	/* ASCII dump */
>  	    	ascii_name = optarg;
> +		if (strlen(ascii_name) > MAX_PART_NAME) {
> +			err_msg("partition name maximum length is %d\n",
> +					MAX_PART_NAME);
> +			exit(1);
> +		}
>  		break;
>  	    case 'z':	/* dump compressed data */
>  		zip_name = optarg;
> +		if (strlen(zip_name) > MAX_PART_NAME) {
> +			err_msg("partition name maximum length is %d\n",
> +					MAX_PART_NAME);
> +			exit(1);
> +		}
>  		break;
>  	    case 'n':	/* nvram-file */
>  		nvram.filename = optarg;
> @@ -1509,6 +1529,11 @@ main (int argc, char *argv[])
>  		break;
>  	    case 'p':	/* update-config partition name */
>  	        config_pname = optarg;
> +		if (strlen(config_pname) > MAX_PART_NAME) {
> +			err_msg("partition name maximum length is %d\n",
> +					MAX_PART_NAME);
> +			exit(1);
> +		}
>  		break;
>  	    case '?':
>  		exit(1);
> diff --git a/src/nvram.h b/src/nvram.h
> index b4961fe..b78e793 100644
> --- a/src/nvram.h
> +++ b/src/nvram.h
> @@ -45,14 +45,20 @@
>  #define MAX_CPUS 128
> 
>  /**
> + * @def MAX_PART_NAME
> + * @brief maximum number of bytes in partition name
> + */
> +#define MAX_PART_NAME 12
> +
> +/**
>   * @struct partition_header
>   * @brief nvram partition header data
>   */
>  struct partition_header {
> -    unsigned char	signature;  /**< partition signature */
> -    unsigned char 	checksum;   /**< partition checksum */
> -    unsigned short 	length;     /**< partition length */
> -    char 		name[12];   /**< partition name */
> +    unsigned char	signature;		/**< partition signature */
> +    unsigned char 	checksum;		/**< partition checksum */
> +    unsigned short 	length;			/**< partition length */
> +    char 		name[MAX_PART_NAME];	/**< partition name */
>  };
> 
>  /* sub-header for error-log partitions */
>