Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shellcheck improvements for genesis-scripts #4923

Closed
wants to merge 2 commits into from

Conversation

samveen
Copy link
Member

@samveen samveen commented Mar 12, 2018

This PR is a partial fix for issue #4610 (the low hanging fruit so to say).

@xcatbot
Copy link

xcatbot commented Mar 12, 2018

CI CHECK RESULT : > PR FORMAT CORRECT> BUILD SUCCESSFUL > INSTALL XCAT SUCCESSFUL> CODE SYNTAX CORRECT> FAST REGRESSION TEST Successful: Totalcase 215 Pass 215 failed 0

@zet809 zet809 requested a review from neo954 March 12, 2018 10:00
@zet809 zet809 added this to the 2.14 milestone Mar 12, 2018
@zet809
Copy link

zet809 commented Mar 12, 2018

Hi, @neo954 , will you pls help to review this PR? Thx!

@zet809 zet809 closed this Mar 12, 2018
@zet809 zet809 reopened this Mar 12, 2018
@samveen samveen force-pushed the gen_shellcheck branch 2 times, most recently from 9e6b9de to bf7ef8c Compare March 12, 2018 12:59
fi
if [ -z "$MTM" -o "$MTM" == ":" ]; then
logger -s -t $log_label -p local4.warning "Couldn't find MTM information in FRU, falling back to DMI (MTMS-based discovery may fail)"
m=`cat /sys/devices/virtual/dmi/id/sys_vendor`
n=`cat /sys/devices/virtual/dmi/id/product_name`
read m </sys/devices/virtual/dmi/id/sys_vendor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest use read -r m instead of read m here.

m=`cat /sys/devices/virtual/dmi/id/sys_vendor`
n=`cat /sys/devices/virtual/dmi/id/product_name`
read m </sys/devices/virtual/dmi/id/sys_vendor
read n </sys/devices/virtual/dmi/id/product_name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest use read -r n instead of read n here.

echo $line > /tmp/cpumod
done
CPUTYPE=`cat /tmp/cpumod|awk -F':' '{print $2}'|sed -e 's/^ //'`
read CPUCOUNT CPUTYPE < <(awk -F: 'BEGIN{c=0; n=""} /model name/{c=c+1; n=$2} END{printf "%d %s",c,n}' /proc/cpuinfo)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as above. I suggest use read -r instead of read.

done
CPUTYPE=`cat /tmp/cpumod|awk -F':' '{print $2}'|sed -e 's/^ //'`
PLATFORM=$(awk '/^platform\s*:/{print \$3}' /proc/cpuinfo)
read CPUCOUNT CPUTYPE < <(awk -F':' 'BEGIN{c=0; n=""} /^cpu\s*:/{c=c+1; n=$2} END{printf "%d %s",c,n}' /proc/cpuinfo)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not going to work. Please see my test results below.

On ppc64le RHEL 7.4

# awk -F':' 'BEGIN{c=0; n=""} /^cpu\s*:/{c=c+1; n=$2} END{printf "%d %s",c,n}' /proc/cpuinfo
8  POWER8 (architected), altivec supported

On ppc64 RHEL 7.4

# awk -F':' 'BEGIN{c=0; n=""} /^cpu\s*:/{c=c+1; n=$2} END{printf "%d %s",c,n}' /proc/cpuinfo
16  POWER7 (architected), altivec supported

On x86-64 RHEL 7.4

# awk -F':' 'BEGIN{c=0; n=""} /^cpu\s*:/{c=c+1; n=$2} END{printf "%d %s",c,n}' /proc/cpuinfo
0
  • It does not work on x86-64
  • On ppc64le or ppc64, CPUTYPE will be assigned to something like POWER8 (architected), altivec supported. But for the original code, it will be assigned to POWER8.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neo954 Can you give me the output of cat /proc/cpuinfo for ppc64 and ppc64le machines? I have no access to a sample for them.

