2:57 p.m.
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
2:57 p.m.
New subject: [libvirt] [PATCH v2 1/2] tests: Add script to display nodeinfo test data
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(a)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
2:57 p.m.
New subject: [libvirt] [PATCH v2 2/2] tests: Add script to copy nodeinfo test data from host
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(a)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
11:43 p.m.
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
10:42 a.m.
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
1:27 p.m.
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
2:43 p.m.
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
9:16 a.m.
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
3253
days inactive
3269
days old
7 comments
3 participants
participants (3)
-
Andrea Bolognani -
John Ferlan -
Martin Kletzander