[libvirt] [PATCH v2 0/2] tests: Add nodeinfo test data utility scripts
Changes from v1: * use a better approach to copy host data (thanks Martin) * acknowledge the fact that the scripts require bash * other small cleanups and improvements Andrea Bolognani (2): tests: Add script to display nodeinfo test data tests: Add script to copy nodeinfo test data from host tests/nodeinfodata/copy-from-host.sh | 113 +++++++++++++++++++++++++++++++++++ tests/nodeinfodata/display.sh | 113 +++++++++++++++++++++++++++++++++++ 2 files changed, 226 insertions(+) create mode 100755 tests/nodeinfodata/copy-from-host.sh create mode 100755 tests/nodeinfodata/display.sh -- 2.4.3
The output has been modeled after that of ppc64_cpu --info but it's not limited to ppc64 test data. --- tests/nodeinfodata/display.sh | 113 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100755 tests/nodeinfodata/display.sh diff --git a/tests/nodeinfodata/display.sh b/tests/nodeinfodata/display.sh new file mode 100755 index 0000000..104c9d7 --- /dev/null +++ b/tests/nodeinfodata/display.sh @@ -0,0 +1,113 @@ +#!/bin/bash + +# display.sh - Display nodeinfo test data in a nice way +# Copyright (C) 2015 Red Hat, Inc. +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library. If not, see +# <http://www.gnu.org/licenses/>. +# +# Author: Andrea Bolognani <abologna@redhat.com> + +die() { + local message=$1 + + echo "$message" >&2 + + exit 1 +} + +list_cpus() { + local datadir=$1 + + local index + + ls -d "$datadir/cpu/cpu"* | while read index; do + index=${index#*cpu/cpu} + case "$index" in + [0-9]*) + echo "$index" + ;; + esac + done | sort -un +} + +is_online() { + local datadir=$1 + local cpu=$2 + + local ret=1 + + # In some cases this file will not be available, eg. for cpu0 on an Intel + # system. libvirt considers such CPU to be online, and so should we + if test -e "$datadir/cpu/cpu$cpu/online"; then + ret=$(cat "$datadir/cpu/cpu$cpu/online") + fi + + # Reverse boolean to use it as return code + case "$ret" in + 0) ret=1 ;; + 1) ret=0 ;; + esac + + return $ret; +} + +main() { + local datadir=$1 + local threads_per_core=$2 + + if test "$#" -lt 2; then + die "Usage: $SELF DATADIR THREADS_PER_CORE" + fi + if ! test -d "$datadir"; then + die "$datadir: No such directory" + fi + # This also takes care of non-numeric values on the command line + if ! test "$threads_per_core" -gt 0 >/dev/null 2>&1; then + die "$threads_per_core: Must be a positive number" + fi + + echo "Threads per core: $threads_per_core" + + # This information might not be available (eg. older kernels) + echo -n 'Present CPUs: ' + if test -e "$datadir/cpu/present"; then + cat "$datadir/cpu/present" + else + echo 'information not available' + fi + + let core=0 + for cpu in $(list_cpus "$datadir"); do + + mod=$(expr "$cpu" % "$threads_per_core") + if test "$mod" -eq "0"; then + printf "\nCore %3d:" "$core" + let core++ + fi + + if is_online "$datadir" "$cpu"; then + printf " %4d*" "$cpu" + else + printf " %4d " "$cpu" + fi + done + echo + + return 0 +} + +SELF=$0 +main "$@" +exit $? -- 2.4.3
Only the files libvirt actually parses are copied over, to avoid needlessly increasing the size of the repository and causing dist issues (eg. symlinks loops). --- tests/nodeinfodata/copy-from-host.sh | 113 +++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100755 tests/nodeinfodata/copy-from-host.sh diff --git a/tests/nodeinfodata/copy-from-host.sh b/tests/nodeinfodata/copy-from-host.sh new file mode 100755 index 0000000..ed37865 --- /dev/null +++ b/tests/nodeinfodata/copy-from-host.sh @@ -0,0 +1,113 @@ +#!/bin/bash + +# copy-from-host.sh - Copy nodeinfo test data from a running host +# Copyright (C) 2015 Red Hat, Inc. +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library. If not, see +# <http://www.gnu.org/licenses/>. +# +# Author: Andrea Bolognani <abologna@redhat.com> + +SYSFS_PATH="/sys/devices/system" +NODE_TOPLEVEL_COPY="online possible" +NODE_COPY="cpu[0-9]* cpulist cpumap meminfo" +CPU_TOPLEVEL_COPY="kernel_max offline online possible present" +CPU_COPY="online topology" + +die() { + local message=$1 + + echo "$message" >&2 + + exit 1 +} + +create() { + local directory=$1 + + if ! mkdir -p "$directory" >/dev/null 2>&1; then + die "$directory: Unable to create directory" + fi +} + +copy() { + local pattern=$1 + local from=$2 + local to=$3 + + local item + + echo " $to/$pattern" + + for item in "$from/"$pattern; do + # Some files, like cpu/cpu0/online on Intel systems or cpu/present on + # older kernels, just don't exist. Skip them without making a fuss + if ! test -e "$item"; then + continue + fi + if ! cp -r "$item" "$to" >/dev/null 2>&1; then + die "$item: Unable to copy item" + fi + done +} + +main() { + local name=$1 + + if test "$#" -lt 1; then + die "Usage: $SELF NAME" + fi + + # Create destination directories + if test -e "$name"; then + die "$name: File or directory already exists" + fi + for directory in "$name" "$name/node" "$name/cpu"; do + create "$directory" + done + + # Copy host files + echo "Global node information..." + for pattern in $NODE_TOPLEVEL_COPY; do + copy "$pattern" "$SYSFS_PATH/node" "$name/node" + done + echo "Per-node information..." + for node in "$SYSFS_PATH/node/node"[0-9]*; do + node=${node##*/} + create "$name/node/$node" + for pattern in $NODE_COPY; do + copy "$pattern" "$SYSFS_PATH/node/$node" "$name/node/$node" + done + done + echo "Global CPU information..." + for pattern in $CPU_TOPLEVEL_COPY; do + copy "$pattern" "$SYSFS_PATH/cpu" "$name/cpu" + done + echo "Per-CPU information..." + for cpu in "$SYSFS_PATH/cpu/cpu"[0-9]*; do + cpu=${cpu##*/} + create "$name/cpu/$cpu" + for pattern in $CPU_COPY; do + copy "$pattern" "$SYSFS_PATH/cpu/$cpu" "$name/cpu/$cpu" + done + done + + echo "All done" + + return 0 +} + +SELF=$0 +main "$@" +exit $? -- 2.4.3
On 10/07/2015 08:57 AM, Andrea Bolognani wrote:
Changes from v1:
* use a better approach to copy host data (thanks Martin) * acknowledge the fact that the scripts require bash * other small cleanups and improvements
Andrea Bolognani (2): tests: Add script to display nodeinfo test data tests: Add script to copy nodeinfo test data from host
tests/nodeinfodata/copy-from-host.sh | 113 +++++++++++++++++++++++++++++++++++ tests/nodeinfodata/display.sh | 113 +++++++++++++++++++++++++++++++++++ 2 files changed, 226 insertions(+) create mode 100755 tests/nodeinfodata/copy-from-host.sh create mode 100755 tests/nodeinfodata/display.sh
I'm ambivalent on this pair. Not sure what the value of patch 1 is? What should I expect to see given the arguments? What does "ppc64_cpu --info" show? Perhaps the better question is - if you run on each directory in nodeinfodata do you get what you expect? As for patch 2, one would have to know they should use the copy-from-host.sh script. Perhaps what might be better and/or somewhat more interesting on this one is some make check rule that scans the nodeinfodata trees looking for files that shouldn't be there. That way if someone does use their own methodology to copy over the tree we'd know it (and could message to use the copy-from-host.sh script... John
On Wed, 2015-10-21 at 17:43 -0400, John Ferlan wrote:
Andrea Bolognani (2): tests: Add script to display nodeinfo test data tests: Add script to copy nodeinfo test data from host
tests/nodeinfodata/copy-from-host.sh | 113 +++++++++++++++++++++++++++++++++++ tests/nodeinfodata/display.sh | 113 +++++++++++++++++++++++++++++++++++ 2 files changed, 226 insertions(+) create mode 100755 tests/nodeinfodata/copy-from-host.sh create mode 100755 tests/nodeinfodata/display.sh
I'm ambivalent on this pair.
Not sure what the value of patch 1 is? What should I expect to see given the arguments? What does "ppc64_cpu --info" show? Perhaps the better question is - if you run on each directory in nodeinfodata do you get what you expect?
I've run the script on every existing dataset and the output was correct, as far as I can tell. The script was immensely useful to me back when I was implementing changes to the way the nodeinfo code counts CPUs when subcorese are involved, eg. $ ./display.sh linux-subcores3 8 Threads per core: 8 Present CPUs: 0-159 Core 0: 0 1 2 3 4 5 6 7 Core 1: 8* 9 10 11 12 13 14 15 Core 2: 16 17 18 19 20 21 22 23 Core 3: 24 25 26 27 28 29 30 31 Core 4: 32 33 34 35 36 37 38 39 Core 5: 40* 41 42 43 44 45 46 47 Core 6: 48* 49 50 51 52 53 54 55 Core 7: 56* 57 58 59 60 61 62 63 Core 8: 64 65 66 67 68* 69 70 71 Core 9: 72* 73 74 75 76 77 78 79 Core 10: 80* 81 82 83 84 85 86 87 Core 11: 88* 89 90 91 92 93 94 95 Core 12: 96* 97 98 99 100 101 102 103 Core 13: 104* 105 106 107 108 109 110 111 Core 14: 112* 113 114 115 116 117 118 119 Core 15: 120 121 122 123 124 125 126 127 Core 16: 128* 129 130 131 132 133 134 135 Core 17: 136* 137 138 139 140 141 142 143 Core 18: 144 145 146 147 148 149 150 151 Core 19: 152* 153* 154* 155* 156* 157* 158* 159* You can see at a glance there's something wrong with this configuration - why is CPU 68 online? What about the last line? This kind of report is especially useful when dealing with processors with a high number of CPUs.
As for patch 2, one would have to know they should use the copy-from-host.sh script. Perhaps what might be better and/or somewhat more interesting on this one is some make check rule that scans the nodeinfodata trees looking for files that shouldn't be there. That way if someone does use their own methodology to copy over the tree we'd know it (and could message to use the copy-from-host.sh script...
I agree, as it stands it's not very discoverable, plus adding the check you suggest would also prevent something like e739d95 from ever being needed again. I'll work on that as soon as I have some time. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
On Thu, Oct 22, 2015 at 10:42:54AM +0200, Andrea Bolognani wrote:
On Wed, 2015-10-21 at 17:43 -0400, John Ferlan wrote:
Andrea Bolognani (2): tests: Add script to display nodeinfo test data tests: Add script to copy nodeinfo test data from host
tests/nodeinfodata/copy-from-host.sh | 113 +++++++++++++++++++++++++++++++++++ tests/nodeinfodata/display.sh | 113 +++++++++++++++++++++++++++++++++++ 2 files changed, 226 insertions(+) create mode 100755 tests/nodeinfodata/copy-from-host.sh create mode 100755 tests/nodeinfodata/display.sh
I'm ambivalent on this pair.
Not sure what the value of patch 1 is? What should I expect to see given the arguments? What does "ppc64_cpu --info" show? Perhaps the better question is - if you run on each directory in nodeinfodata do you get what you expect?
I've run the script on every existing dataset and the output was correct, as far as I can tell.
The script was immensely useful to me back when I was implementing changes to the way the nodeinfo code counts CPUs when subcorese are involved, eg.
$ ./display.sh linux-subcores3 8 Threads per core: 8 Present CPUs: 0-159
Core 0: 0 1 2 3 4 5 6 7 Core 1: 8* 9 10 11 12 13 14 15 Core 2: 16 17 18 19 20 21 22 23 Core 3: 24 25 26 27 28 29 30 31 Core 4: 32 33 34 35 36 37 38 39 Core 5: 40* 41 42 43 44 45 46 47 Core 6: 48* 49 50 51 52 53 54 55 Core 7: 56* 57 58 59 60 61 62 63 Core 8: 64 65 66 67 68* 69 70 71 Core 9: 72* 73 74 75 76 77 78 79 Core 10: 80* 81 82 83 84 85 86 87 Core 11: 88* 89 90 91 92 93 94 95 Core 12: 96* 97 98 99 100 101 102 103 Core 13: 104* 105 106 107 108 109 110 111 Core 14: 112* 113 114 115 116 117 118 119 Core 15: 120 121 122 123 124 125 126 127 Core 16: 128* 129 130 131 132 133 134 135 Core 17: 136* 137 138 139 140 141 142 143 Core 18: 144 145 146 147 148 149 150 151 Core 19: 152* 153* 154* 155* 156* 157* 158* 159*
You can see at a glance there's something wrong with this configuration - why is CPU 68 online? What about the last line? This kind of report is especially useful when dealing with processors with a high number of CPUs.
As for patch 2, one would have to know they should use the copy-from-host.sh script. Perhaps what might be better and/or somewhat more interesting on this one is some make check rule that scans the nodeinfodata trees looking for files that shouldn't be there. That way if someone does use their own methodology to copy over the tree we'd know it (and could message to use the copy-from-host.sh script...
I agree, as it stands it's not very discoverable, plus adding the check you suggest would also prevent something like e739d95 from ever being needed again.
I'll work on that as soon as I have some time.
Maybe simple .gitignore entry would suffice.
Cheers.
-- Andrea Bolognani Software Engineer - Virtualization Team
On Thu, 2015-10-22 at 13:27 +0200, Martin Kletzander wrote:
As for patch 2, one would have to know they should use the copy-from-host.sh script. Perhaps what might be better and/or somewhat more interesting on this one is some make check rule that scans the nodeinfodata trees looking for files that shouldn't be there. That way if someone does use their own methodology to copy over the tree we'd know it (and could message to use the copy-from-host.sh script...
I agree, as it stands it's not very discoverable, plus adding the check you suggest would also prevent something like e739d95 from ever being needed again.
I'll work on that as soon as I have some time.
Maybe simple .gitignore entry would suffice.
Problem is, .gitignore works well if you have the list of files you want to avoid, but we have the complementary information: the list of files we want to keep. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
On Thu, Oct 22, 2015 at 02:43:09PM +0200, Andrea Bolognani wrote:
On Thu, 2015-10-22 at 13:27 +0200, Martin Kletzander wrote:
As for patch 2, one would have to know they should use the copy-from-host.sh script. Perhaps what might be better and/or somewhat more interesting on this one is some make check rule that scans the nodeinfodata trees looking for files that shouldn't be there. That way if someone does use their own methodology to copy over the tree we'd know it (and could message to use the copy-from-host.sh script...
I agree, as it stands it's not very discoverable, plus adding the check you suggest would also prevent something like e739d95 from ever being needed again.
I'll work on that as soon as I have some time.
Maybe simple .gitignore entry would suffice.
Problem is, .gitignore works well if you have the list of files you want to avoid, but we have the complementary information: the list of files we want to keep.
You could do that as well, I took the liberty of trying that out ant it looks like it works, but looking at it, it's very ugly. Decide yourself whether you want to use it or not, I'll just paste the file below. The .gitignore file goes into tests/nodeinfodata/.gitignore: /*/* !/*/node/ /*/node/* !/*/node/online !/*/node/possible !/*/node/node*/ /*/node/node*/* !/*/node/node*/cpu[0-9]* !/*/node/node*/cpulist !/*/node/node*/cpumap !/*/node/node*/meminfo !/*/cpu/ /*/cpu/* !/*/cpu/kernel_max !/*/cpu/offline !/*/cpu/online !/*/cpu/possible !/*/cpu/present !/*/cpu/cpu[0-9]*/ /*/cpu/cpu*/* !/*/cpu/cpu*/online !/*/cpu/cpu*/topology
participants (3)
-
Andrea Bolognani -
John Ferlan -
Martin Kletzander