echo $line > /tmp/cpumod
done
CPUTYPE=`cat /tmp/cpumod|awk -F':' '{print $2}'|sed -e 's/^ //'`
PLATFORM=$(awk '/^platform\s*:/{print \$3}' /proc/cpuinfo)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you are using single quote, no need to add backslash for '$'. And double quotes are needed outside the parentheses.

PLATFORM="$(awk '/^platform\s*:/{print $3}' /proc/cpuinfo)"

<pcilocation>$pci</pcilocation>
<model>${tmp##*: }</model>
</nic>
__ENDL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a multiple-line echo or use curly brackets is better.

if grep -q 'BEGIN CERTIFICATE' /tmp/certresp.xml ; then
awk '/BEGIN CERTIFICATE/,/END CERTIFICATE/' < /tmp/certresp.xml > /etc/xcat/cert.pem
#stop transmitting sysDesc, allowing the public key to age out of validity
while read iface ; do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest use read -r instead of read.

done
rm /tmp/destreq.xml
DESTINY=`grep '<destiny>' /tmp/destiny.xml | awk -F'>' '{print $2}'|awk -F'<' '{print $1}'`
DESTINY="$(awk -F'>' '/<destiny>/{print $2}' /tmp/destiny.xml|awk -F'<' '{print $1}')"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there only one line in file /tmp/destiny.xml? Otherwise this line will not work.

Copy link
Member Author

@samveen samveen Mar 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, I am replacing grep PAT | awk '{AWK_SCRIPT}' with awk '/PAT/{AWK_SCRIPT}'. No change in the logic of this line.

XCATDEST=`echo $parm |awk -F= '{print $2}'`
fi
done
read XCATDEST < <(grep xcatd= /proc/cmdline| sed 's/.*xcatd=\([^ ]*\).*/\1/')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest read -r instead of read.

What if there is something like some_prefix_xcatd=something in /proc/cpuinfo. Maybe, need \s before xcatd=.

sed -e 's/.*\sxcatd=\([^ ]*\).*/\1/'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the kcmdline starts with xcatd=? It doesn't matter either way as I am replacing everything before xcatd anyway, whether it contains spaces or not.

Can you give an example of a kcmline element name that may contain xcatd as a substring

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is fine. Just leave it as is.

XCATDEST=`echo $parm |awk -F= '{print $2}'`
fi
done
read XCATDEST < <(grep xcatd= /proc/cmdline| sed 's/.*xcatd=\([^ ]*\).*/\1/')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest read -r instead of read.

Need \s before xcatd=.

sed -e 's/.*\sxcatd=\([^ ]*\).*/\1/'

@samveen
Copy link
Member Author

samveen commented Mar 14, 2018

@neo954 please review again. fixes done as per your review.

echo $line > /tmp/cpumod
done
CPUTYPE=`cat /tmp/cpumod|awk -F':' '{print $2}'|sed -e 's/^ //'`
read -r CPUCOUNT CPUTYPE < <(awk -F: 'BEGIN{c=0; n=""} /model name/{c=c+1; n=$2} END{printf "%d %s",c,n}' /proc/cpuinfo)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, the file, /proc/cpuinfo from a Power 8 test machine.

# cat /proc/cpuinfo
processor	: 0
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 1
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 2
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 3
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 4
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 5
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 6
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 7
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 8
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 9
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 10
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 11
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 12
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 13
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 14
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 15
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 16
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 17
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 18
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 19
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 20
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 21
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 22
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 23
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 24
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 25
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 26
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 27
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 28
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 29
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 30
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 31
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 32
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 33
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 34
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 35
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 36
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 37
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 38
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 39
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 40
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 41
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 42
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 43
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 44
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 45
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 46
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 47
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 48
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 49
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 50
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 51
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 52
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 53
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 54
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 55
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 56
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 57
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 58
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 59
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 60
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 61
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 62
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 63
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 64
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 65
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 66
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 67
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 68
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 69
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 70
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 71
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 72
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 73
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 74
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 75
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 76
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 77
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 78
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 79
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 80
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 81
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 82
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 83
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 84
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 85
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 86
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 87
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 88
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 89
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 90
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 91
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 92
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 93
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 94
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 95
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 96
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 97
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 98
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 99
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 100
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 101
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 102
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 103
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 104
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 105
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 106
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 107
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 108
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 109
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 110
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 111
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 112
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 113
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 114
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 115
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 116
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 117
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 118
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 119
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 120
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 121
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 122
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 123
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 124
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 125
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 126
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

processor	: 127
cpu		: POWER8 (raw), altivec supported
clock		: 3857.000000MHz
revision	: 2.0 (pvr 004d 0200)

timebase	: 512000000
platform	: PowerNV
model		: 8001-22C
machine		: PowerNV 8001-22C
firmware	: OPAL

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neo954 Can you confirm the value of CPUTYPE with the old code against the above? I get the same value with old code as with new code for the cpuinfo contents for P8 as above:

[samveen@development bin]$ grep -e "^cpu\s*:" /tmp/cpuinfo.P8 | while read line; do  echo $line > /tmp/cpumod ; done
[samveen@development bin]$ cat /tmp/cpumod|awk -F':' '{print $2}'|sed -e 's/^ //'
POWER8 (raw), altivec supported

echo $line > /tmp/cpumod
done
CPUTYPE=`cat /tmp/cpumod|awk -F':' '{print $2}'|sed -e 's/^ //'`
PLATFORM="$(awk '/^platform\s*:/{print $3}' /proc/cpuinfo)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, the file, /proc/cpuinfo from a Power 7 Logical Partition.

# cat /proc/cpuinfo
processor	: 0
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 1
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 2
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 3
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 4
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 5
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 6
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 7
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 8
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 9
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 10
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 11
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 12
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 13
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 14
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

processor	: 15
cpu		: POWER7 (architected), altivec supported
clock		: 3550.000000MHz
revision	: 2.0 (pvr 003f 0200)

timebase	: 512000000
platform	: pSeries
model		: IBM,8233-E8B
machine		: CHRP IBM,8233-E8B

Copy link
Member Author

@samveen samveen Mar 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neo954 Can you confirm the output expected from the old code. I get CPUTYPE value POWER7 (architected), altivec supported from the old code too.

[samveen@dev bin]$ grep -e "^cpu\s*:" /tmp/cpuinfo.P7LP | while read line; do 
> echo $line > /tmp/cpumod 
> done
[samveen@dev bin]$ cat /tmp/cpumod|awk -F':' '{print $2}'|sed -e 's/^ //'
POWER7 (architected), altivec supported

done
CPUTYPE=`cat /tmp/cpumod|awk -F':' '{print $2}'|sed -e 's/^ //'`
PLATFORM="$(awk '/^platform\s*:/{print $3}' /proc/cpuinfo)"
read CPUCOUNT CPUTYPE < <(awk -F':' 'BEGIN{c=0; n=""} /^cpu\s*:/{c=c+1; n=$2} END{printf "%d %s",c,n}' /proc/cpuinfo)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, the file, /proc/cpuinfo from a Power 8 KVM guest.

# cat /proc/cpuinfo
processor	: 0
cpu		: POWER8E (raw), altivec supported
clock		: 3425.000000MHz
revision	: 2.1 (pvr 004b 0201)

processor	: 1
cpu		: POWER8E (raw), altivec supported
clock		: 3425.000000MHz
revision	: 2.1 (pvr 004b 0201)

processor	: 2
cpu		: POWER8E (raw), altivec supported
clock		: 3425.000000MHz
revision	: 2.1 (pvr 004b 0201)

processor	: 3
cpu		: POWER8E (raw), altivec supported
clock		: 3425.000000MHz
revision	: 2.1 (pvr 004b 0201)

timebase	: 512000000
platform	: pSeries
model		: IBM pSeries (emulated by qemu)
machine		: CHRP IBM pSeries (emulated by qemu)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neo954 Can you confirm the value of CPUTYPE with the old code against the above? I get the same value with old code as with new code for P8 KVM guest:

[samveen@development bin]$ grep -e "^cpu\s*:" /tmp/cpuinfo.P8KVMG | while read line; do  echo $line > /tmp/cpumod ; done
[samveen@development bin]$ cat /tmp/cpumod|awk -F':' '{print $2}'|sed -e 's/^ //'
POWER8E (raw), altivec supported

@samveen
Copy link
Member Author

samveen commented Apr 13, 2018

@neo954 Can you please check my comments and review again?

@neo954
Copy link
Contributor

neo954 commented Apr 13, 2018

@samveen, It is great if you can paste your unit test result in this pull request.

@neo954 neo954 modified the milestones: 2.14, 2.14.1 Apr 13, 2018
@neo954 neo954 removed this from the 2.14.1 milestone May 29, 2018
@robin2008
Copy link
Member

@neo954 Do you have any further comments on this PR after @samveen updated?

@robin2008 robin2008 added this to the 2.14.2 milestone May 30, 2018
@samveen
Copy link
Member Author

samveen commented Jun 12, 2018

@neo954 please give me an example set of results you expect. My testing shows no difference between the output of the old code and the new code as shown above.

@neo954
Copy link
Contributor

neo954 commented Jun 12, 2018

@samveen, It is great if you can paste your unit test result in this pull request.

@zet809
Copy link

zet809 commented Jul 11, 2018

hi, @samveen , there are so many files changed and hard to verify files one by one, it will be greatly helpful if you can paste your UT result in this PR. Thx a lot!

@zet809 zet809 modified the milestones: 2.14.2, 2.14.3 Jul 11, 2018
@xcat2 xcat2 deleted a comment from chuckbrazie Jul 14, 2018
@whowutwut
Copy link
Member

@zet809 @hu-weihua Can we build this PR branch and run it through our daily regression? Or target next weekend for a run of this branch? Would that help resolve some uncertainty?

@hu-weihua
Copy link

@whowutwut, we can build this PR branch, but I do not think that daily regression can help to verify this PR. Due to the all change of this PR is against genesis, there are only 3 test cases in daily regression will touch this part, but they just test very few code branch of genesis, such as enter shell, cmdline and runimg. Normally, FVT test discovery process manually. So, if we need to verify all the change of this PR, we need to plan a detailed test which needs big effort.

@samveen
Copy link
Member Author

samveen commented Jul 23, 2018

@whowutwut @zet809 Due to the acquisition of my company by a larger company, xCAT as a provisioning and control system has been deprecated with the expectation that we're to move all operations to the systems of the acquiring company, so I haven't been able to concentrate much on xCAT related work, as it is no longer a management sanctioned use of my time. So, please give me a little time to try and rebase all changes on master, and see what I can do about unit test results.

@zet809
Copy link

zet809 commented Jul 23, 2018

Thx, @samveen
BTW, what tool you are using to do system management in the acquiring company?

@samveen
Copy link
Member Author

samveen commented Jul 27, 2018

@zet809 It's a homebrewed system that they've developed over the years, amazing but pretty customized to their requirements.

@zet809 zet809 removed this from the 2.14.3 milestone Aug 17, 2018
@samveen
Copy link
Member Author

samveen commented Sep 17, 2018

@zet809 @neo954 I will close this MR, rebase on master and break it into 2 MRs:

  • One MR with the changes that don't affect Power hosts
  • a second MR with the changes that affect Power hosts

@gurevichmark
Copy link
Contributor

Due to limited resources can no resolve conflicts or verify code changes.

@whowutwut
Copy link
Member

@gurevichmark Like we had discussed, if we want to create more robust testing framework where we can catch issues that @neo954 pointed out about cross architecture issues automatically by running the PR through a test bucket, this would be a perfect candidate for adding a corner case...

But maybe we would never get around to it, so i'm OK to close this also 😄

@samveen
Copy link
Member Author

samveen commented Jul 4, 2019

@whowutwut I've finally got around to splitting this MR into 2 portions: simple improvements vs architecturally dependent improvements. The first of these is up for review and merge. Once that is done, I'll check in the 2nd portion and create an MR for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants