Re: [libvirt] [PATCH] Fix nodeinfo output on PPC64 KVM hosts

[Andrea Bolognani]

 
  
  





  


  67108864
  1048576
  0
  0
  




  
  





  


  67108864
  1048576
  0
  0
  




  
  





  

  


With the patch:


  

  263635136
  







  

  


This issue needs to be addressed before the changes can be considered
for inclusion. I'd also highly recommend writing tests cases in order
to prevent regressions in the future.

Cheers.

--
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Fix typo incomaptible -> incompatible

[Andrea Bolognani]

---
 src/cpu/cpu_x86.c | 4 ++--
 src/libvirt-domain-snapshot.c | 2 +-
 src/qemu/qemu_driver.c| 8 
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 2a14705..f5f7697 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -1507,14 +1507,14 @@ x86Compute(virCPUDefPtr host,
 static virCPUCompareResult
 x86Compare(virCPUDefPtr host,
virCPUDefPtr cpu,
-   bool failIncomaptible)
+   bool failIncompatible)
 {
 virCPUCompareResult ret;
 char *message = NULL;
 
 ret = x86Compute(host, cpu, NULL, &message);
 
-if (failIncomaptible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) {
+if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) {
 ret = VIR_CPU_COMPARE_ERROR;
 if (message) {
 virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", message);
diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c
index 4625e03..b7c566f 100644
--- a/src/libvirt-domain-snapshot.c
+++ b/src/libvirt-domain-snapshot.c
@@ -1048,7 +1048,7 @@ virDomainSnapshotHasMetadata(virDomainSnapshotPtr 
snapshot,
  * new hypervisor instance rather than reusing the existing hypervisor
  * (since this would terminate all connections to the domain, such as
  * such as VNC or Spice graphics) - this condition arises from active
- * snapshots that are provably ABI incomaptible, as well as from
+ * snapshots that are provably ABI incompatible, as well as from
  * inactive snapshots with a @flags request to start the domain after
  * the revert.
  *
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2b530c8..bdf9b15 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12948,7 +12948,7 @@ qemuConnectCompareCPU(virConnectPtr conn,
 virQEMUDriverPtr driver = conn->privateData;
 int ret = VIR_CPU_COMPARE_ERROR;
 virCapsPtr caps = NULL;
-bool failIncomaptible;
+bool failIncompatible;
 
 virCheckFlags(VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE,
   VIR_CPU_COMPARE_ERROR);
@@ -12956,14 +12956,14 @@ qemuConnectCompareCPU(virConnectPtr conn,
 if (virConnectCompareCPUEnsureACL(conn) < 0)
 goto cleanup;
 
-failIncomaptible = !!(flags & VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE);
+failIncompatible = !!(flags & VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE);
 
 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
 goto cleanup;
 
 if (!caps->host.cpu ||
 !caps->host.cpu->model) {
-if (failIncomaptible) {
+if (failIncompatible) {
 virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s",
_("cannot get host CPU capabilities"));
 } else {
@@ -12971,7 +12971,7 @@ qemuConnectCompareCPU(virConnectPtr conn,
 ret = VIR_CPU_COMPARE_INCOMPATIBLE;
 }
 } else {
-ret = cpuCompareXML(caps->host.cpu, xmlDesc, failIncomaptible);
+ret = cpuCompareXML(caps->host.cpu, xmlDesc, failIncompatible);
 }
 
  cleanup:
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Fix nodeinfo output on PPC64 KVM hosts

[Andrea Bolognani]

On Wed, 2015-07-01 at 19:26 +0530, Shivaprasad bhat wrote:
> 
> I have posted the v2 as per your suggestions. Also added test case to
> test the nodeinfo. I moved the ioctl to
> a new function to help mocking it for testing.

Only commit 1/2 seems to have made it to the list...

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 1/2] Fix nodeinfo output on PPC64 KVM hosts

[Andrea Bolognani]

From: Shivaprasad G Bhat 

The nodeinfo is reporting incorrect number of cpus and incorrect host
topology on PPC64 KVM hosts. The KVM hypervisor on PPC64 needs only
the primary thread in a core to be online, and the secondaries offlined.
While scheduling a guest in, the kvm scheduler wakes up the secondaries to
run in guest context.

The host scheduling of the guests happen at the core level(as only primary
thread is online). The kvm scheduler exploits as many threads of the core
as needed by guest. Further, starting POWER8, the processor allows splitting
a physical core into multiple subcores with 2 or 4 threads each. Again, only
the primary thread in a subcore is online in the host. The KVM-PPC
scheduler allows guests to exploit all the offline threads in the subcore,
by bringing them online when needed.
(Kernel patches on split-core 
http://www.spinics.net/lists/kvm-ppc/msg09121.html)

Recently with dynamic micro-threading changes in ppc-kvm, makes sure
to utilize all the offline cpus across guests, and across guests with
different cpu topologies.
(https://www.mail-archive.com/kvm@vger.kernel.org/msg115978.html)

Since the offline cpus are brought online in the guest context, it is safe
to count them as online. Nodeinfo today discounts these offline cpus from
cpu count/topology calclulation, and the nodeinfo output is not of any help
and the host appears overcommited when it is actually not.

The patch carefully counts those offline threads whose primary threads are
online. The host topology displayed by the nodeinfo is also fixed when the
host is in valid kvm state.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Andrea Bolognani 
---
 src/libvirt_private.syms |   1 +
 src/nodeinfo.c   | 140 ++-
 src/nodeinfo.h   |   1 +
 3 files changed, 129 insertions(+), 13 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1566d11..64644a2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1008,6 +1008,7 @@ nodeGetInfo;
 nodeGetMemory;
 nodeGetMemoryParameters;
 nodeGetMemoryStats;
+nodeGetThreadsPerSubcore;
 nodeSetMemoryParameters;
 
 
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 2fafe2d..25a6471 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -32,6 +32,12 @@
 #include 
 #include 
 #include "conf/domain_conf.h"
+#include 
+#include 
+
+#if HAVE_LINUX_KVM_H
+# include 
+#endif
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
 # include 
@@ -428,28 +434,88 @@ virNodeParseNode(const char *node,
 unsigned int cpu;
 int online;
 int direrr;
+int lastonline;
+virBitmapPtr cpu_map = NULL;
+int threads_per_subcore = 0;
 
 *threads = 0;
 *cores = 0;
 *sockets = 0;
 
+/* PPC-KVM needs the secondary threads of a core to be offline on the
+ * host. The kvm scheduler brings the secondary threads online in the
+ * guest context. Moreover, P8 processor has split-core capability
+ * where, there can be 1,2 or 4 subcores per core. The primaries of the
+ * subcores alone will be online on the host for a subcore in the
+ * host. Even though the actual threads per core for P8 processor is 8,
+ * depending on the subcores_per_core = 1, 2 or 4, the threads per
+ * subcore will vary accordingly to 8, 4 and 2 repectively.
+ * So, On host threads_per_core what is arrived at from sysfs in the
+ * current logic is actually the subcores_per_core. Threads per subcore
+ * can only be obtained from the kvm device. For example, on P8 wih 1
+ * core having 8 threads, sub_cores_percore=4, the threads 0,2,4 & 6
+ * will be online. The sysfs reflects this and in the current logic
+ * variable 'threads' will be 4 which is nothing but subcores_per_core.
+ * If the user tampers the cpu online/offline states using chcpu or other
+ * means, then it is an unsupported configuration for kvm.
+ * The code below tries to keep in mind
+ *  - when the libvirtd is run inside a KVM guest or Phyp based guest.
+ *  - Or on the kvm host where user manually tampers the cpu states to
+ *offline/online randomly.
+ * On hosts other than POWER this will be 0, in which case a simpler
+ * thread-counting logic will be used  */
+if ((threads_per_subcore = nodeGetThreadsPerSubcore(arch)) < 0)
+goto cleanup;
+
+/* Keep track of node CPUs in a bitmap so that we can iterate
+ * through them in guaranteed numeric order, which is required to
+ * find out whether a thread is primary or secondary */
+if ((cpu_map = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN)) == NULL)
+goto cleanup;
+
 if (!(cpudir = opendir(node))) {
 virReportSystemError(errno, _("cannot opendir %s"), node);
 goto cleanup;
 }
 
-/* enumerate sockets in the node */
-CPU_ZERO(&sock_map);
 while ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0)

[libvirt] [PATCH v3 0/2] Fix nodeinfo output on PPC64 KVM hosts

[Andrea Bolognani]

I have reworked the original code quite a bit, while
maintaining the same logic and hopefully introducing
no bugs of my own :)

Notable changes from v2 to v3:

  * the function to get the number of threads per subcore
has been moved to the from virarch.c, which deals with
architecture names only and is therefore not the right
place to read host configuration, to nodeinfo.c where
the rest of this stuff lives;

  * said function has also been given a shorter name;

  * the "valid subcore mode" boolean has been removed:
threads_per_subcore will be a positive number if
subcores should be taken into account, and if that's
not the case (x86 host, tainted configuration) it
will simply be zero, so now the code needs to keep
track of a single variable instead of two;

  * the test case has been renamed to be more
descriptive;

  * the test data has been cleaned up by removing all
cpu/cpu*/node* links, which prevented 'make dist'
from working due to recursive linking.

I've added a Signed-off-by: tag to each commit and
maintained Shivaprasad as commit author; if that's not
a proper way to handle authorship information in this
situation please let me know.

Shivaprasad G Bhat (2):
  Fix nodeinfo output on PPC64 KVM hosts
  Add testcase for PPC64 kvm host nodeinfo

 src/libvirt_private.syms   |   1 +
 src/nodeinfo.c | 140 +++--
 src/nodeinfo.h |   1 +
 tests/Makefile.am  |   6 +
 tests/nodeinfodata/linux-ppc64-subcores.cpuinfo|  59 +
 tests/nodeinfodata/linux-ppc64-subcores.expected   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu0/online  |   1 +
 .../linux-subcores/cpu/cpu0/physical_id|   1 +
 .../linux-subcores/cpu/cpu0/topology/core_id   |   1 +
 .../linux-subcores/cpu/cpu0/topology/core_siblings |   1 +
 .../cpu/cpu0/topology/core_siblings_list   |   1 +
 .../cpu/cpu0/topology/physical_package_id  |   1 +
 .../cpu/cpu0/topology/thread_siblings  |   1 +
 .../cpu/cpu0/topology/thread_siblings_list |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu1/online  |   1 +
 .../linux-subcores/cpu/cpu1/physical_id|   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu10/online |   1 +
 .../linux-subcores/cpu/cpu10/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu11/online |   1 +
 .../linux-subcores/cpu/cpu11/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu12/online |   1 +
 .../linux-subcores/cpu/cpu12/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu13/online |   1 +
 .../linux-subcores/cpu/cpu13/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu14/online |   1 +
 .../linux-subcores/cpu/cpu14/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu15/online |   1 +
 .../linux-subcores/cpu/cpu15/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu16/online |   1 +
 .../linux-subcores/cpu/cpu16/physical_id   |   1 +
 .../linux-subcores/cpu/cpu16/topology/core_id  |   1 +
 .../cpu/cpu16/topology/core_siblings   |   1 +
 .../cpu/cpu16/topology/core_siblings_list  |   1 +
 .../cpu/cpu16/topology/physical_package_id |   1 +
 .../cpu/cpu16/topology/thread_siblings |   1 +
 .../cpu/cpu16/topology/thread_siblings_list|   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu17/online |   1 +
 .../linux-subcores/cpu/cpu17/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu18/online |   1 +
 .../linux-subcores/cpu/cpu18/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu19/online |   1 +
 .../linux-subcores/cpu/cpu19/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu2/online  |   1 +
 .../linux-subcores/cpu/cpu2/physical_id|   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu20/online |   1 +
 .../linux-subcores/cpu/cpu20/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu21/online |   1 +
 .../linux-subcores/cpu/cpu21/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu22/online |   1 +
 .../linux-subcores/cpu/cpu22/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu23/online |   1 +
 .../linux-subcores/cpu/cpu23/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu24/online |   1 +
 .../linux-subcores/cpu/cpu24/physical_id   |   1 +
 .../linux-subcores/cpu/cpu24/topology/core_id  |   1 +
 .../cpu/cpu24/topology/core_siblings   |   1 +
 .../cpu/cpu24/topology/core_siblings_list  |   1 +
 .../cpu/cpu24/topology/physical_package_id |   1 +
 .../cpu/cpu24/topology/thread_siblings |   1 +
 .../cpu/cpu24/topology/thread_siblings_list|   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu25/onli

Re: [libvirt] [PATCH] Fix nodeinfo output on PPC64 KVM hosts

[Andrea Bolognani]

On Wed, 2015-07-01 at 19:26 +0530, Shivaprasad bhat wrote:
> 
> I have posted the v2 as per your suggestions. Also added test case to
> test the nodeinfo. I moved the ioctl to
> a new function to help mocking it for testing.

Hi Shivaprasad,

I've just posted a reworked patch set; the changes are described in the
cover letter. Can you please take a look at it and let me know if it
looks okay to you?

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v4 1/2] Fix nodeinfo output on PPC64 KVM hosts

[Andrea Bolognani]

From: Shivaprasad G Bhat 

The nodeinfo is reporting incorrect number of cpus and incorrect host
topology on PPC64 KVM hosts. The KVM hypervisor on PPC64 needs only
the primary thread in a core to be online, and the secondaries offlined.
While scheduling a guest in, the kvm scheduler wakes up the secondaries to
run in guest context.

The host scheduling of the guests happen at the core level(as only primary
thread is online). The kvm scheduler exploits as many threads of the core
as needed by guest. Further, starting POWER8, the processor allows splitting
a physical core into multiple subcores with 2 or 4 threads each. Again, only
the primary thread in a subcore is online in the host. The KVM-PPC
scheduler allows guests to exploit all the offline threads in the subcore,
by bringing them online when needed.
(Kernel patches on split-core 
http://www.spinics.net/lists/kvm-ppc/msg09121.html)

Recently with dynamic micro-threading changes in ppc-kvm, makes sure
to utilize all the offline cpus across guests, and across guests with
different cpu topologies.
(https://www.mail-archive.com/kvm@vger.kernel.org/msg115978.html)

Since the offline cpus are brought online in the guest context, it is safe
to count them as online. Nodeinfo today discounts these offline cpus from
cpu count/topology calclulation, and the nodeinfo output is not of any help
and the host appears overcommited when it is actually not.

The patch carefully counts those offline threads whose primary threads are
online. The host topology displayed by the nodeinfo is also fixed when the
host is in valid kvm state.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Andrea Bolognani 
---
 src/libvirt_private.syms |   1 +
 src/nodeinfo.c   | 138 ++-
 src/nodeinfo.h   |   1 +
 3 files changed, 127 insertions(+), 13 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1566d11..64644a2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1008,6 +1008,7 @@ nodeGetInfo;
 nodeGetMemory;
 nodeGetMemoryParameters;
 nodeGetMemoryStats;
+nodeGetThreadsPerSubcore;
 nodeSetMemoryParameters;
 
 
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 2fafe2d..0b78d7d 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -32,6 +32,12 @@
 #include 
 #include 
 #include "conf/domain_conf.h"
+#include 
+#include 
+
+#if HAVE_LINUX_KVM_H
+# include 
+#endif
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
 # include 
@@ -428,28 +434,86 @@ virNodeParseNode(const char *node,
 unsigned int cpu;
 int online;
 int direrr;
+int lastonline;
+virBitmapPtr cpu_map = NULL;
+int threads_per_subcore = 0;
 
 *threads = 0;
 *cores = 0;
 *sockets = 0;
 
+/* PPC-KVM needs the secondary threads of a core to be offline on the
+ * host. The kvm scheduler brings the secondary threads online in the
+ * guest context. Moreover, P8 processor has split-core capability
+ * where, there can be 1,2 or 4 subcores per core. The primaries of the
+ * subcores alone will be online on the host for a subcore in the
+ * host. Even though the actual threads per core for P8 processor is 8,
+ * depending on the subcores_per_core = 1, 2 or 4, the threads per
+ * subcore will vary accordingly to 8, 4 and 2 repectively.
+ * So, On host threads_per_core what is arrived at from sysfs in the
+ * current logic is actually the subcores_per_core. Threads per subcore
+ * can only be obtained from the kvm device. For example, on P8 wih 1
+ * core having 8 threads, sub_cores_percore=4, the threads 0,2,4 & 6
+ * will be online. The sysfs reflects this and in the current logic
+ * variable 'threads' will be 4 which is nothing but subcores_per_core.
+ * If the user tampers the cpu online/offline states using chcpu or other
+ * means, then it is an unsupported configuration for kvm.
+ * The code below tries to keep in mind
+ *  - when the libvirtd is run inside a KVM guest or Phyp based guest.
+ *  - Or on the kvm host where user manually tampers the cpu states to
+ *offline/online randomly.
+ * On hosts other than POWER this will be 0, in which case a simpler
+ * thread-counting logic will be used  */
+if ((threads_per_subcore = nodeGetThreadsPerSubcore(arch)) < 0)
+goto cleanup;
+
+/* Keep track of node CPUs in a bitmap so that we can iterate
+ * through them in guaranteed numeric order, which is required to
+ * find out whether a thread is primary or secondary */
+if ((cpu_map = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN)) == NULL)
+goto cleanup;
+
 if (!(cpudir = opendir(node))) {
 virReportSystemError(errno, _("cannot opendir %s"), node);
 goto cleanup;
 }
 
-/* enumerate sockets in the node */
-CPU_ZERO(&sock_map);
 while ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0)

[libvirt] [PATCH v4 0/2] Fix nodeinfo output on PPC64 KVM hosts

[Andrea Bolognani]

Changes from v3 to v4:

  * removed a printf() statement;

  * fixed typo in a commit message.

Shivaprasad G Bhat (2):
  Fix nodeinfo output on PPC64 KVM hosts
  Add testcase for PPC64 kvm host nodeinfo

 src/libvirt_private.syms   |   1 +
 src/nodeinfo.c | 138 +++--
 src/nodeinfo.h |   1 +
 tests/Makefile.am  |   6 +
 tests/nodeinfodata/linux-ppc64-subcores.cpuinfo|  59 +
 tests/nodeinfodata/linux-ppc64-subcores.expected   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu0/online  |   1 +
 .../linux-subcores/cpu/cpu0/physical_id|   1 +
 .../linux-subcores/cpu/cpu0/topology/core_id   |   1 +
 .../linux-subcores/cpu/cpu0/topology/core_siblings |   1 +
 .../cpu/cpu0/topology/core_siblings_list   |   1 +
 .../cpu/cpu0/topology/physical_package_id  |   1 +
 .../cpu/cpu0/topology/thread_siblings  |   1 +
 .../cpu/cpu0/topology/thread_siblings_list |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu1/online  |   1 +
 .../linux-subcores/cpu/cpu1/physical_id|   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu10/online |   1 +
 .../linux-subcores/cpu/cpu10/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu11/online |   1 +
 .../linux-subcores/cpu/cpu11/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu12/online |   1 +
 .../linux-subcores/cpu/cpu12/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu13/online |   1 +
 .../linux-subcores/cpu/cpu13/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu14/online |   1 +
 .../linux-subcores/cpu/cpu14/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu15/online |   1 +
 .../linux-subcores/cpu/cpu15/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu16/online |   1 +
 .../linux-subcores/cpu/cpu16/physical_id   |   1 +
 .../linux-subcores/cpu/cpu16/topology/core_id  |   1 +
 .../cpu/cpu16/topology/core_siblings   |   1 +
 .../cpu/cpu16/topology/core_siblings_list  |   1 +
 .../cpu/cpu16/topology/physical_package_id |   1 +
 .../cpu/cpu16/topology/thread_siblings |   1 +
 .../cpu/cpu16/topology/thread_siblings_list|   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu17/online |   1 +
 .../linux-subcores/cpu/cpu17/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu18/online |   1 +
 .../linux-subcores/cpu/cpu18/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu19/online |   1 +
 .../linux-subcores/cpu/cpu19/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu2/online  |   1 +
 .../linux-subcores/cpu/cpu2/physical_id|   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu20/online |   1 +
 .../linux-subcores/cpu/cpu20/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu21/online |   1 +
 .../linux-subcores/cpu/cpu21/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu22/online |   1 +
 .../linux-subcores/cpu/cpu22/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu23/online |   1 +
 .../linux-subcores/cpu/cpu23/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu24/online |   1 +
 .../linux-subcores/cpu/cpu24/physical_id   |   1 +
 .../linux-subcores/cpu/cpu24/topology/core_id  |   1 +
 .../cpu/cpu24/topology/core_siblings   |   1 +
 .../cpu/cpu24/topology/core_siblings_list  |   1 +
 .../cpu/cpu24/topology/physical_package_id |   1 +
 .../cpu/cpu24/topology/thread_siblings |   1 +
 .../cpu/cpu24/topology/thread_siblings_list|   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu25/online |   1 +
 .../linux-subcores/cpu/cpu25/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu26/online |   1 +
 .../linux-subcores/cpu/cpu26/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu27/online |   1 +
 .../linux-subcores/cpu/cpu27/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu28/online |   1 +
 .../linux-subcores/cpu/cpu28/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu29/online |   1 +
 .../linux-subcores/cpu/cpu29/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu3/online  |   1 +
 .../linux-subcores/cpu/cpu3/physical_id|   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu30/online |   1 +
 .../linux-subcores/cpu/cpu30/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu31/online |   1 +
 .../linux-subcores/cpu/cpu31/physical_id   |   1 +
 tests/nodeinfodata/linux-subcores/cpu/cpu32/online |   1 +
 .../linux-subcores/cpu/cpu32/physical_id   |   1 +
 .../linux-subcores/cpu/cpu32/topology/core_id  |   1 +
 .../cpu/cpu32/topology/co

Re: [libvirt] [PATCH v3 1/2] Fix nodeinfo output on PPC64 KVM hosts

[Andrea Bolognani]

On Mon, 2015-07-06 at 17:34 +0530, Shivaprasad bhat wrote:
> 
> Thanks a lot Andrea. The cleanups are really nice. I had a chance to
> test the patch and it
> seems to work consistently in all sucores_per_core modes.
> 
> Only two comments written inline .

Glad you're happy with the changes!

> > +nodeGetThreadsPerSubcore;
> 
> The nodeGetThreadsPerSubcore being PPC specific, is it good
> to have it in nodeinfo.h ? That was the rational on which I had the
> ioctl wrapper
> written in virarch.c in v2.

Even though it's ppc64 specific, all the logic that uses that function
and needs to take the value of threads_per_subcore into account is
already part of that file, so I think it makes sense to have it there.

The maintainers might disagree, of course :)

> > +if (virBitmapSetBit(cpu_map, cpu) < 0) {
> > +printf("virBitmapSetBit(%d)\n", cpu);
> 
> Using printf here. May be you wanted virReportError(?). I think we 
> can
> ignore the SetBit return,
> given this is from directory parsing.

That was indeed not supposed to be there, good catch! I've removed it
and posted v4 of the series.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 1/2] Fix nodeinfo output on PPC64 KVM hosts

[Andrea Bolognani]

On Mon, 2015-07-06 at 17:34 +0530, Shivaprasad bhat wrote:
> 
> I had a chance to
> test the patch and it
> seems to work consistently in all sucores_per_core modes.

Hi Shivaprasad,

while reworking the series to address Martin's comments, I have created
a couple more test cases and I've run into a situation I'm not sure how
to handle.

Suppose we have the following situation on the host:

 0*   1234567  
 8*   9   10   11   12   13   14   15  
16*  17*  18*  19*  20   21   22   23  
24*  25   26   27   28   29   30   31  
32*  33   34   35   36   37   38   39  
40*  41   42   43   44   45   46   47  
48   49   50   51   52   53   54   55  
56   57   58   59   60   61   62   63  
64   65   66   67   68   69   70*  71  
72*  73   74   75   76   77   78   79  
80*  81   82   83   84   85   86   87  
88*  89*  90*  91*  92*  93*  94*  95* 

Basically the user has *really* messed up its configuration :)

Nodes 0 (CPUs 0-23), 2 (CPUs 48-71) and 3 (CPUs 72-95) are easy: in all
cases at least one of the secondary threads is online, so we should
fallback to the subcore-unaware logic and just count the online CPUs.

Node 1 (CPUs 24-47) is different, though: all the primary threads are
online, and all the secondary threads are offline, just like they're
supposed to.

How should the threads in node 1 be counted, then? Should the
subcore-aware logic be applied (resulting in 41 online CPUs overall) or
should we use the fallback logic for all nodes because at least one of
them is violating our expectations (resulting in 20 online CPUs
overall)?

I'm tending towards "it doesn't really matter", eg. if you mess with
the configuration you can't expect the reported capacity to make any
sense. I'd probably go with the former option (because it requires less
changes to the code) and add a paragraph about it to the documentation,
but I'd love to hear your opinion on the subject.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] shall libvirtd validate guest's name ?

[Andrea Bolognani]

On Thu, 2015-07-09 at 17:19 +0800, zhang bo wrote:
> linux-ZyvZnF:~ # virsh list --all
>  IdName   State
> 
>  - redhat7;reboot shut off
>  - oscar-vm-5 shut off
> 
> 
> As shown above, 
> 1 we use command "virsh define a.xml" to define a guest with a name 
> containing ';', that's 'redhat7;reboot'
> 2 then we start the guest: "virsh start redhat7;reboot"
> 3 shell consider the command as
>   a) run "virsh start redhat7", failed
>   b) run "reboot", to reboot the host
>   And *the host get rebooted*.
> 
> shall libvirtd do the guest-name-validation work? Or other 
> suggustions?

Proper usage of string escaping is the user's responsibility.
Unfortunately a lot of shell scripts get this wrong, leading
to more or less catastrophic results.

The same reasoning, by the way, applies to file names, and eg.
the Nautilus file manager happily allows me to use foo;reboot
as name when creating or renaming files...

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v4 0/2] Fix nodeinfo output on PPC64 KVM hosts

[Andrea Bolognani]

On Thu, 2015-07-09 at 11:46 -0400, John Ferlan wrote:
> 
> On 07/07/2015 03:25 AM, Andrea Bolognani wrote:
> > Changes from v3 to v4:
> > 
> >   * removed a printf() statement;
> > 
> >   * fixed typo in a commit message.
> > 
> > Shivaprasad G Bhat (2):
> >   Fix nodeinfo output on PPC64 KVM hosts
> >   Add testcase for PPC64 kvm host nodeinfo
> 
> Never saw the v4 2/2 come through (nor do I see it in the archive);
> however, I assume it's the same as the v3 patch:
> 
> http://www.redhat.com/archives/libvir-list/2015-July/msg00155.html

It apparently didn't make it to the list... It was a pretty big message
so it might be stuck in the moderation queue as previous versions of
the same commit were.

That said yes, v4 2/2 is the same as v3 2/2 so all your comments apply.

> Given it is and what I found reviewing the following:
> 
> http://www.redhat.com/archives/libvir-list/2015-July/msg00219.html
> 
> regarding nodeinfo.c not really using the tests/nodeinfodata local
> path
> instead the running host's sysfs (/sys/devices/system) path.
> 
> I found while testing that the proposed patch wouldn't run correctly
> on
> my host because my /sys/devices/system/cpu/present is "0-3" and the
> patch would fail on any test with cpu4+ since the tests/nodeinfodata/
> present file isn't referenced (if it existed).
> 
> I created a series which adjusts the SYSFS_SYSTEM_PATH logic in
> nodeinfo.c to allow for a supplied path or uses the default:
> 
> http://www.redhat.com/archives/libvir-list/2015-July/msg00278.html
> 
> Not looking for a review of the 9 patch sysfs series, but I am
> curious
> to get a perspective on the patch I initially reviewed which modifies
> virNodeParseNode to "filter out" or "exclude" cpu's that are offline
> because they're defective/empty and perhaps how/if that applies to
> this
> environment as well.

I was actually already planning to review your series, I just
haven't found the time yet :)

As for the change you're referring to, I guess the point is to
detect the case where a cpu is listed among node/node*/cpu* but is
not present, because of hardware faults I guess?

I'd like to hear the rationale from the patch's author, but I can't
find it in the list archives...

> I'm also curious what happens if the 2/2 patch is run on a PPC64 host
> with less than 96 cores (from .../cpu/present) since the results seem
> to
> expect the 96 cores to be present.  It would seem the existing code
> without the sysfs path redirection would fail, since the caller
> linuxNodeInfoCPUPopulate would be using the host's sysfs path rather
> than the tests sysfs path.

The test cases I've added run fine on my laptop, because the code
is not using functions that look at host files.

The current situation, as you noticed, is not optimal because while
some of the functions can be passed a prefix and are hence test-ready,
other use absolute paths and break in mocked environments.

Your series (which I haven't looked at in detail yet) seems to
address that, which is definitely a step forward. Once I've reviewed
it, I will rebase the new version of this patch, which I'm already
working on, on top of your series and send it to the list.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 7/9] nodeinfo: Add sysfs_prefix to nodeCapsInitNUMA

[Andrea Bolognani]

On Tue, 2015-07-07 at 20:26 -0400, John Ferlan wrote:
> Add the sysfs_prefix argument to the call to allow for setting the
> path for tests to something other than SYSFS_CPU_PATH which is a
> derivative of SYSFS_SYSTEM_PATH
> 
> Use cpupath for nodeCapsInitNUMAFake and remove SYSFS_CPU_PATH
> 
> Signed-off-by: John Ferlan 

Just one remark below.

> @@ -1810,26 +1811,27 @@ nodeGetMemoryFake(unsigned long long *mem,
>  
>  /* returns 1 on success, 0 if the detection failed and -1 on hard 
> error */
>  static int
> -virNodeCapsFillCPUInfo(int cpu_id ATTRIBUTE_UNUSED,
> +virNodeCapsFillCPUInfo(const char *cpupath,
> +   int cpu_id ATTRIBUTE_UNUSED,
> virCapsHostNUMACellCPUPtr cpu 
> ATTRIBUTE_UNUSED)

cpupath should be marked ATTRIBUTE_UNUSED as well.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/9] Add sysfs_prefix to nodeinfo.c API's

[Andrea Bolognani]

On Tue, 2015-07-07 at 20:26 -0400, John Ferlan wrote:
> This series adds/processes a sysfs_prefix for the nodeinfo.c API's.
> 
> Although the nodeinfotest.c passes a local test directory path, it 
> was
> never used. 
> 
> This was all brought to light by patch 9 in this series which is
> essentially Kothapally Madhu Pavan's v3 patch:
> 
> http://www.redhat.com/archives/libvir-list/2015-June/msg00395.html
> 
> With the adjustment to call nodeGetPresentCPUBitmap and 
> virNodeParseNode
> with the sysfs prefix.
> 
> Without the first 5 patches, the patch as posted caused nodeinfotest
> failure in my test environment which doesn't have the "larger" 
> environments
> that the test tried to set up because the test environment used my
> present mask file.

Patches 1-8 look good to me. Great job splitting the changes in
such a nice way! I've commented on patch 7 in a separate mail.

I'll look at patch 9 on Monday.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] tests: Add nodeinfo test for non-present CPUs

[Andrea Bolognani]

Some of the possible CPUs in a system might not be present, eg. they
might be defective or might have been deconfigured from the ASM console
in a Power system. Due to this fact, Linux keeps track of what CPUs are
possible and what are present separately.

This test uses the data from a system where not all the possible CPUs
are present to make sure libvirt handles this situation correctly.
---

This patch must be applied on top of John's series of nodeinfo
refactors, especially

  [PATCH 9/9] nodeinfo: fix to parse present cpus rather than possible cpus

which introduces the very fix this new test case is meant to test.

 .../linux-deconfigured-cpus/cpu/cpu0/online|  1 +
 .../linux-deconfigured-cpus/cpu/cpu1/online|  1 +
 .../linux-deconfigured-cpus/cpu/cpu10/online   |  1 +
 .../linux-deconfigured-cpus/cpu/cpu100/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu101/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu102/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu103/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu104/online  |  1 +
 .../cpu/cpu104/topology/core_id|  1 +
 .../cpu/cpu104/topology/core_siblings  |  1 +
 .../cpu/cpu104/topology/core_siblings_list |  1 +
 .../cpu/cpu104/topology/physical_package_id|  1 +
 .../cpu/cpu104/topology/thread_siblings|  1 +
 .../cpu/cpu104/topology/thread_siblings_list   |  1 +
 .../linux-deconfigured-cpus/cpu/cpu105/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu106/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu107/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu108/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu109/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu11/online   |  1 +
 .../linux-deconfigured-cpus/cpu/cpu110/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu111/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu112/online  |  1 +
 .../cpu/cpu112/topology/core_id|  1 +
 .../cpu/cpu112/topology/core_siblings  |  1 +
 .../cpu/cpu112/topology/core_siblings_list |  1 +
 .../cpu/cpu112/topology/physical_package_id|  1 +
 .../cpu/cpu112/topology/thread_siblings|  1 +
 .../cpu/cpu112/topology/thread_siblings_list   |  1 +
 .../linux-deconfigured-cpus/cpu/cpu113/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu114/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu115/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu116/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu117/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu118/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu119/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu12/online   |  1 +
 .../linux-deconfigured-cpus/cpu/cpu120/online  |  1 +
 .../cpu/cpu120/topology/core_id|  1 +
 .../cpu/cpu120/topology/core_siblings  |  1 +
 .../cpu/cpu120/topology/core_siblings_list |  1 +
 .../cpu/cpu120/topology/physical_package_id|  1 +
 .../cpu/cpu120/topology/thread_siblings|  1 +
 .../cpu/cpu120/topology/thread_siblings_list   |  1 +
 .../linux-deconfigured-cpus/cpu/cpu121/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu122/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu123/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu124/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu125/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu126/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu127/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu128/online  |  1 +
 .../cpu/cpu128/topology/core_id|  1 +
 .../cpu/cpu128/topology/core_siblings  |  1 +
 .../cpu/cpu128/topology/core_siblings_list |  1 +
 .../cpu/cpu128/topology/physical_package_id|  1 +
 .../cpu/cpu128/topology/thread_siblings|  1 +
 .../cpu/cpu128/topology/thread_siblings_list   |  1 +
 .../linux-deconfigured-cpus/cpu/cpu129/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu13/online   |  1 +
 .../linux-deconfigured-cpus/cpu/cpu130/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu131/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu132/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu133/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu134/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu135/online  |  1 +
 .../linux-deconfigured-cpus/cpu/cpu136/online  |  1 +
 .../cpu/cpu136/topology/core_id|  1 +
 .../cpu/cpu136/topology/core_siblings  |  1 +
 .../cpu/cpu136/topology/core_siblings_list |  1 +
 .../cpu/cpu136/topology/physical_package_id|  1 +
 .../cpu/cpu136/topology/thread_siblings|  1 +
 .../cpu/cpu136/topology/thread_siblings_list   |  1 +
 .../linux-deconfigured-cpus/cpu/cpu137/online  |  1 +
 ...

Re: [libvirt] [PATCH 0/9] Add sysfs_prefix to nodeinfo.c API's

[Andrea Bolognani]

On Fri, 2015-07-10 at 17:05 +0200, Andrea Bolognani wrote:
> 
> Patches 1-8 look good to me. Great job splitting the changes in
> such a nice way! I've commented on patch 7 in a separate mail.
> 
> I'll look at patch 9 on Monday.

Patch 9 looks good as well, so ACK series with the previously
mentioned nit fixed.

I've also just sent a patch introducing a test case meant to
make sure patch 9 is actually fixing the issue :)

Hopefully it will make it to the list despite being biggish.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/2] nodeinfo: Formatting changes

[Andrea Bolognani]

---
 src/nodeinfo.c | 36 
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 60dfc8b..c874fa6 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -208,7 +208,7 @@ freebsdNodeGetCPUStats(int cpuNum,
 
 static int
 freebsdNodeGetMemoryStats(virNodeMemoryStatsPtr params,
-   int *nparams)
+  int *nparams)
 {
 size_t i, j = 0;
 unsigned long pagesize = getpagesize() >> 10;
@@ -552,10 +552,11 @@ virNodeParseNode(const char *sysfs_prefix,
 return ret;
 }
 
-int linuxNodeInfoCPUPopulate(const char *sysfs_prefix,
- FILE *cpuinfo,
- virArch arch,
- virNodeInfoPtr nodeinfo)
+int
+linuxNodeInfoCPUPopulate(const char *sysfs_prefix,
+ FILE *cpuinfo,
+ virArch arch,
+ virNodeInfoPtr nodeinfo)
 {
 const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SYSTEM_PATH;
 char line[1024];
@@ -1046,8 +1047,9 @@ virNodeGetSiblingsList(const char *dir, int cpu_id)
 }
 #endif
 
-int nodeGetInfo(const char *sysfs_prefix ATTRIBUTE_UNUSED,
-virNodeInfoPtr nodeinfo)
+int
+nodeGetInfo(const char *sysfs_prefix ATTRIBUTE_UNUSED,
+virNodeInfoPtr nodeinfo)
 {
 virArch hostarch = virArchFromHost();
 
@@ -1123,10 +1125,11 @@ int nodeGetInfo(const char *sysfs_prefix 
ATTRIBUTE_UNUSED,
 #endif
 }
 
-int nodeGetCPUStats(int cpuNum ATTRIBUTE_UNUSED,
-virNodeCPUStatsPtr params ATTRIBUTE_UNUSED,
-int *nparams ATTRIBUTE_UNUSED,
-unsigned int flags)
+int
+nodeGetCPUStats(int cpuNum ATTRIBUTE_UNUSED,
+virNodeCPUStatsPtr params ATTRIBUTE_UNUSED,
+int *nparams ATTRIBUTE_UNUSED,
+unsigned int flags)
 {
 virCheckFlags(0, -1);
 
@@ -1153,11 +1156,12 @@ int nodeGetCPUStats(int cpuNum ATTRIBUTE_UNUSED,
 #endif
 }
 
-int nodeGetMemoryStats(const char *sysfs_prefix ATTRIBUTE_UNUSED,
-   int cellNum ATTRIBUTE_UNUSED,
-   virNodeMemoryStatsPtr params ATTRIBUTE_UNUSED,
-   int *nparams ATTRIBUTE_UNUSED,
-   unsigned int flags)
+int
+nodeGetMemoryStats(const char *sysfs_prefix ATTRIBUTE_UNUSED,
+   int cellNum ATTRIBUTE_UNUSED,
+   virNodeMemoryStatsPtr params ATTRIBUTE_UNUSED,
+   int *nparams ATTRIBUTE_UNUSED,
+   unsigned int flags)
 {
 virCheckFlags(0, -1);
 
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/2] nodeinfo: Various cleanups

[Andrea Bolognani]

The first patch builds on the sysfs_prefix work by John,
while the the second one contains unrelated formatting
changes that increase internal consistency.

Neither introduces behavioral changes.

Andrea Bolognani (2):
  nodeinfo: Make sysfs_prefix usage more consistent
  nodeinfo: Formatting changes

 src/nodeinfo.c   | 73 ++--
 src/nodeinfopriv.h   |  4 +--
 tests/nodeinfotest.c | 14 +-
 3 files changed, 46 insertions(+), 45 deletions(-)

-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/2] nodeinfo: Make sysfs_prefix usage more consistent

[Andrea Bolognani]

Make sure sysfs_prefix, when present, is always the first argument
to a function; don't use a different name to refer to it; check
whether it is NULL, and hence SYSFS_SYSTEM_PATH should be used, only
when using it directly and not just passing it down to another
function; always pass down the same value we've been passed when
calling another function.
---
 src/nodeinfo.c   | 41 +++--
 src/nodeinfopriv.h   |  4 ++--
 tests/nodeinfotest.c | 14 +++---
 3 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 5158680..60dfc8b 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -415,7 +415,6 @@ virNodeParseNode(const char *sysfs_prefix,
  int *threads,
  int *offline)
 {
-const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SYSTEM_PATH;
 int ret = -1;
 int processors = 0;
 DIR *cpudir = NULL;
@@ -441,7 +440,7 @@ virNodeParseNode(const char *sysfs_prefix,
 goto cleanup;
 }
 
-present_cpumap = nodeGetPresentCPUBitmap(prefix);
+present_cpumap = nodeGetPresentCPUBitmap(sysfs_prefix);
 
 /* enumerate sockets in the node */
 CPU_ZERO(&sock_map);
@@ -553,11 +552,12 @@ virNodeParseNode(const char *sysfs_prefix,
 return ret;
 }
 
-int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
- const char *sysfs_dir,
+int linuxNodeInfoCPUPopulate(const char *sysfs_prefix,
+ FILE *cpuinfo,
  virArch arch,
  virNodeInfoPtr nodeinfo)
 {
+const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SYSTEM_PATH;
 char line[1024];
 DIR *nodedir = NULL;
 struct dirent *nodedirent = NULL;
@@ -652,7 +652,7 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
 /* OK, we've parsed clock speed out of /proc/cpuinfo. Get the
  * core, node, socket, thread and topology information from /sys
  */
-if (virAsprintf(&sysfs_nodedir, "%s/node", sysfs_dir) < 0)
+if (virAsprintf(&sysfs_nodedir, "%s/node", prefix) < 0)
 goto cleanup;
 
 if (!(nodedir = opendir(sysfs_nodedir))) {
@@ -667,10 +667,10 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
 nodeinfo->nodes++;
 
 if (virAsprintf(&sysfs_cpudir, "%s/node/%s",
-sysfs_dir, nodedirent->d_name) < 0)
+prefix, nodedirent->d_name) < 0)
 goto cleanup;
 
-if ((cpus = virNodeParseNode(sysfs_dir, sysfs_cpudir, arch,
+if ((cpus = virNodeParseNode(sysfs_prefix, sysfs_cpudir, arch,
  &socks, &cores,
  &threads, &offline)) < 0)
 goto cleanup;
@@ -698,10 +698,10 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
  fallback:
 VIR_FREE(sysfs_cpudir);
 
-if (virAsprintf(&sysfs_cpudir, "%s/cpu", sysfs_dir) < 0)
+if (virAsprintf(&sysfs_cpudir, "%s/cpu", prefix) < 0)
 goto cleanup;
 
-if ((cpus = virNodeParseNode(sysfs_dir, sysfs_cpudir, arch,
+if ((cpus = virNodeParseNode(sysfs_prefix, sysfs_cpudir, arch,
  &socks, &cores,
  &threads, &offline)) < 0)
 goto cleanup;
@@ -1059,7 +1059,6 @@ int nodeGetInfo(const char *sysfs_prefix ATTRIBUTE_UNUSED,
 #ifdef __linux__
 {
 int ret = -1;
-const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SYSTEM_PATH;
 FILE *cpuinfo = fopen(CPUINFO_PATH, "r");
 
 if (!cpuinfo) {
@@ -1068,7 +1067,7 @@ int nodeGetInfo(const char *sysfs_prefix ATTRIBUTE_UNUSED,
 return -1;
 }
 
-ret = linuxNodeInfoCPUPopulate(cpuinfo, prefix,
+ret = linuxNodeInfoCPUPopulate(sysfs_prefix, cpuinfo,
hostarch, nodeinfo);
 if (ret < 0)
 goto cleanup;
@@ -1225,7 +1224,7 @@ nodeGetCPUCount(const char *sysfs_prefix ATTRIBUTE_UNUSED)
 char *cpupath = NULL;
 int ncpu = -1;
 
-if (!(present_path = linuxGetCPUPresentPath(prefix)))
+if (!(present_path = linuxGetCPUPresentPath(sysfs_prefix)))
 return -1;
 
 if (virFileExists(present_path)) {
@@ -1273,13 +1272,12 @@ nodeGetPresentCPUBitmap(const char *sysfs_prefix)
 char *present_path = NULL;
 virBitmapPtr bitmap = NULL;
 #endif
-const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SYSTEM_PATH;
 
-if ((max_present = nodeGetCPUCount(prefix)) < 0)
+if ((max_present = nodeGetCPUCount(sysfs_prefix)) < 0)
 return NULL;
 
 #ifdef __linux__
-if (!(present_path = linuxGetCPUPresentPath(prefix)))
+if (!(present_path = linuxGetCPUPresentPath(sysfs_prefix)))
 return NULL;
 if (virFileExists(present_path))
 bitmap = linuxParseCPUmap(max_present, present_path);
@@ -1301,7 +1299,7 @@ nodeGetCPUBitmap(const char *sysfs_prefix 
ATTRIBUTE_UNUSED,
 virBitmapPtr cpumap;
 int present;
 
-present = nodeGetCPUCount(prefix);
+pres

Re: [libvirt] [PATCH v4 0/2] Fix nodeinfo output on PPC64 KVM hosts

[Andrea Bolognani]

On Tue, 2015-07-14 at 14:56 +0530, Shivaprasad bhat wrote:
> 
> > As you could see in the series I referenced - there are a number of
> > nodeinfo.c API's which don't process the sysfs properly, e.g. they
> > assume /sys/devices/system.
> > 
> > I haven't been fully convinced that the patch which ends up as 
> > patch9 in
> > my series won't have some sort of negative affect somewhere down 
> > the
> > line. Consider if your "*/cpu/present" contained "0-47,64-95" 
> > instead of
> > "0-95" - what "expectations" would you have in this patch series?
> > 
> > The point being if the expectation is that 48-63 would/should have 
> > some
> > specific state and they don't, then I can certainly see the need 
> > for the
> > other patch. Since you had a reason to be in the code, I figured to 
> > pick
> > your brain over this logic while the code was still fresh in your 
> > mind!
> >  It's more a datapoint for the need of the filtering patch.
> > 
> > Once that patch is in place, the call added by that patch to
> > nodeGetPresentCPUBitmap could certainly have altered results if the
> > PPC64 host it was running on didn't have 96 CPU's.
> 
> Thanks a lot for pointing out John. I am planning to test the patch 
> on
> such configuration and see how it goes. I expect, as you mentioned to 
> discard
> the offline cpus 48-63 during counting.

FWIW, now that John's series has been merged I'm going to
rebase my version of the patch on top of it and post if for
review. Shouldn't take long.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] GITHUB readonly mirror site

[Andrea Bolognani]

On Mon, 2015-06-01 at 09:26 +0100, Daniel P. Berrange wrote:
> 
> Thanks, I've successfully push to gitlab & github mirrors now

Both gitlab and github mirrors seem to have stopped working
on July 6, at least for the libvirt repository; Other
repositories such as libvirt-glib seem to be okay.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/3] nodeinfo: Various fixes

[Andrea Bolognani]

This series fixes a bunch of issues currently affecting nodeinfo
and related tests.

Andrea Bolognani (3):
  tests: Restore links in deconfigured-cpus nodeinfo test
  nodeinfo: Add nodeGetPresentCPUBitmap() to libvirt_private.syms
  nodeinfo: Fix nodeGetCPUBitmap()'s fallback code path

 src/libvirt_private.syms   |  1 +
 src/nodeinfo.c |  8 -
 .../cpu/cpu128/topology/core_id|  2 +-
 .../cpu/cpu16/topology/core_id |  2 +-
 .../cpu/cpu24/topology/core_id |  2 +-
 .../cpu/cpu48/topology/core_id |  2 +-
 .../cpu/cpu72/topology/core_id |  2 +-
 .../linux-deconfigured-cpus/node/node0/cpu0|  1 +
 .../linux-deconfigured-cpus/node/node0/cpu1|  1 +
 .../linux-deconfigured-cpus/node/node0/cpu10   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu100  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu101  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu102  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu103  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu11   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu12   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu13   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu14   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu144  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu145  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu146  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu147  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu148  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu149  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu15   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu150  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu151  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu152  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu153  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu154  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu155  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu156  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu157  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu158  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu159  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu16   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu17   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu18   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu19   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu2|  1 +
 .../linux-deconfigured-cpus/node/node0/cpu20   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu21   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu22   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu23   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu24   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu25   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu26   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu27   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu28   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu29   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu3|  1 +
 .../linux-deconfigured-cpus/node/node0/cpu30   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu31   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu32   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu33   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu34   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu35   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu36   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu37   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu38   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu39   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu4|  1 +
 .../linux-deconfigured-cpus/node/node0/cpu5|  1 +
 .../linux-deconfigured-cpus/node/node0/cpu56   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu57   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu58   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu59   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu6|  1 +
 .../linux-deconfigured-cpus/node/node0/cpu60   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu61   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu62   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu63   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu64   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu65   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu66   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu67   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu68   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu69   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu7|  1 +
 .../linux-deconfigured-cpus/node/

[libvirt] [PATCH 3/3] nodeinfo: Fix nodeGetCPUBitmap()'s fallback code path

[Andrea Bolognani]

During the recent refactoring/cleanups, a bug has been introduced
that caused all CPUs to be reported as online unless the sysfs
cpu/present file was available.

This commit fixes the fallback code path by building the directory
path passed to virNodeGetCpuValue() correctly.
---
 src/nodeinfo.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index c874fa6..105d7ab 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -1300,6 +1300,7 @@ nodeGetCPUBitmap(const char *sysfs_prefix 
ATTRIBUTE_UNUSED,
 #ifdef __linux__
 const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SYSTEM_PATH;
 char *online_path = NULL;
+char *cpudir = NULL;
 virBitmapPtr cpumap;
 int present;
 
@@ -1317,8 +1318,12 @@ nodeGetCPUBitmap(const char *sysfs_prefix 
ATTRIBUTE_UNUSED,
 cpumap = virBitmapNew(present);
 if (!cpumap)
 goto cleanup;
+
+if (virAsprintf(&cpudir, "%s/cpu", prefix) < 0)
+goto cleanup;
+
 for (i = 0; i < present; i++) {
-int online = virNodeGetCpuValue(prefix, i, "online", 1);
+int online = virNodeGetCpuValue(cpudir, i, "online", 1);
 if (online < 0) {
 virBitmapFree(cpumap);
 cpumap = NULL;
@@ -1332,6 +1337,7 @@ nodeGetCPUBitmap(const char *sysfs_prefix 
ATTRIBUTE_UNUSED,
 *max_id = present;
  cleanup:
 VIR_FREE(online_path);
+VIR_FREE(cpudir);
 return cpumap;
 #else
 virReportError(VIR_ERR_NO_SUPPORT, "%s",
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/3] nodeinfo: Add nodeGetPresentCPUBitmap() to libvirt_private.syms

[Andrea Bolognani]

---
 src/libvirt_private.syms | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index eb7ec76..b6347de 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1008,6 +1008,7 @@ nodeGetInfo;
 nodeGetMemory;
 nodeGetMemoryParameters;
 nodeGetMemoryStats;
+nodeGetPresentCPUBitmap;
 nodeSetMemoryParameters;
 
 
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 01/10] nodeinfo: Introduce linuxGetCPUGlobalPath()

[Andrea Bolognani]

This is just a more generic version of linuxGetCPUPresentPath(),
which is now implemented by calling the new function appropriately.
---
 src/nodeinfo.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 105d7ab..64b12e6 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -958,16 +958,21 @@ linuxNodeGetMemoryStats(FILE *meminfo,
 }
 
 static char *
-linuxGetCPUPresentPath(const char *sysfs_prefix)
+linuxGetCPUGlobalPath(const char *sysfs_prefix,
+  const char *file)
 {
 const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SYSTEM_PATH;
 char *path = NULL;
 
-if (virAsprintf(&path, "%s/cpu/present", prefix) < 0)
+if (virAsprintf(&path, "%s/cpu/%s", prefix, file) < 0)
 return NULL;
+
 return path;
 }
 
+# define linuxGetCPUPresentPath(sysfs_prefix)\
+linuxGetCPUGlobalPath(sysfs_prefix, "present")
+
 /* Determine the maximum cpu id from a Linux sysfs cpu/present file. */
 static int
 linuxParseCPUmax(const char *path)
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 02/10] nodeinfo: Introduce linuxGetCPUOnlinePath()

[Andrea Bolognani]

---
 src/nodeinfo.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 64b12e6..7a12d54 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -973,6 +973,9 @@ linuxGetCPUGlobalPath(const char *sysfs_prefix,
 # define linuxGetCPUPresentPath(sysfs_prefix)\
 linuxGetCPUGlobalPath(sysfs_prefix, "present")
 
+# define linuxGetCPUOnlinePath(sysfs_prefix) \
+linuxGetCPUGlobalPath(sysfs_prefix, "online")
+
 /* Determine the maximum cpu id from a Linux sysfs cpu/present file. */
 static int
 linuxParseCPUmax(const char *path)
@@ -1313,7 +1316,7 @@ nodeGetCPUBitmap(const char *sysfs_prefix 
ATTRIBUTE_UNUSED,
 if (present < 0)
 return NULL;
 
-if (virAsprintf(&online_path, "%s/cpu/online", prefix) < 0)
+if (!(online_path = linuxGetCPUOnlinePath(sysfs_prefix)))
 return NULL;
 if (virFileExists(online_path)) {
 cpumap = linuxParseCPUmap(present, online_path);
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 08/10] nodeinfo: Use nodeGetOnlineCPUBitmap() when parsing node

[Andrea Bolognani]

No need to look up the online status of each CPU separately when we
can get all the information in one go.
---
 src/nodeinfo.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 61a3b33..eceecc6 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -409,6 +409,7 @@ virNodeParseNode(const char *sysfs_prefix,
 DIR *cpudir = NULL;
 struct dirent *cpudirent = NULL;
 virBitmapPtr present_cpumap = NULL;
+virBitmapPtr online_cpus_map = NULL;
 virBitmapPtr sockets_map = NULL;
 virBitmapPtr *cores_maps = NULL;
 int sock_max = 0;
@@ -417,7 +418,6 @@ virNodeParseNode(const char *sysfs_prefix,
 size_t i;
 int siblings;
 unsigned int cpu;
-int online;
 int direrr;
 
 *threads = 0;
@@ -432,6 +432,9 @@ virNodeParseNode(const char *sysfs_prefix,
 present_cpumap = nodeGetPresentCPUBitmap(sysfs_prefix, NULL);
 if (!present_cpumap)
 goto cleanup;
+online_cpus_map = nodeGetOnlineCPUBitmap(sysfs_prefix, NULL);
+if (!online_cpus_map)
+goto cleanup;
 
 /* enumerate sockets in the node */
 if (!(sockets_map = virBitmapNew(ID_MAX + 1)))
@@ -444,10 +447,7 @@ virNodeParseNode(const char *sysfs_prefix,
 if (!virBitmapIsBitSet(present_cpumap, cpu))
 continue;
 
-if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
-goto cleanup;
-
-if (!online)
+if (!virBitmapIsBitSet(online_cpus_map, cpu))
 continue;
 
 /* Parse socket */
@@ -489,10 +489,7 @@ virNodeParseNode(const char *sysfs_prefix,
 if (!virBitmapIsBitSet(present_cpumap, cpu))
 continue;
 
-if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
-goto cleanup;
-
-if (!online) {
+if (!virBitmapIsBitSet(online_cpus_map, cpu)) {
 (*offline)++;
 continue;
 }
@@ -560,6 +557,7 @@ virNodeParseNode(const char *sysfs_prefix,
 virBitmapFree(cores_maps[i]);
 VIR_FREE(cores_maps);
 virBitmapFree(sockets_map);
+virBitmapFree(online_cpus_map);
 virBitmapFree(present_cpumap);
 
 return ret;
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 03/10] nodeinfo: Rename linuxParseCPUmax() to linuxParseCPUCount()

[Andrea Bolognani]

The original name was confusing because the function returns the number
of CPUs, not the maximum CPU id. The comment above the function has
been updated to reflect this.

No functional changes.
---
 src/nodeinfo.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 7a12d54..52f5594 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -976,9 +976,10 @@ linuxGetCPUGlobalPath(const char *sysfs_prefix,
 # define linuxGetCPUOnlinePath(sysfs_prefix) \
 linuxGetCPUGlobalPath(sysfs_prefix, "online")
 
-/* Determine the maximum cpu id from a Linux sysfs cpu/present file. */
+/* Determine the number of CPUs (maximum CPU id + 1) from a file containing
+ * a list of CPU ids, like the Linux sysfs cpu/present file */
 static int
-linuxParseCPUmax(const char *path)
+linuxParseCPUCount(const char *path)
 {
 char *str = NULL;
 char *tmp;
@@ -1240,7 +1241,7 @@ nodeGetCPUCount(const char *sysfs_prefix ATTRIBUTE_UNUSED)
 return -1;
 
 if (virFileExists(present_path)) {
-ncpu = linuxParseCPUmax(present_path);
+ncpu = linuxParseCPUCount(present_path);
 goto cleanup;
 }
 
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 05/10] nodeinfo: Add out parameter to nodeGetPresentCPUBitmap()

[Andrea Bolognani]

This aligns it with nodeGetCPUBitmap(), which already has a
similar out parameters, and relieves users of this API from the
need to call virBitmapSize() on the returned bitmap.
---
 src/nodeinfo.c   | 8 ++--
 src/nodeinfo.h   | 6 --
 src/util/vircgroup.c | 4 +---
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 5aa0607..7aecc8f 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -440,7 +440,7 @@ virNodeParseNode(const char *sysfs_prefix,
 goto cleanup;
 }
 
-present_cpumap = nodeGetPresentCPUBitmap(sysfs_prefix);
+present_cpumap = nodeGetPresentCPUBitmap(sysfs_prefix, NULL);
 if (!present_cpumap)
 goto cleanup;
 
@@ -1280,7 +1280,8 @@ nodeGetCPUCount(const char *sysfs_prefix ATTRIBUTE_UNUSED)
 }
 
 virBitmapPtr
-nodeGetPresentCPUBitmap(const char *sysfs_prefix ATTRIBUTE_UNUSED)
+nodeGetPresentCPUBitmap(const char *sysfs_prefix ATTRIBUTE_UNUSED,
+int *size ATTRIBUTE_UNUSED)
 {
 #ifdef __linux__
 virBitmapPtr present_cpus = NULL;
@@ -1313,6 +1314,9 @@ nodeGetPresentCPUBitmap(const char *sysfs_prefix 
ATTRIBUTE_UNUSED)
  cleanup:
 VIR_FREE(present_path);
 
+if (present_cpus && size)
+*size = npresent_cpus;
+
 return present_cpus;
 #endif
 virReportError(VIR_ERR_NO_SUPPORT, "%s",
diff --git a/src/nodeinfo.h b/src/nodeinfo.h
index 4f983c2..e83db7b 100644
--- a/src/nodeinfo.h
+++ b/src/nodeinfo.h
@@ -44,8 +44,10 @@ int nodeGetCellsFreeMemory(unsigned long long *freeMems,
 int nodeGetMemory(unsigned long long *mem,
   unsigned long long *freeMem);
 
-virBitmapPtr nodeGetPresentCPUBitmap(const char *sysfs_prefix);
-virBitmapPtr nodeGetCPUBitmap(const char *sysfs_prefix, int *max_id);
+virBitmapPtr nodeGetPresentCPUBitmap(const char *sysfs_prefix,
+ int *size);
+virBitmapPtr nodeGetCPUBitmap(const char *sysfs_prefix,
+  int *size);
 int nodeGetCPUCount(const char *sysfs_prefix);
 
 int nodeGetMemoryParameters(virTypedParameterPtr params,
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 0ef2d29..5011f9c 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -3043,11 +3043,9 @@ virCgroupGetPercpuStats(virCgroupPtr group,
 }
 
 /* To parse account file, we need to know how many cpus are present.  */
-if (!(cpumap = nodeGetPresentCPUBitmap(NULL)))
+if (!(cpumap = nodeGetPresentCPUBitmap(NULL, &total_cpus)))
 return rv;
 
-total_cpus = virBitmapSize(cpumap);
-
 if (ncpus == 0) {
 rv = total_cpus;
 goto cleanup;
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 07/10] nodeinfo: Phase out cpu_set_t usage

[Andrea Bolognani]

Swap out all instances of cpu_set_t and replace them with virBitmap,
which some of the code was already using anyway.

The changes are pretty mechanical, with one notable exception: an
assumption has been added on the max value we can run into while
reading either socket_it or core_id.

While this specific assumption was not in place before, we were
using cpu_set_t improperly by not making sure not to set any bit
past CPU_SETSIZE or explicitly allocating bigger bitmaps; in fact
the default size of a cpu_set_t, 1024, is way too low to run our
testsuite, which includes core_id values in the 2000s.
---
 src/nodeinfo.c | 65 ++
 1 file changed, 38 insertions(+), 27 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 44983b8..61a3b33 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -30,7 +30,6 @@
 #include 
 #include 
 #include 
-#include 
 #include "conf/domain_conf.h"
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
@@ -388,19 +387,6 @@ virNodeParseSocket(const char *dir,
 return ret;
 }
 
-# ifndef CPU_COUNT
-static int
-CPU_COUNT(cpu_set_t *set)
-{
-size_t i, count = 0;
-
-for (i = 0; i < CPU_SETSIZE; i++)
-if (CPU_ISSET(i, set))
-count++;
-return count;
-}
-# endif /* !CPU_COUNT */
-
 /* parses a node entry, returning number of processors in the node and
  * filling arguments */
 static int
@@ -415,15 +401,18 @@ virNodeParseNode(const char *sysfs_prefix,
  int *threads,
  int *offline)
 {
+/* Biggest value we can expect to be used as either socket id
+ * or core id. Bitmaps will need to be sized accordingly */
+const int ID_MAX = 4095;
 int ret = -1;
 int processors = 0;
 DIR *cpudir = NULL;
 struct dirent *cpudirent = NULL;
 virBitmapPtr present_cpumap = NULL;
+virBitmapPtr sockets_map = NULL;
+virBitmapPtr *cores_maps = NULL;
 int sock_max = 0;
-cpu_set_t sock_map;
 int sock;
-cpu_set_t *core_maps = NULL;
 int core;
 size_t i;
 int siblings;
@@ -445,7 +434,9 @@ virNodeParseNode(const char *sysfs_prefix,
 goto cleanup;
 
 /* enumerate sockets in the node */
-CPU_ZERO(&sock_map);
+if (!(sockets_map = virBitmapNew(ID_MAX + 1)))
+goto cleanup;
+
 while ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0) {
 if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
 continue;
@@ -462,7 +453,15 @@ virNodeParseNode(const char *sysfs_prefix,
 /* Parse socket */
 if ((sock = virNodeParseSocket(node, arch, cpu)) < 0)
 goto cleanup;
-CPU_SET(sock, &sock_map);
+if (sock > ID_MAX) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Socket %d can't be handled (max socket is %d)"),
+   sock, ID_MAX);
+goto cleanup;
+}
+
+if (virBitmapSetBit(sockets_map, sock) < 0)
+goto cleanup;
 
 if (sock > sock_max)
 sock_max = sock;
@@ -473,12 +472,13 @@ virNodeParseNode(const char *sysfs_prefix,
 
 sock_max++;
 
-/* allocate cpu maps for each socket */
-if (VIR_ALLOC_N(core_maps, sock_max) < 0)
+/* allocate cores maps for each socket */
+if (VIR_ALLOC_N(cores_maps, sock_max) < 0)
 goto cleanup;
 
 for (i = 0; i < sock_max; i++)
-CPU_ZERO(&core_maps[i]);
+if (!(cores_maps[i] = virBitmapNew(ID_MAX + 1)))
+goto cleanup;
 
 /* iterate over all CPU's in the node */
 rewinddir(cpudir);
@@ -502,7 +502,7 @@ virNodeParseNode(const char *sysfs_prefix,
 /* Parse socket */
 if ((sock = virNodeParseSocket(node, arch, cpu)) < 0)
 goto cleanup;
-if (!CPU_ISSET(sock, &sock_map)) {
+if (!virBitmapIsBitSet(sockets_map, sock)) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("CPU socket topology has changed"));
 goto cleanup;
@@ -515,8 +515,15 @@ virNodeParseNode(const char *sysfs_prefix,
 } else {
 core = virNodeGetCpuValue(node, cpu, "topology/core_id", 0);
 }
+if (core > ID_MAX) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Core %d can't be handled (max core is %d)"),
+   core, ID_MAX);
+goto cleanup;
+}
 
-CPU_SET(core, &core_maps[sock]);
+if (virBitmapSetBit(cores_maps[sock], core) < 0)
+goto cleanup;
 
 if (!(siblings = virNodeCountThreadSiblings(node, cpu)))
 goto cleanup;
@@ -529,13 +536,13 @@ virNodeParseNode(const char *sysfs_prefix,
 goto cleanup;
 
 /* finalize the returned data */
-*sockets = CPU_COUNT(&sock_map);
+*sockets = virBitmapCountBits(sockets_map);
 
 for (i = 0; i < sock_max; i++) {
-if (!CPU_ISSET(i, &sock_map))
+if (!virBitmapIsBitSet(sockets_map, i))

[libvirt] [PATCH 06/10] nodeinfo: Rename nodeGetCPUBitmap() to nodeGetOnlineCPUBitmap()

[Andrea Bolognani]

The new name makes it clear that the returned bitmap contains the
information about which CPUs are online, not eg. which CPUs are
present.

Change the name of the out parameter from max_id, which didn't
reflect the actual value returned, to size. Update the error
message as well.

No functional changes.
---
 src/libvirt_private.syms |  2 +-
 src/nodeinfo.c   | 12 ++--
 src/nodeinfo.h   |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b6347de..7f42e40 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -999,7 +999,6 @@ virLockManagerRelease;
 nodeAllocPages;
 nodeCapsInitNUMA;
 nodeGetCellsFreeMemory;
-nodeGetCPUBitmap;
 nodeGetCPUCount;
 nodeGetCPUMap;
 nodeGetCPUStats;
@@ -1008,6 +1007,7 @@ nodeGetInfo;
 nodeGetMemory;
 nodeGetMemoryParameters;
 nodeGetMemoryStats;
+nodeGetOnlineCPUBitmap;
 nodeGetPresentCPUBitmap;
 nodeSetMemoryParameters;
 
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 7aecc8f..44983b8 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -1325,8 +1325,8 @@ nodeGetPresentCPUBitmap(const char *sysfs_prefix 
ATTRIBUTE_UNUSED,
 }
 
 virBitmapPtr
-nodeGetCPUBitmap(const char *sysfs_prefix ATTRIBUTE_UNUSED,
- int *max_id ATTRIBUTE_UNUSED)
+nodeGetOnlineCPUBitmap(const char *sysfs_prefix ATTRIBUTE_UNUSED,
+   int *size ATTRIBUTE_UNUSED)
 {
 #ifdef __linux__
 const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SYSTEM_PATH;
@@ -1364,15 +1364,15 @@ nodeGetCPUBitmap(const char *sysfs_prefix 
ATTRIBUTE_UNUSED,
 ignore_value(virBitmapSetBit(cpumap, i));
 }
 }
-if (max_id && cpumap)
-*max_id = present;
+if (size && cpumap)
+*size = present;
  cleanup:
 VIR_FREE(online_path);
 VIR_FREE(cpudir);
 return cpumap;
 #else
 virReportError(VIR_ERR_NO_SUPPORT, "%s",
-   _("node cpumap not implemented on this platform"));
+   _("node online CPU map not implemented on this platform"));
 return NULL;
 #endif
 }
@@ -1695,7 +1695,7 @@ nodeGetCPUMap(const char *sysfs_prefix,
 if (!cpumap && !online)
 return nodeGetCPUCount(sysfs_prefix);
 
-if (!(cpus = nodeGetCPUBitmap(sysfs_prefix, &maxpresent)))
+if (!(cpus = nodeGetOnlineCPUBitmap(sysfs_prefix, &maxpresent)))
 goto cleanup;
 
 if (cpumap && virBitmapToData(cpus, cpumap, &dummy) < 0)
diff --git a/src/nodeinfo.h b/src/nodeinfo.h
index e83db7b..f47e330 100644
--- a/src/nodeinfo.h
+++ b/src/nodeinfo.h
@@ -46,8 +46,8 @@ int nodeGetMemory(unsigned long long *mem,
 
 virBitmapPtr nodeGetPresentCPUBitmap(const char *sysfs_prefix,
  int *size);
-virBitmapPtr nodeGetCPUBitmap(const char *sysfs_prefix,
-  int *size);
+virBitmapPtr nodeGetOnlineCPUBitmap(const char *sysfs_prefix,
+int *size);
 int nodeGetCPUCount(const char *sysfs_prefix);
 
 int nodeGetMemoryParameters(virTypedParameterPtr params,
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 09/10] nodeinfo: Use a bitmap to keep track of node CPUs

[Andrea Bolognani]

Keep track of what CPUs belong to the current node while walking
through the sysfs node entry, so we don't need to do it a second
time immediately afterwards.

This also allows us to loop through all CPUs that are part of a
node in guaranteed ascending order, which is something that is
required for some upcoming changes.
---
 src/nodeinfo.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index eceecc6..503fcdd 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -410,8 +410,10 @@ virNodeParseNode(const char *sysfs_prefix,
 struct dirent *cpudirent = NULL;
 virBitmapPtr present_cpumap = NULL;
 virBitmapPtr online_cpus_map = NULL;
+virBitmapPtr node_cpus_map = NULL;
 virBitmapPtr sockets_map = NULL;
 virBitmapPtr *cores_maps = NULL;
+int npresent_cpus;
 int sock_max = 0;
 int sock;
 int core;
@@ -429,13 +431,17 @@ virNodeParseNode(const char *sysfs_prefix,
 goto cleanup;
 }
 
-present_cpumap = nodeGetPresentCPUBitmap(sysfs_prefix, NULL);
+present_cpumap = nodeGetPresentCPUBitmap(sysfs_prefix, &npresent_cpus);
 if (!present_cpumap)
 goto cleanup;
 online_cpus_map = nodeGetOnlineCPUBitmap(sysfs_prefix, NULL);
 if (!online_cpus_map)
 goto cleanup;
 
+/* Keep track of the CPUs that belong to the current node */
+if (!(node_cpus_map = virBitmapNew(npresent_cpus)))
+goto cleanup;
+
 /* enumerate sockets in the node */
 if (!(sockets_map = virBitmapNew(ID_MAX + 1)))
 goto cleanup;
@@ -447,6 +453,10 @@ virNodeParseNode(const char *sysfs_prefix,
 if (!virBitmapIsBitSet(present_cpumap, cpu))
 continue;
 
+/* Mark this CPU as part of the current node */
+if (virBitmapSetBit(node_cpus_map, cpu) < 0)
+goto cleanup;
+
 if (!virBitmapIsBitSet(online_cpus_map, cpu))
 continue;
 
@@ -480,13 +490,11 @@ virNodeParseNode(const char *sysfs_prefix,
 if (!(cores_maps[i] = virBitmapNew(ID_MAX + 1)))
 goto cleanup;
 
-/* iterate over all CPU's in the node */
-rewinddir(cpudir);
-while ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0) {
-if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
-continue;
+/* Iterate over all CPUs in the node, in ascending order */
+for (cpu = 0; cpu < npresent_cpus; cpu++) {
 
-if (!virBitmapIsBitSet(present_cpumap, cpu))
+/* Skip CPUs that are not part of the current node */
+if (!virBitmapIsBitSet(node_cpus_map, cpu))
 continue;
 
 if (!virBitmapIsBitSet(online_cpus_map, cpu)) {
@@ -529,9 +537,6 @@ virNodeParseNode(const char *sysfs_prefix,
 *threads = siblings;
 }
 
-if (direrr < 0)
-goto cleanup;
-
 /* finalize the returned data */
 *sockets = virBitmapCountBits(sockets_map);
 
@@ -557,6 +562,7 @@ virNodeParseNode(const char *sysfs_prefix,
 virBitmapFree(cores_maps[i]);
 VIR_FREE(cores_maps);
 virBitmapFree(sockets_map);
+virBitmapFree(node_cpus_map);
 virBitmapFree(online_cpus_map);
 virBitmapFree(present_cpumap);
 
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 04/10] nodeinfo: Add old kernel compatibility to nodeGetPresentCPUBitmap()

[Andrea Bolognani]

If the cpu/present file is not available, we assume that the kernel
is too old to support non-consecutive CPU ids and return a bitmap
with all the bits set to represent this fact. This assumption is
already exploited in nodeGetCPUCount().

This means users of this API can expect the information to always
be available unless an error has occurred, and no longer need to
treat the NULL return value as a special case.

The error message has been updated as well.
---
 src/nodeinfo.c | 46 --
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 52f5594..5aa0607 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -441,6 +441,8 @@ virNodeParseNode(const char *sysfs_prefix,
 }
 
 present_cpumap = nodeGetPresentCPUBitmap(sysfs_prefix);
+if (!present_cpumap)
+goto cleanup;
 
 /* enumerate sockets in the node */
 CPU_ZERO(&sock_map);
@@ -448,7 +450,7 @@ virNodeParseNode(const char *sysfs_prefix,
 if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
 continue;
 
-if (present_cpumap && !(virBitmapIsBitSet(present_cpumap, cpu)))
+if (!virBitmapIsBitSet(present_cpumap, cpu))
 continue;
 
 if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
@@ -484,7 +486,7 @@ virNodeParseNode(const char *sysfs_prefix,
 if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
 continue;
 
-if (present_cpumap && !(virBitmapIsBitSet(present_cpumap, cpu)))
+if (!virBitmapIsBitSet(present_cpumap, cpu))
 continue;
 
 if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
@@ -1278,27 +1280,43 @@ nodeGetCPUCount(const char *sysfs_prefix 
ATTRIBUTE_UNUSED)
 }
 
 virBitmapPtr
-nodeGetPresentCPUBitmap(const char *sysfs_prefix)
+nodeGetPresentCPUBitmap(const char *sysfs_prefix ATTRIBUTE_UNUSED)
 {
-int max_present;
 #ifdef __linux__
+virBitmapPtr present_cpus = NULL;
 char *present_path = NULL;
-virBitmapPtr bitmap = NULL;
-#endif
+int npresent_cpus;
+int cpu;
 
-if ((max_present = nodeGetCPUCount(sysfs_prefix)) < 0)
-return NULL;
+if ((npresent_cpus = nodeGetCPUCount(sysfs_prefix)) < 0)
+goto cleanup;
 
-#ifdef __linux__
 if (!(present_path = linuxGetCPUPresentPath(sysfs_prefix)))
-return NULL;
-if (virFileExists(present_path))
-bitmap = linuxParseCPUmap(max_present, present_path);
+goto cleanup;
+
+/* If the cpu/present file is available, parse it and exit */
+if (virFileExists(present_path)) {
+present_cpus = linuxParseCPUmap(npresent_cpus, present_path);
+goto cleanup;
+}
+
+/* If the file is not available, we can assume that the kernel is
+ * too old to support non-consecutive CPU ids and just mark all
+ * possible CPUs as present */
+if (!(present_cpus = virBitmapNew(npresent_cpus)))
+goto cleanup;
+
+for (cpu = 0; cpu < npresent_cpus; cpu++)
+if (virBitmapSetBit(present_cpus, cpu) < 0)
+goto cleanup;
+
+ cleanup:
 VIR_FREE(present_path);
-return bitmap;
+
+return present_cpus;
 #endif
 virReportError(VIR_ERR_NO_SUPPORT, "%s",
-   _("non-continuous host cpu numbers not implemented on this 
platform"));
+   _("node present CPU map not implemented on this platform"));
 return NULL;
 }
 
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v5 2/5] tests: Prepare for subcore tests

[Andrea Bolognani]

From: Shivaprasad G Bhat 

The nodeGetThreadsPerSubcore() function is mocked to return 8 for
ppc64 tests, which corresponds to the default subcore mode.

Update the expected output for the deconfigured-cpus nodeinfo
test to account for this change.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Andrea Bolognani 
---
 tests/Makefile.am  |  6 
 .../linux-ppc64-deconfigured-cpus.expected |  2 +-
 tests/nodeinfomock.c   | 35 ++
 tests/nodeinfotest.c   |  2 +-
 4 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100644 tests/nodeinfomock.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index b202195..bde7f5b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -412,6 +412,7 @@ test_libraries = libshunload.la \
vircgroupmock.la \
virpcimock.la \
virnetdevmock.la \
+   nodeinfomock.la \
$(NULL)
 if WITH_QEMU
 test_libraries += libqemumonitortestutils.la \
@@ -1048,6 +1049,11 @@ virpcimock_la_LIBADD = $(GNULIB_LIBS) \
   ../src/libvirt.la
 virpcimock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
 
+nodeinfomock_la_SOURCES = \
+   nodeinfomock.c
+nodeinfomock_la_CFLAGS = $(AM_CFLAGS)
+nodeinfomock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
+
 virnetdevtest_SOURCES = \
virnetdevtest.c testutils.h testutils.c
 virnetdevtest_CFLAGS = $(AM_CFLAGS) $(LIBNL_CFLAGS)
diff --git a/tests/nodeinfodata/linux-ppc64-deconfigured-cpus.expected 
b/tests/nodeinfodata/linux-ppc64-deconfigured-cpus.expected
index 304f423..113bfa8 100644
--- a/tests/nodeinfodata/linux-ppc64-deconfigured-cpus.expected
+++ b/tests/nodeinfodata/linux-ppc64-deconfigured-cpus.expected
@@ -1 +1 @@
-CPUs: 10/80, MHz: 3690, Nodes: 1, Sockets: 1, Cores: 80, Threads: 1
+CPUs: 80/80, MHz: 3690, Nodes: 1, Sockets: 1, Cores: 80, Threads: 1
diff --git a/tests/nodeinfomock.c b/tests/nodeinfomock.c
new file mode 100644
index 000..b9c0152
--- /dev/null
+++ b/tests/nodeinfomock.c
@@ -0,0 +1,35 @@
+/*
+ * 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/>.
+ *
+ */
+
+#include 
+
+#include "internal.h"
+#include "nodeinfo.h"
+
+int
+nodeGetThreadsPerSubcore(virArch arch)
+{
+int threads_per_subcore = 0;
+
+// Emulate SMT=8 on POWER hardware
+if (ARCH_IS_PPC64(arch))
+threads_per_subcore = 8;
+
+return threads_per_subcore;
+}
diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c
index 60467bc..1c026ec 100644
--- a/tests/nodeinfotest.c
+++ b/tests/nodeinfotest.c
@@ -256,6 +256,6 @@ mymain(void)
 return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
-VIRT_TEST_MAIN(mymain)
+VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/nodeinfomock.so")
 
 #endif /* __linux__ */
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v5 1/5] nodeinfo: Fix output on PPC64 KVM hosts

[Andrea Bolognani]

From: Shivaprasad G Bhat 

The nodeinfo is reporting incorrect number of cpus and incorrect host
topology on PPC64 KVM hosts. The KVM hypervisor on PPC64 needs only
the primary thread in a core to be online, and the secondaries offlined.
While scheduling a guest in, the kvm scheduler wakes up the secondaries to
run in guest context.

The host scheduling of the guests happen at the core level(as only primary
thread is online). The kvm scheduler exploits as many threads of the core
as needed by guest. Further, starting POWER8, the processor allows splitting
a physical core into multiple subcores with 2 or 4 threads each. Again, only
the primary thread in a subcore is online in the host. The KVM-PPC
scheduler allows guests to exploit all the offline threads in the subcore,
by bringing them online when needed.
(Kernel patches on split-core 
http://www.spinics.net/lists/kvm-ppc/msg09121.html)

Recently with dynamic micro-threading changes in ppc-kvm, makes sure
to utilize all the offline cpus across guests, and across guests with
different cpu topologies.
(https://www.mail-archive.com/kvm@vger.kernel.org/msg115978.html)

Since the offline cpus are brought online in the guest context, it is safe
to count them as online. Nodeinfo today discounts these offline cpus from
cpu count/topology calclulation, and the nodeinfo output is not of any help
and the host appears overcommited when it is actually not.

The patch carefully counts those offline threads whose primary threads are
online. The host topology displayed by the nodeinfo is also fixed when the
host is in valid kvm state.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Andrea Bolognani 
---
 src/libvirt_private.syms |   1 +
 src/nodeinfo.c   | 142 +--
 src/nodeinfo.h   |   1 +
 3 files changed, 140 insertions(+), 4 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7f42e40..46e535e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1009,6 +1009,7 @@ nodeGetMemoryParameters;
 nodeGetMemoryStats;
 nodeGetOnlineCPUBitmap;
 nodeGetPresentCPUBitmap;
+nodeGetThreadsPerSubcore;
 nodeSetMemoryParameters;
 
 
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 0f72a25..03c2025 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -31,6 +31,12 @@
 #include 
 #include 
 #include "conf/domain_conf.h"
+#include 
+#include 
+
+#if HAVE_LINUX_KVM_H
+# include 
+#endif
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
 # include 
@@ -391,14 +397,15 @@ virNodeParseSocket(const char *dir,
  * filling arguments */
 static int
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3)
-ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6)
-ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(8)
-ATTRIBUTE_NONNULL(9)
+ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(7)
+ATTRIBUTE_NONNULL(8) ATTRIBUTE_NONNULL(9)
+ATTRIBUTE_NONNULL(10)
 virNodeParseNode(const char *node,
  virArch arch,
  virBitmapPtr present_cpus_map,
  int npresent_cpus,
  virBitmapPtr online_cpus_map,
+ int threads_per_subcore,
  int *sockets,
  int *cores,
  int *threads,
@@ -491,7 +498,18 @@ virNodeParseNode(const char *node,
 continue;
 
 if (!virBitmapIsBitSet(online_cpus_map, cpu)) {
-(*offline)++;
+if (threads_per_subcore > 0 &&
+cpu % threads_per_subcore != 0 &&
+virBitmapIsBitSet(online_cpus_map,
+  cpu - (cpu % threads_per_subcore))) {
+/* Secondary offline threads are counted as online when
+ * subcores are in use and the corresponding primary
+ * thread is online */
+processors++;
+} else {
+/* But they are counted as offline otherwise */
+(*offline)++;
+}
 continue;
 }
 
@@ -542,6 +560,12 @@ virNodeParseNode(const char *node,
 *cores = core;
 }
 
+if (threads_per_subcore > 0) {
+/* The thread count ignores offline threads, which means that only
+ * only primary threads have been considered so far. If subcores
+ * are in use, we need to also account for secondary threads */
+*threads *= threads_per_subcore;
+}
 ret = processors;
 
  cleanup:
@@ -560,6 +584,46 @@ virNodeParseNode(const char *node,
 return ret;
 }
 
+/* Check whether the host subcore configuration is valid.
+ *
+ * A valid configuration is one where no secondary thread is online;
+ * the primary thread in a subcore is always the first one */
+static bool
+nodeHasValidSubcoreConfiguration(const char *sysfs_prefix,
+ int threads_per_subcore)
+{
+virBitmapPtr online_cpus = NULL;
+int nonline_cpus;
+int cpu;
+bool ret = false;
+
+/* No point in checking if su

[libvirt] [PATCH 1/3] tests: Restore links in deconfigured-cpus nodeinfo test

[Andrea Bolognani]

When cleaning up the data (taken from a running system) for inclusion
I went a little too far and deleted a bunch of links that should have
been left alone. The test worked despite this because it was going
through a fallback code path.

A few other files are affected as well: again, the data is taken from
a running system, so even thought we would probably be okay if we
just added the links, aligning everything is definitely safer.
---
 .../cpu/cpu128/topology/core_id|  2 +-
 .../cpu/cpu16/topology/core_id |  2 +-
 .../cpu/cpu24/topology/core_id |  2 +-
 .../cpu/cpu48/topology/core_id |  2 +-
 .../cpu/cpu72/topology/core_id |  2 +-
 .../linux-deconfigured-cpus/node/node0/cpu0|  1 +
 .../linux-deconfigured-cpus/node/node0/cpu1|  1 +
 .../linux-deconfigured-cpus/node/node0/cpu10   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu100  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu101  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu102  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu103  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu11   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu12   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu13   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu14   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu144  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu145  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu146  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu147  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu148  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu149  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu15   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu150  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu151  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu152  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu153  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu154  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu155  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu156  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu157  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu158  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu159  |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu16   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu17   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu18   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu19   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu2|  1 +
 .../linux-deconfigured-cpus/node/node0/cpu20   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu21   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu22   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu23   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu24   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu25   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu26   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu27   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu28   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu29   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu3|  1 +
 .../linux-deconfigured-cpus/node/node0/cpu30   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu31   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu32   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu33   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu34   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu35   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu36   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu37   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu38   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu39   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu4|  1 +
 .../linux-deconfigured-cpus/node/node0/cpu5|  1 +
 .../linux-deconfigured-cpus/node/node0/cpu56   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu57   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu58   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu59   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu6|  1 +
 .../linux-deconfigured-cpus/node/node0/cpu60   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu61   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu62   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu63   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu64   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu65   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu66   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu67   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu68   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu69   |  1 +
 .../linux-deconfigured-cpus/node/node0/cpu7|  1 +
 .../linux

[libvirt] [PATCH 00/10] nodeinfo: Various cleanups

[Andrea Bolognani]

Note: this series is to be applied on top of the

  [PATCH 00/03] nodeinfo: Various fixes

series I've posted at the same time.

A bunch of improvements and cleanups that make the nodeinfo
code a bit nicer, more streamlined and less redundant, hopefully
not just to my eyes.

Andrea Bolognani (10):
  nodeinfo: Introduce linuxGetCPUGlobalPath()
  nodeinfo: Introduce linuxGetCPUOnlinePath()
  nodeinfo: Rename linuxParseCPUmax() to linuxParseCPUCount()
  nodeinfo: Add old kernel compatibility to nodeGetPresentCPUBitmap()
  nodeinfo: Add out parameter to nodeGetPresentCPUBitmap()
  nodeinfo: Rename nodeGetCPUBitmap() to nodeGetOnlineCPUBitmap()
  nodeinfo: Phase out cpu_set_t usage
  nodeinfo: Use nodeGetOnlineCPUBitmap() when parsing node
  nodeinfo: Use a bitmap to keep track of node CPUs
  nodeinfo: Calculate present and online CPUs only once

 src/libvirt_private.syms |   2 +-
 src/nodeinfo.c   | 211 +--
 src/nodeinfo.h   |   6 +-
 src/util/vircgroup.c |   4 +-
 4 files changed, 139 insertions(+), 84 deletions(-)

-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v4 0/2] Fix nodeinfo output on PPC64 KVM hosts

[Andrea Bolognani]

On Tue, 2015-07-14 at 11:47 +0200, Andrea Bolognani wrote:
> 
> FWIW, now that John's series has been merged I'm going to
> rebase my version of the patch on top of it and post if for
> review. Shouldn't take long.

Well, it took way more than I hoped it would, but that's
what happens when you dive into the rabbit's hole.

I've split my work into three series: the first one
contains fixes, the second one cleanups, and the third one
is v5 of the subcore series. Hopefully the split will make
it easier to review and make sense of it all.

John, I've CCed you on all three not because I expect you
to review everything[1] but because you've worked on this
area quite a bit lately so you might want to keep an eye
on my changes, which are quite significant.

Cheers.


[1] Not that I would complain if you decided to ;)
-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/2] tests: Add nodeinfo test data utility scripts

[Andrea Bolognani]

Both scripts can be useful when adding new test cases to the
nodeinfo test.

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 | 170 +++
 tests/nodeinfodata/display.sh| 101 +
 2 files changed, 271 insertions(+)
 create mode 100755 tests/nodeinfodata/copy-from-host.sh
 create mode 100755 tests/nodeinfodata/display.sh

-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/2] tests: Add script to display nodeinfo test data

[Andrea Bolognani]

---
 tests/nodeinfodata/display.sh | 101 ++
 1 file changed, 101 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 000..bc260e2
--- /dev/null
+++ b/tests/nodeinfodata/display.sh
@@ -0,0 +1,101 @@
+#!/bin/sh
+
+# 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
+# .
+
+die() {
+typeset message=$1
+
+echo "$message" >&2
+
+exit 1
+}
+
+list_cpus() {
+typeset datadir=$1
+
+ls -d "$datadir/cpu/cpu"* | while read o; do
+o=${o#*cpu/cpu}
+case "$o" in
+[0-9]*)
+echo "$o"
+;;
+esac
+done | sort -un
+}
+
+is_online() {
+typeset datadir=$1
+typeset cpu=$2
+
+typeset ret=1
+
+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() {
+typeset datadir=$1
+typeset threads_per_core=$2
+
+if ! test "$#" -eq "2"; then
+die "Usage: $SELF DATADIR THREADS_PER_CORE"
+fi
+if ! test -d "$datadir"; then
+die "$datadir: No such directory"
+fi
+
+echo "Threads per core: $threads_per_core"
+
+echo -n 'Present CPUs: '
+if test -e "$datadir/cpu/present"; then
+cat "$datadir/cpu/present"
+else
+echo 'information not available'
+fi
+
+for cpu in $(list_cpus "$datadir"); do
+
+mod=$(expr "$cpu" % "$threads_per_core")
+if test "$mod" = "0"; then
+echo
+fi
+
+if is_online "$datadir" "$cpu"; then
+printf " %3d*" "$cpu"
+else
+printf " %3d " "$cpu"
+fi
+done
+echo
+
+return 0
+}
+
+SELF=$0
+main "$@"
+exit $?
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/2] tests: Add script to copy nodeinfo test data from host

[Andrea Bolognani]

Files we don't need, including symbolic links that might result
problematic to make dist, are removed from the copied data.
---
 tests/nodeinfodata/copy-from-host.sh | 170 +++
 1 file changed, 170 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 000..b5c5e8e
--- /dev/null
+++ b/tests/nodeinfodata/copy-from-host.sh
@@ -0,0 +1,170 @@
+#!/bin/sh
+
+# 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
+# .
+
+SYSFS_PATH="/sys/devices/system"
+KEEP_LINKS="cpu[0-9]*"
+KEEP_DIRECTORIES="cpu
+  cpu[0-9]*
+  node
+  node[0-9]*
+  topology"
+KEEP_FILES="core_id
+core_siblings*
+kernel_max
+meminfo
+offline
+online
+physical_package_id
+possible
+present
+thread_siblings*"
+
+die() {
+typeset message=$1
+
+echo "$message" >&2
+
+exit 1
+}
+
+keep() {
+typeset tag=$1
+typeset path=$2
+
+echo "  $tag $path"
+
+return 0
+}
+
+delete() {
+typeset tag=$1
+typeset path=$2
+
+rm -rf "$path"
+
+return $?
+}
+
+is_valid_name() {
+typeset name=$1
+typeset ret=0
+
+case "$name" in
+*/*)
+# We don't want to have to deal with subdirectories, so
+# names containing slashes are rejected
+ret=1
+;;
+esac
+
+return $ret
+}
+
+matches_any_pattern() {
+typeset name=$1
+typeset patterns=$2
+typeset ret=1
+
+for pattern in $patterns; do
+case "$name" in
+$pattern)
+ret=0
+;;
+esac
+done
+
+return $ret
+}
+
+main() {
+typeset name=$1
+
+if ! test "$name"; then
+die "Usage: $SELF NAME"
+fi
+
+if ! is_valid_name "$name"; then
+die "Name '$name' is not valid"
+fi
+
+# Create directory
+if test -e "$name"; then
+die "$name: File or directory already exists"
+fi
+
+if ! mkdir "$name" >/dev/null 2>&1; then
+die "$name: Unable to create directory"
+fi
+
+echo "Copying host data..."
+
+# Copy data from host. Errors are ignored because some files we don't
+# care about always give input/output error when read
+cp -r "$SYSFS_PATH/cpu" "$name/" >/dev/null 2>&1
+cp -r "$SYSFS_PATH/node" "$name/" >/dev/null 2>&1
+
+if ! test -d "$name/cpu" || ! test -d "$name/node"; then
+die "Error while copying host data"
+fi
+
+echo "Cleaning up data..."
+
+# Delete symbolic links
+find "$name" -type l | while read l; do
+b=${l##*/}
+if matches_any_pattern "$b" "$KEEP_LINKS"; then
+keep "l" "$l"
+else
+delete "l" "$l"
+fi
+done
+
+# Delete directories
+find "$name" -type d | while read d; do
+b=${d##*/}
+# We don't want to delete the directory we just created :)
+if test "$b" = "$name"; then
+continue
+fi
+if matches_any_pattern "$b" "$KEEP_DIRECTORIES"; then
+keep "d" "$d"
+else
+delete "d" "$d"
+fi
+done
+
+# Delete files
+find "$name" -type f | while read f; do
+b=${f##*/}
+if matches_any_pattern "$b" "$KEEP_FILES"; then
+keep "f" "$f"
+else
+delete "f" "$f"
+fi
+done
+
+echo "All done"
+
+return 0
+}
+
+SELF=$0
+main "$@"
+exit $?
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 02/10] nodeinfo: Introduce linuxGetCPUOnlinePath()

[Andrea Bolognani]

On Mon, 2015-07-20 at 11:34 +0200, Peter Krempa wrote:
> 
> > @@ -973,6 +973,9 @@ linuxGetCPUGlobalPath(const char *sysfs_prefix,
> >  # define linuxGetCPUPresentPath(sysfs_prefix)\
> >  linuxGetCPUGlobalPath(sysfs_prefix, "present")
> >  
> > +# define linuxGetCPUOnlinePath(sysfs_prefix) \
> > +linuxGetCPUGlobalPath(sysfs_prefix, "online")
> 
> Either add a function ...
> 
> > +
> >  /* Determine the maximum cpu id from a Linux sysfs cpu/present 
> > file. */
> >  static int
> >  linuxParseCPUmax(const char *path)
> > @@ -1313,7 +1316,7 @@ nodeGetCPUBitmap(const char *sysfs_prefix 
> > ATTRIBUTE_UNUSED,
> >  if (present < 0)
> >  return NULL;
> >  
> > -if (virAsprintf(&online_path, "%s/cpu/online", prefix) < 0)
> > +if (!(online_path = linuxGetCPUOnlinePath(sysfs_prefix)))
> 
> Or use the global helper here directly.

I'll send v2 converting both this and linuxGetCPUPresentPath()
to functions once you're done reviewing the series :)

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/4] cpu: Rename {powerpc, ppc} => ppc64 (internal symbols)

[Andrea Bolognani]

Update the names of the symbols used internally by the driver.

No functional changes.
---
 src/cpu/cpu_ppc64.c | 250 ++--
 1 file changed, 125 insertions(+), 125 deletions(-)

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index bec4bf8..7a48903 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -40,26 +40,26 @@ VIR_LOG_INIT("cpu.cpu_ppc64");
 
 static const virArch archs[] = { VIR_ARCH_PPC64, VIR_ARCH_PPC64LE };
 
-struct ppc_vendor {
+struct ppc64_vendor {
 char *name;
-struct ppc_vendor *next;
+struct ppc64_vendor *next;
 };
 
-struct ppc_model {
+struct ppc64_model {
 char *name;
-const struct ppc_vendor *vendor;
+const struct ppc64_vendor *vendor;
 struct cpuPPC64Data data;
-struct ppc_model *next;
+struct ppc64_model *next;
 };
 
-struct ppc_map {
-struct ppc_vendor *vendors;
-struct ppc_model *models;
+struct ppc64_map {
+struct ppc64_vendor *vendors;
+struct ppc64_model *models;
 };
 
 
 static void
-ppcModelFree(struct ppc_model *model)
+ppc64ModelFree(struct ppc64_model *model)
 {
 if (model == NULL)
 return;
@@ -68,11 +68,11 @@ ppcModelFree(struct ppc_model *model)
 VIR_FREE(model);
 }
 
-static struct ppc_model *
-ppcModelFind(const struct ppc_map *map,
- const char *name)
+static struct ppc64_model *
+ppc64ModelFind(const struct ppc64_map *map,
+   const char *name)
 {
-struct ppc_model *model;
+struct ppc64_model *model;
 
 model = map->models;
 while (model != NULL) {
@@ -85,11 +85,11 @@ ppcModelFind(const struct ppc_map *map,
 return NULL;
 }
 
-static struct ppc_model *
-ppcModelFindPVR(const struct ppc_map *map,
-uint32_t pvr)
+static struct ppc64_model *
+ppc64ModelFindPVR(const struct ppc64_map *map,
+  uint32_t pvr)
 {
-struct ppc_model *model;
+struct ppc64_model *model;
 
 model = map->models;
 while (model != NULL) {
@@ -105,19 +105,19 @@ ppcModelFindPVR(const struct ppc_map *map,
  * If the exact CPU isn't found, return the nearest matching CPU generation
  */
 if (pvr & 0xul)
-return ppcModelFindPVR(map, (pvr & 0xul));
+return ppc64ModelFindPVR(map, (pvr & 0xul));
 
 return NULL;
 }
 
-static struct ppc_model *
-ppcModelCopy(const struct ppc_model *model)
+static struct ppc64_model *
+ppc64ModelCopy(const struct ppc64_model *model)
 {
-struct ppc_model *copy;
+struct ppc64_model *copy;
 
 if (VIR_ALLOC(copy) < 0 ||
 VIR_STRDUP(copy->name, model->name) < 0) {
-ppcModelFree(copy);
+ppc64ModelFree(copy);
 return NULL;
 }
 
@@ -127,11 +127,11 @@ ppcModelCopy(const struct ppc_model *model)
 return copy;
 }
 
-static struct ppc_vendor *
-ppcVendorFind(const struct ppc_map *map,
-  const char *name)
+static struct ppc64_vendor *
+ppc64VendorFind(const struct ppc64_map *map,
+const char *name)
 {
-struct ppc_vendor *vendor;
+struct ppc64_vendor *vendor;
 
 vendor = map->vendors;
 while (vendor) {
@@ -145,7 +145,7 @@ ppcVendorFind(const struct ppc_map *map,
 }
 
 static void
-ppcVendorFree(struct ppc_vendor *vendor)
+ppc64VendorFree(struct ppc64_vendor *vendor)
 {
 if (!vendor)
 return;
@@ -154,34 +154,34 @@ ppcVendorFree(struct ppc_vendor *vendor)
 VIR_FREE(vendor);
 }
 
-static struct ppc_model *
-ppcModelFromCPU(const virCPUDef *cpu,
-const struct ppc_map *map)
+static struct ppc64_model *
+ppc64ModelFromCPU(const virCPUDef *cpu,
+  const struct ppc64_map *map)
 {
-struct ppc_model *model = NULL;
+struct ppc64_model *model = NULL;
 
-if ((model = ppcModelFind(map, cpu->model)) == NULL) {
+if ((model = ppc64ModelFind(map, cpu->model)) == NULL) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown CPU model %s"), cpu->model);
 goto error;
 }
 
-if ((model = ppcModelCopy(model)) == NULL)
+if ((model = ppc64ModelCopy(model)) == NULL)
 goto error;
 
 return model;
 
  error:
-ppcModelFree(model);
+ppc64ModelFree(model);
 return NULL;
 }
 
 
 static int
-ppcVendorLoad(xmlXPathContextPtr ctxt,
-  struct ppc_map *map)
+ppc64VendorLoad(xmlXPathContextPtr ctxt,
+struct ppc64_map *map)
 {
-struct ppc_vendor *vendor = NULL;
+struct ppc64_vendor *vendor = NULL;
 
 if (VIR_ALLOC(vendor) < 0)
 return -1;
@@ -193,7 +193,7 @@ ppcVendorLoad(xmlXPathContextPtr ctxt,
 goto ignore;
 }
 
-if (ppcVendorFind(map, vendor->name)) {
+if (ppc64VendorFind(map, vendor->name)) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("CPU vendor %s already defined"), vendor->name);
 goto ignore;
@@ -210,15 +210,15 @@ ppcVendorLoad(xmlXPathContextPtr ctxt,
 return 0;
 
  ignore:
-ppcVendorFree(vendor);
+p

[libvirt] [PATCH 4/4] cpu: Indentation changes in the ppc64 driver

[Andrea Bolognani]

---
 src/cpu/cpu_ppc64.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 7a48903..c3a51fb 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -340,7 +340,8 @@ ppc64LoadMap(void)
 }
 
 static virCPUDataPtr
-ppc64MakeCPUData(virArch arch, struct cpuPPC64Data *data)
+ppc64MakeCPUData(virArch arch,
+ struct cpuPPC64Data *data)
 {
 virCPUDataPtr cpuData;
 
@@ -695,9 +696,9 @@ ppc64GetModels(char ***models)
 }
 
 struct cpuArchDriver cpuDriverPPC64 = {
-.name = "ppc64",
-.arch = archs,
-.narch = ARRAY_CARDINALITY(archs),
+.name   = "ppc64",
+.arch   = archs,
+.narch  = ARRAY_CARDINALITY(archs),
 .compare= ppc64Compare,
 .decode = ppc64Decode,
 .encode = NULL,
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/4] cpu: Rename {powerpc, ppc} => ppc64 (exported symbols)

[Andrea Bolognani]

Only the symbols exported by the driver have been updated;
the driver implementation itself still uses the old names
internally.

No functional changes.
---
 src/cpu/cpu.c|  2 +-
 src/cpu/cpu.h|  2 +-
 src/cpu/cpu_ppc64.c  | 18 +-
 src/cpu/cpu_ppc64.h  | 10 +-
 src/cpu/cpu_ppc64_data.h | 10 +-
 5 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 2e34f81..731df26 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -44,7 +44,7 @@ VIR_LOG_INIT("cpu.cpu");
 
 static struct cpuArchDriver *drivers[] = {
 &cpuDriverX86,
-&cpuDriverPowerPC,
+&cpuDriverPPC64,
 &cpuDriverS390,
 &cpuDriverArm,
 &cpuDriverAARCH64,
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index c0371cd..49d4226 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -38,7 +38,7 @@ struct _virCPUData {
 virArch arch;
 union {
 virCPUx86Data *x86;
-struct cpuPPCData ppc;
+struct cpuPPC64Data ppc64;
 /* generic driver needs no data */
 } data;
 };
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 7866bdd..bec4bf8 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -1,5 +1,5 @@
 /*
- * cpu_ppc64.c: CPU driver for PowerPC CPUs
+ * cpu_ppc64.c: CPU driver for 64-bit PowerPC CPUs
  *
  * Copyright (C) 2013 Red Hat, Inc.
  * Copyright (C) IBM Corporation, 2010
@@ -36,7 +36,7 @@
 
 #define VIR_FROM_THIS VIR_FROM_CPU
 
-VIR_LOG_INIT("cpu.cpu_powerpc");
+VIR_LOG_INIT("cpu.cpu_ppc64");
 
 static const virArch archs[] = { VIR_ARCH_PPC64, VIR_ARCH_PPC64LE };
 
@@ -48,7 +48,7 @@ struct ppc_vendor {
 struct ppc_model {
 char *name;
 const struct ppc_vendor *vendor;
-struct cpuPPCData data;
+struct cpuPPC64Data data;
 struct ppc_model *next;
 };
 
@@ -340,7 +340,7 @@ ppcLoadMap(void)
 }
 
 static virCPUDataPtr
-ppcMakeCPUData(virArch arch, struct cpuPPCData *data)
+ppcMakeCPUData(virArch arch, struct cpuPPC64Data *data)
 {
 virCPUDataPtr cpuData;
 
@@ -348,7 +348,7 @@ ppcMakeCPUData(virArch arch, struct cpuPPCData *data)
 return NULL;
 
 cpuData->arch = arch;
-cpuData->data.ppc = *data;
+cpuData->data.ppc64 = *data;
 data = NULL;
 
 return cpuData;
@@ -480,10 +480,10 @@ ppcDecode(virCPUDefPtr cpu,
 if (data == NULL || (map = ppcLoadMap()) == NULL)
 return -1;
 
-if (!(model = ppcModelFindPVR(map, data->data.ppc.pvr))) {
+if (!(model = ppcModelFindPVR(map, data->data.ppc64.pvr))) {
 virReportError(VIR_ERR_OPERATION_FAILED,
_("Cannot find CPU model with PVR 0x%08x"),
-   data->data.ppc.pvr);
+   data->data.ppc64.pvr);
 goto cleanup;
 }
 
@@ -529,7 +529,7 @@ ppcNodeData(virArch arch)
 
 #if defined(__powerpc__) || defined(__powerpc64__)
 asm("mfpvr %0"
-: "=r" (cpuData->data.ppc.pvr));
+: "=r" (cpuData->data.ppc64.pvr));
 #endif
 
 return cpuData;
@@ -694,7 +694,7 @@ ppcGetModels(char ***models)
 goto cleanup;
 }
 
-struct cpuArchDriver cpuDriverPowerPC = {
+struct cpuArchDriver cpuDriverPPC64 = {
 .name = "ppc64",
 .arch = archs,
 .narch = ARRAY_CARDINALITY(archs),
diff --git a/src/cpu/cpu_ppc64.h b/src/cpu/cpu_ppc64.h
index e9ef2be..a6c9659 100644
--- a/src/cpu/cpu_ppc64.h
+++ b/src/cpu/cpu_ppc64.h
@@ -1,5 +1,5 @@
 /*
- * cpu_ppc64.h: CPU driver for PowerPC CPUs
+ * cpu_ppc64.h: CPU driver for 64-bit PowerPC CPUs
  *
  * Copyright (C) Copyright (C) IBM Corporation, 2010
  *
@@ -22,11 +22,11 @@
  *  Prerna Saxena 
  */
 
-#ifndef __VIR_CPU_POWERPC_H__
-# define __VIR_CPU_POWERPC_H__
+#ifndef __VIR_CPU_PPC64_H__
+# define __VIR_CPU_PPC64_H__
 
 # include "cpu.h"
 
-extern struct cpuArchDriver cpuDriverPowerPC;
+extern struct cpuArchDriver cpuDriverPPC64;
 
-#endif /* __VIR_CPU_POWERPC_H__ */
+#endif /* __VIR_CPU_PPC64_H__ */
diff --git a/src/cpu/cpu_ppc64_data.h b/src/cpu/cpu_ppc64_data.h
index a70b099..45152de 100644
--- a/src/cpu/cpu_ppc64_data.h
+++ b/src/cpu/cpu_ppc64_data.h
@@ -1,5 +1,5 @@
 /*
- * cpu_ppc64_data.h: PowerPC specific CPU data
+ * cpu_ppc64_data.h: 64-bit PowerPC CPU specific data
  *
  * Copyright (C) 2012 IBM Corporation.
  *
@@ -21,13 +21,13 @@
  *  Li Zhang 
  */
 
-#ifndef __VIR_CPU_PPC_DATA_H__
-# define __VIR_CPU_PPC_DATA_H__
+#ifndef __VIR_CPU_PPC64_DATA_H__
+# define __VIR_CPU_PPC64_DATA_H__
 
 # include 
 
-struct cpuPPCData {
+struct cpuPPC64Data {
 uint32_t pvr;
 };
 
-#endif /* __VIR_CPU_PPC_DATA_H__ */
+#endif /* __VIR_CPU_PPC64_DATA_H__ */
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/4] cpu: Rename {powerpc,ppc} => ppc64

[Andrea Bolognani]

The naming of files and symbols belonging to the ppc64 CPU
driver was all over the place: this series brings
inner-peace-inducing consistency to that corner of libvirt
via a series of straightforward string replacements.

More substantial changes coming next.

Andrea Bolognani (4):
  cpu: Rename {powerpc,ppc} => ppc64 (filesystem)
  cpu: Rename {powerpc,ppc} => ppc64 (exported symbols)
  cpu: Rename {powerpc,ppc} => ppc64 (internal symbols)
  cpu: Indentation changes in the ppc64 driver

 po/POTFILES.in   |   2 +-
 src/Makefile.am  |   5 +-
 src/cpu/cpu.c|   4 +-
 src/cpu/cpu.h|   4 +-
 src/cpu/cpu_powerpc.c| 711 --
 src/cpu/cpu_powerpc.h|  32 ---
 src/cpu/cpu_ppc64.c  | 712 +++
 src/cpu/cpu_ppc64.h  |  32 +++
 src/cpu/cpu_ppc64_data.h |  33 +++
 src/cpu/cpu_ppc_data.h   |  33 ---
 10 files changed, 785 insertions(+), 783 deletions(-)
 delete mode 100644 src/cpu/cpu_powerpc.c
 delete mode 100644 src/cpu/cpu_powerpc.h
 create mode 100644 src/cpu/cpu_ppc64.c
 create mode 100644 src/cpu/cpu_ppc64.h
 create mode 100644 src/cpu/cpu_ppc64_data.h
 delete mode 100644 src/cpu/cpu_ppc_data.h

-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/4] cpu: Rename {powerpc, ppc} => ppc64 (filesystem)

[Andrea Bolognani]

The driver only supports VIR_ARCH_PPC64 and VIR_ARCH_PPC64LE.

Just shuffling files around and updating the build system
accordingly. No functional changes.
---
 po/POTFILES.in   |   2 +-
 src/Makefile.am  |   5 +-
 src/cpu/cpu.c|   2 +-
 src/cpu/cpu.h|   2 +-
 src/cpu/cpu_powerpc.c| 711 ---
 src/cpu/cpu_powerpc.h|  32 ---
 src/cpu/cpu_ppc64.c  | 711 +++
 src/cpu/cpu_ppc64.h  |  32 +++
 src/cpu/cpu_ppc64_data.h |  33 +++
 src/cpu/cpu_ppc_data.h   |  33 ---
 10 files changed, 782 insertions(+), 781 deletions(-)
 delete mode 100644 src/cpu/cpu_powerpc.c
 delete mode 100644 src/cpu/cpu_powerpc.h
 create mode 100644 src/cpu/cpu_ppc64.c
 create mode 100644 src/cpu/cpu_ppc64.h
 create mode 100644 src/cpu/cpu_ppc64_data.h
 delete mode 100644 src/cpu/cpu_ppc_data.h

diff --git a/po/POTFILES.in b/po/POTFILES.in
index a75f5ae..c58a7c1 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -39,7 +39,7 @@ src/conf/virchrdev.c
 src/cpu/cpu.c
 src/cpu/cpu_generic.c
 src/cpu/cpu_map.c
-src/cpu/cpu_powerpc.c
+src/cpu/cpu_ppc64.c
 src/cpu/cpu_x86.c
 src/driver.c
 src/esx/esx_driver.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 7338ab9..c4d49a5 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1008,8 +1008,9 @@ CPU_SOURCES = 
\
cpu/cpu_s390.h cpu/cpu_s390.c   \
cpu/cpu_arm.h cpu/cpu_arm.c \
cpu/cpu_aarch64.h cpu/cpu_aarch64.c \
-   cpu/cpu_map.h cpu/cpu_map.c cpu/cpu_powerpc.h   \
-   cpu/cpu_powerpc.c cpu/cpu_ppc_data.h
+   cpu/cpu_ppc64.h cpu/cpu_ppc64.c \
+   cpu/cpu_ppc64_data.h\
+   cpu/cpu_map.h cpu/cpu_map.c
 
 VMX_SOURCES =  \
vmx/vmx.c vmx/vmx.h
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 9e67ddd..2e34f81 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -29,7 +29,7 @@
 #include "cpu.h"
 #include "cpu_map.h"
 #include "cpu_x86.h"
-#include "cpu_powerpc.h"
+#include "cpu_ppc64.h"
 #include "cpu_s390.h"
 #include "cpu_arm.h"
 #include "cpu_aarch64.h"
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 09e9538..c0371cd 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -29,7 +29,7 @@
 # include "virarch.h"
 # include "conf/cpu_conf.h"
 # include "cpu_x86_data.h"
-# include "cpu_ppc_data.h"
+# include "cpu_ppc64_data.h"
 
 
 typedef struct _virCPUData virCPUData;
diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
deleted file mode 100644
index 733a0cd..000
--- a/src/cpu/cpu_powerpc.c
+++ /dev/null
@@ -1,711 +0,0 @@
-/*
- * cpu_powerpc.c: CPU driver for PowerPC CPUs
- *
- * Copyright (C) 2013 Red Hat, Inc.
- * Copyright (C) IBM Corporation, 2010
- *
- * 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
- * .
- *
- * Authors:
- *  Anton Blanchard 
- *  Prerna Saxena 
- *  Li Zhang 
- */
-
-#include 
-#include 
-
-#include "virlog.h"
-#include "viralloc.h"
-#include "cpu.h"
-#include "virstring.h"
-#include "cpu_map.h"
-#include "virbuffer.h"
-
-#define VIR_FROM_THIS VIR_FROM_CPU
-
-VIR_LOG_INIT("cpu.cpu_powerpc");
-
-static const virArch archs[] = { VIR_ARCH_PPC64, VIR_ARCH_PPC64LE };
-
-struct ppc_vendor {
-char *name;
-struct ppc_vendor *next;
-};
-
-struct ppc_model {
-char *name;
-const struct ppc_vendor *vendor;
-struct cpuPPCData data;
-struct ppc_model *next;
-};
-
-struct ppc_map {
-struct ppc_vendor *vendors;
-struct ppc_model *models;
-};
-
-
-static void
-ppcModelFree(struct ppc_model *model)
-{
-if (model == NULL)
-return;
-
-VIR_FREE(model->name);
-VIR_FREE(model);
-}
-
-static struct ppc_model *
-ppcModelFind(const struct ppc_map *map,
- const char *name)
-{
-struct ppc_model *model;
-
-model = map->models;
-while (model != NULL) {
-if (STREQ(model->name, name))
-return model;
-
-model = model->next;
-}
-
-return NULL;
-}
-
-static struct ppc_model *
-ppcModelFindPVR(const struct ppc_map *map,
-uint32_t pvr)
-{
-struct ppc_model *model;
-
-model = map->models;
-while (mo

[libvirt] [PATCH 10/10] nodeinfo: Calculate present and online CPUs only once

[Andrea Bolognani]

Move the calls to the respective functions from virNodeParseNode(),
which is executed once for every NUMA node, to
linuxNodeInfoCPUPopulate(), which is executed just once per host.
---
 src/nodeinfo.c | 49 +
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 503fcdd..0f72a25 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -390,12 +390,15 @@ virNodeParseSocket(const char *dir,
 /* parses a node entry, returning number of processors in the node and
  * filling arguments */
 static int
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
-ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5)
-ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(7)
-virNodeParseNode(const char *sysfs_prefix,
- const char *node,
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3)
+ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6)
+ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(8)
+ATTRIBUTE_NONNULL(9)
+virNodeParseNode(const char *node,
  virArch arch,
+ virBitmapPtr present_cpus_map,
+ int npresent_cpus,
+ virBitmapPtr online_cpus_map,
  int *sockets,
  int *cores,
  int *threads,
@@ -408,12 +411,9 @@ virNodeParseNode(const char *sysfs_prefix,
 int processors = 0;
 DIR *cpudir = NULL;
 struct dirent *cpudirent = NULL;
-virBitmapPtr present_cpumap = NULL;
-virBitmapPtr online_cpus_map = NULL;
 virBitmapPtr node_cpus_map = NULL;
 virBitmapPtr sockets_map = NULL;
 virBitmapPtr *cores_maps = NULL;
-int npresent_cpus;
 int sock_max = 0;
 int sock;
 int core;
@@ -431,13 +431,6 @@ virNodeParseNode(const char *sysfs_prefix,
 goto cleanup;
 }
 
-present_cpumap = nodeGetPresentCPUBitmap(sysfs_prefix, &npresent_cpus);
-if (!present_cpumap)
-goto cleanup;
-online_cpus_map = nodeGetOnlineCPUBitmap(sysfs_prefix, NULL);
-if (!online_cpus_map)
-goto cleanup;
-
 /* Keep track of the CPUs that belong to the current node */
 if (!(node_cpus_map = virBitmapNew(npresent_cpus)))
 goto cleanup;
@@ -450,7 +443,7 @@ virNodeParseNode(const char *sysfs_prefix,
 if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
 continue;
 
-if (!virBitmapIsBitSet(present_cpumap, cpu))
+if (!virBitmapIsBitSet(present_cpus_map, cpu))
 continue;
 
 /* Mark this CPU as part of the current node */
@@ -563,8 +556,6 @@ virNodeParseNode(const char *sysfs_prefix,
 VIR_FREE(cores_maps);
 virBitmapFree(sockets_map);
 virBitmapFree(node_cpus_map);
-virBitmapFree(online_cpus_map);
-virBitmapFree(present_cpumap);
 
 return ret;
 }
@@ -576,10 +567,13 @@ linuxNodeInfoCPUPopulate(const char *sysfs_prefix,
  virNodeInfoPtr nodeinfo)
 {
 const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SYSTEM_PATH;
+virBitmapPtr present_cpus_map = NULL;
+virBitmapPtr online_cpus_map = NULL;
 char line[1024];
 DIR *nodedir = NULL;
 struct dirent *nodedirent = NULL;
 int cpus, cores, socks, threads, offline = 0;
+int npresent_cpus;
 unsigned int node;
 int ret = -1;
 char *sysfs_nodedir = NULL;
@@ -667,6 +661,15 @@ linuxNodeInfoCPUPopulate(const char *sysfs_prefix,
 }
 }
 
+/* Get information about what CPUs are present in the host and what
+ * CPUs are online, so that we don't have to so for each node */
+present_cpus_map = nodeGetPresentCPUBitmap(sysfs_prefix, &npresent_cpus);
+if (!present_cpus_map)
+goto cleanup;
+online_cpus_map = nodeGetOnlineCPUBitmap(sysfs_prefix, NULL);
+if (!online_cpus_map)
+goto cleanup;
+
 /* OK, we've parsed clock speed out of /proc/cpuinfo. Get the
  * core, node, socket, thread and topology information from /sys
  */
@@ -688,7 +691,9 @@ linuxNodeInfoCPUPopulate(const char *sysfs_prefix,
 prefix, nodedirent->d_name) < 0)
 goto cleanup;
 
-if ((cpus = virNodeParseNode(sysfs_prefix, sysfs_cpudir, arch,
+if ((cpus = virNodeParseNode(sysfs_cpudir, arch,
+ present_cpus_map, npresent_cpus,
+ online_cpus_map,
  &socks, &cores,
  &threads, &offline)) < 0)
 goto cleanup;
@@ -719,7 +724,9 @@ linuxNodeInfoCPUPopulate(const char *sysfs_prefix,
 if (virAsprintf(&sysfs_cpudir, "%s/cpu", prefix) < 0)
 goto cleanup;
 
-if ((cpus = virNodeParseNode(sysfs_prefix, sysfs_cpudir, arch,
+if ((cpus = virNodeParseNode(sysfs_cpudir, arch,
+ present_cpus_map, npresent_cpus,
+ online_cpus_map,
  &socks, &cores,
  &threads, &offline)) < 0)
 goto cleanup;
@

Re: [libvirt] [PATCH 00/10] nodeinfo: Various cleanups

[Andrea Bolognani]

On Mon, 2015-07-20 at 15:48 +0200, Peter Krempa wrote:
> 
> > Andrea Bolognani (10):
> >   nodeinfo: Introduce linuxGetCPUGlobalPath()
> >   nodeinfo: Introduce linuxGetCPUOnlinePath()
> >   nodeinfo: Rename linuxParseCPUmax() to linuxParseCPUCount()
> >   nodeinfo: Add old kernel compatibility to 
> > nodeGetPresentCPUBitmap()
> >   nodeinfo: Add out parameter to nodeGetPresentCPUBitmap()
> >   nodeinfo: Rename nodeGetCPUBitmap() to nodeGetOnlineCPUBitmap()
> >   nodeinfo: Phase out cpu_set_t usage
> >   nodeinfo: Use nodeGetOnlineCPUBitmap() when parsing node
> >   nodeinfo: Use a bitmap to keep track of node CPUs
> >   nodeinfo: Calculate present and online CPUs only once
> 
> Patch 10/10 is missing in the list.

Yeah, sorry about that. It's in now.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 05/10] nodeinfo: Add out parameter to nodeGetPresentCPUBitmap()

[Andrea Bolognani]

On Mon, 2015-07-20 at 15:15 +0200, Peter Krempa wrote:
> 
> > -virBitmapPtr nodeGetPresentCPUBitmap(const char *sysfs_prefix);
> > -virBitmapPtr nodeGetCPUBitmap(const char *sysfs_prefix, int 
> > *max_id);
> > +virBitmapPtr nodeGetPresentCPUBitmap(const char *sysfs_prefix,
> > + int *size);
> > +virBitmapPtr nodeGetCPUBitmap(const char *sysfs_prefix,
> > +  int *size);
> 
> I'd prefer something like "ncpus" or maxcpu rather than size. For
> getting size virBitmapSize() is totally apropriate.

I've used "size" on purpose, because I didn't want people to
mistake that for a count of online or present CPUs: it's the
size of the returned bitmap, same value you'd get if you
called virBitmapSize() on it.

Sounds reasonable?

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 05/10] nodeinfo: Add out parameter to nodeGetPresentCPUBitmap()

[Andrea Bolognani]

On Mon, 2015-07-20 at 16:18 +0200, Peter Krempa wrote:
> On Mon, Jul 20, 2015 at 16:07:42 +0200, Andrea Bolognani wrote:
> > On Mon, 2015-07-20 at 15:15 +0200, Peter Krempa wrote:
> > > 
> > > > -virBitmapPtr nodeGetPresentCPUBitmap(const char 
> > > > *sysfs_prefix);
> > > > -virBitmapPtr nodeGetCPUBitmap(const char *sysfs_prefix, int 
> > > > *max_id);
> > > > +virBitmapPtr nodeGetPresentCPUBitmap(const char *sysfs_prefix,
> > > > + int *size);
> > > > +virBitmapPtr nodeGetCPUBitmap(const char *sysfs_prefix,
> > > > +  int *size);
> > > 
> > > I'd prefer something like "ncpus" or maxcpu rather than size. For
> > > getting size virBitmapSize() is totally apropriate.
> > 
> > I've used "size" on purpose, because I didn't want people to
> > mistake that for a count of online or present CPUs: it's the
> > size of the returned bitmap, same value you'd get if you
> > called virBitmapSize() on it.
> 
> I thin the 'max_id' or perhaps 'max_cpu_id' were better. Otherwise 
> I'd
> stay with calling virBitmapSize. It doesn't then look like it's 
> adding
> any value on top of calling virBitmapSize directly and could actually 
> be
> optimized out.

Using "max_id" is wrong though, because the returned value is
the size of the bitmap: if you have 4 CPUs, it will return 4,
not 3 as the name "max_id" would suggest. 

Since virBitmapSize() does very little work anyway, I vote
for getting rid of the out parameter altogether.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 07/10] nodeinfo: Phase out cpu_set_t usage

[Andrea Bolognani]

On Mon, 2015-07-20 at 15:41 +0200, Peter Krempa wrote:
> 
> > +/* Biggest value we can expect to be used as either socket id
> > + * or core id. Bitmaps will need to be sized accordingly */
> > +const int ID_MAX = 4095;
> 
> I think this should be a more global setting. We have quite a few 
> places
> where we invent arbitrary maximum cpu counts. One of them is
> virProcessSetAffinity.

Definitely agreed. We should define such limits in a single
place and stick to them.

> Otherwise looks good to me, but I'd really want to avoid multiple
> definitions of the same maximum variable.

I've left the code unchanged in v2 because this looks like
a task that would require quite a bit of research, and I'd
prefer if that didn't block an otherwise ACKed series which
in turn is a requirement of another series I've posted.

So I'm going to look into it and remove duplicate
definitions in a follow-up patch, if you're okay with that.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 09/10] nodeinfo: Use a bitmap to keep track of node CPUs

[Andrea Bolognani]

On Mon, 2015-07-20 at 15:47 +0200, Peter Krempa wrote:
> 
> > +/* Iterate over all CPUs in the node, in ascending order */
> > +for (cpu = 0; cpu < npresent_cpus; cpu++) {
> >  
> > -if (!virBitmapIsBitSet(present_cpumap, cpu))
> > +/* Skip CPUs that are not part of the current node */
> > +if (!virBitmapIsBitSet(node_cpus_map, cpu))
> 
> Perhaps you can use virBitmapNextSetBit to simplify the iteration.
> 
> >  continue;
> >  
> >  if (!virBitmapIsBitSet(online_cpus_map, cpu)) {
> 
> ACK with or without the iteration modified.

I tried following your suggestion but I didn't like the
result very much, so I've left the loop as-is in v2.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 01/10] nodeinfo: Introduce linuxGetCPUGlobalPath()

[Andrea Bolognani]

This is just a more generic version of linuxGetCPUPresentPath(),
which is now implemented by calling the new function appropriately.
---
 src/nodeinfo.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 105d7ab..b09a4fd 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -958,16 +958,24 @@ linuxNodeGetMemoryStats(FILE *meminfo,
 }
 
 static char *
-linuxGetCPUPresentPath(const char *sysfs_prefix)
+linuxGetCPUGlobalPath(const char *sysfs_prefix,
+  const char *file)
 {
 const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SYSTEM_PATH;
 char *path = NULL;
 
-if (virAsprintf(&path, "%s/cpu/present", prefix) < 0)
+if (virAsprintf(&path, "%s/cpu/%s", prefix, file) < 0)
 return NULL;
+
 return path;
 }
 
+static char *
+linuxGetCPUPresentPath(const char *sysfs_prefix)
+{
+return linuxGetCPUGlobalPath(sysfs_prefix, "present");
+}
+
 /* Determine the maximum cpu id from a Linux sysfs cpu/present file. */
 static int
 linuxParseCPUmax(const char *path)
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 03/10] nodeinfo: Rename linuxParseCPUmax() to linuxParseCPUCount()

[Andrea Bolognani]

The original name was confusing because the function returns the number
of CPUs, not the maximum CPU id. The comment above the function has
been updated to reflect this.

No behavioral changes.
---
 src/nodeinfo.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 5459cc6..a7a5d98 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -982,9 +982,10 @@ linuxGetCPUOnlinePath(const char *sysfs_prefix)
 return linuxGetCPUGlobalPath(sysfs_prefix, "online");
 }
 
-/* Determine the maximum cpu id from a Linux sysfs cpu/present file. */
+/* Determine the number of CPUs (maximum CPU id + 1) from a file containing
+ * a list of CPU ids, like the Linux sysfs cpu/present file */
 static int
-linuxParseCPUmax(const char *path)
+linuxParseCPUCount(const char *path)
 {
 char *str = NULL;
 char *tmp;
@@ -1246,7 +1247,7 @@ nodeGetCPUCount(const char *sysfs_prefix ATTRIBUTE_UNUSED)
 return -1;
 
 if (virFileExists(present_path)) {
-ncpu = linuxParseCPUmax(present_path);
+ncpu = linuxParseCPUCount(present_path);
 goto cleanup;
 }
 
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 10/10] nodeinfo: Calculate present and online CPUs only once

[Andrea Bolognani]

Move the calls to the respective functions from virNodeParseNode(),
which is executed once for every NUMA node, to
linuxNodeInfoCPUPopulate(), which is executed just once per host.
---
 src/nodeinfo.c | 53 +++--
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 2328a86..31095a4 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -390,12 +390,15 @@ virNodeParseSocket(const char *dir,
 /* parses a node entry, returning number of processors in the node and
  * filling arguments */
 static int
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
-ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5)
-ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(7)
-virNodeParseNode(const char *sysfs_prefix,
- const char *node,
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3)
+ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6)
+ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(8)
+ATTRIBUTE_NONNULL(9)
+virNodeParseNode(const char *node,
  virArch arch,
+ virBitmapPtr present_cpus_map,
+ int npresent_cpus,
+ virBitmapPtr online_cpus_map,
  int *sockets,
  int *cores,
  int *threads,
@@ -408,12 +411,9 @@ virNodeParseNode(const char *sysfs_prefix,
 int processors = 0;
 DIR *cpudir = NULL;
 struct dirent *cpudirent = NULL;
-virBitmapPtr present_cpumap = NULL;
-virBitmapPtr online_cpus_map = NULL;
 virBitmapPtr node_cpus_map = NULL;
 virBitmapPtr sockets_map = NULL;
 virBitmapPtr *cores_maps = NULL;
-int npresent_cpus;
 int sock_max = 0;
 int sock;
 int core;
@@ -431,15 +431,6 @@ virNodeParseNode(const char *sysfs_prefix,
 goto cleanup;
 }
 
-present_cpumap = nodeGetPresentCPUBitmap(sysfs_prefix);
-if (!present_cpumap)
-goto cleanup;
-online_cpus_map = nodeGetOnlineCPUBitmap(sysfs_prefix);
-if (!online_cpus_map)
-goto cleanup;
-
-npresent_cpus = virBitmapSize(present_cpumap);
-
 /* Keep track of the CPUs that belong to the current node */
 if (!(node_cpus_map = virBitmapNew(npresent_cpus)))
 goto cleanup;
@@ -452,7 +443,7 @@ virNodeParseNode(const char *sysfs_prefix,
 if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
 continue;
 
-if (!virBitmapIsBitSet(present_cpumap, cpu))
+if (!virBitmapIsBitSet(present_cpus_map, cpu))
 continue;
 
 /* Mark this CPU as part of the current node */
@@ -565,8 +556,6 @@ virNodeParseNode(const char *sysfs_prefix,
 VIR_FREE(cores_maps);
 virBitmapFree(sockets_map);
 virBitmapFree(node_cpus_map);
-virBitmapFree(online_cpus_map);
-virBitmapFree(present_cpumap);
 
 return ret;
 }
@@ -578,10 +567,13 @@ linuxNodeInfoCPUPopulate(const char *sysfs_prefix,
  virNodeInfoPtr nodeinfo)
 {
 const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SYSTEM_PATH;
+virBitmapPtr present_cpus_map = NULL;
+virBitmapPtr online_cpus_map = NULL;
 char line[1024];
 DIR *nodedir = NULL;
 struct dirent *nodedirent = NULL;
 int cpus, cores, socks, threads, offline = 0;
+int npresent_cpus;
 unsigned int node;
 int ret = -1;
 char *sysfs_nodedir = NULL;
@@ -669,6 +661,17 @@ linuxNodeInfoCPUPopulate(const char *sysfs_prefix,
 }
 }
 
+/* Get information about what CPUs are present in the host and what
+ * CPUs are online, so that we don't have to so for each node */
+present_cpus_map = nodeGetPresentCPUBitmap(sysfs_prefix);
+if (!present_cpus_map)
+goto cleanup;
+online_cpus_map = nodeGetOnlineCPUBitmap(sysfs_prefix);
+if (!online_cpus_map)
+goto cleanup;
+
+npresent_cpus = virBitmapSize(present_cpus_map);
+
 /* OK, we've parsed clock speed out of /proc/cpuinfo. Get the
  * core, node, socket, thread and topology information from /sys
  */
@@ -690,7 +693,9 @@ linuxNodeInfoCPUPopulate(const char *sysfs_prefix,
 prefix, nodedirent->d_name) < 0)
 goto cleanup;
 
-if ((cpus = virNodeParseNode(sysfs_prefix, sysfs_cpudir, arch,
+if ((cpus = virNodeParseNode(sysfs_cpudir, arch,
+ present_cpus_map, npresent_cpus,
+ online_cpus_map,
  &socks, &cores,
  &threads, &offline)) < 0)
 goto cleanup;
@@ -721,7 +726,9 @@ linuxNodeInfoCPUPopulate(const char *sysfs_prefix,
 if (virAsprintf(&sysfs_cpudir, "%s/cpu", prefix) < 0)
 goto cleanup;
 
-if ((cpus = virNodeParseNode(sysfs_prefix, sysfs_cpudir, arch,
+if ((cpus = virNodeParseNode(sysfs_cpudir, arch,
+ present_cpus_map, npresent_cpus,
+ online_cpus_map,
  &socks, &cores,
  

[libvirt] [PATCH v2 08/10] nodeinfo: Use nodeGetOnlineCPUBitmap() when parsing node

[Andrea Bolognani]

No need to look up the online status of each CPU separately when we
can get all the information in one go.
---
 src/nodeinfo.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 7d0e6b0..c97dece 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -409,6 +409,7 @@ virNodeParseNode(const char *sysfs_prefix,
 DIR *cpudir = NULL;
 struct dirent *cpudirent = NULL;
 virBitmapPtr present_cpumap = NULL;
+virBitmapPtr online_cpus_map = NULL;
 virBitmapPtr sockets_map = NULL;
 virBitmapPtr *cores_maps = NULL;
 int sock_max = 0;
@@ -417,7 +418,6 @@ virNodeParseNode(const char *sysfs_prefix,
 size_t i;
 int siblings;
 unsigned int cpu;
-int online;
 int direrr;
 
 *threads = 0;
@@ -432,6 +432,9 @@ virNodeParseNode(const char *sysfs_prefix,
 present_cpumap = nodeGetPresentCPUBitmap(sysfs_prefix);
 if (!present_cpumap)
 goto cleanup;
+online_cpus_map = nodeGetOnlineCPUBitmap(sysfs_prefix, NULL);
+if (!online_cpus_map)
+goto cleanup;
 
 /* enumerate sockets in the node */
 if (!(sockets_map = virBitmapNew(ID_MAX + 1)))
@@ -444,10 +447,7 @@ virNodeParseNode(const char *sysfs_prefix,
 if (!virBitmapIsBitSet(present_cpumap, cpu))
 continue;
 
-if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
-goto cleanup;
-
-if (!online)
+if (!virBitmapIsBitSet(online_cpus_map, cpu))
 continue;
 
 /* Parse socket */
@@ -489,10 +489,7 @@ virNodeParseNode(const char *sysfs_prefix,
 if (!virBitmapIsBitSet(present_cpumap, cpu))
 continue;
 
-if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
-goto cleanup;
-
-if (!online) {
+if (!virBitmapIsBitSet(online_cpus_map, cpu)) {
 (*offline)++;
 continue;
 }
@@ -560,6 +557,7 @@ virNodeParseNode(const char *sysfs_prefix,
 virBitmapFree(cores_maps[i]);
 VIR_FREE(cores_maps);
 virBitmapFree(sockets_map);
+virBitmapFree(online_cpus_map);
 virBitmapFree(present_cpumap);
 
 return ret;
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 02/10] nodeinfo: Introduce linuxGetCPUOnlinePath()

[Andrea Bolognani]

---
 src/nodeinfo.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index b09a4fd..5459cc6 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -976,6 +976,12 @@ linuxGetCPUPresentPath(const char *sysfs_prefix)
 return linuxGetCPUGlobalPath(sysfs_prefix, "present");
 }
 
+static char *
+linuxGetCPUOnlinePath(const char *sysfs_prefix)
+{
+return linuxGetCPUGlobalPath(sysfs_prefix, "online");
+}
+
 /* Determine the maximum cpu id from a Linux sysfs cpu/present file. */
 static int
 linuxParseCPUmax(const char *path)
@@ -1316,7 +1322,7 @@ nodeGetCPUBitmap(const char *sysfs_prefix 
ATTRIBUTE_UNUSED,
 if (present < 0)
 return NULL;
 
-if (virAsprintf(&online_path, "%s/cpu/online", prefix) < 0)
+if (!(online_path = linuxGetCPUOnlinePath(sysfs_prefix)))
 return NULL;
 if (virFileExists(online_path)) {
 cpumap = linuxParseCPUmap(present, online_path);
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 04/10] nodeinfo: Add old kernel compatibility to nodeGetPresentCPUBitmap()

[Andrea Bolognani]

If the cpu/present file is not available, we assume that the kernel
is too old to support non-consecutive CPU ids and return a bitmap
with all the bits set to represent this fact. This assumption is
already exploited in nodeGetCPUCount().

This means users of this API can expect the information to always
be available unless an error has occurred, and no longer need to
treat the NULL return value as a special case.

The error message has been updated as well.
---
 src/nodeinfo.c | 43 +--
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index a7a5d98..ceb517a 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -441,6 +441,8 @@ virNodeParseNode(const char *sysfs_prefix,
 }
 
 present_cpumap = nodeGetPresentCPUBitmap(sysfs_prefix);
+if (!present_cpumap)
+goto cleanup;
 
 /* enumerate sockets in the node */
 CPU_ZERO(&sock_map);
@@ -448,7 +450,7 @@ virNodeParseNode(const char *sysfs_prefix,
 if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
 continue;
 
-if (present_cpumap && !(virBitmapIsBitSet(present_cpumap, cpu)))
+if (!virBitmapIsBitSet(present_cpumap, cpu))
 continue;
 
 if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
@@ -484,7 +486,7 @@ virNodeParseNode(const char *sysfs_prefix,
 if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
 continue;
 
-if (present_cpumap && !(virBitmapIsBitSet(present_cpumap, cpu)))
+if (!virBitmapIsBitSet(present_cpumap, cpu))
 continue;
 
 if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
@@ -1284,27 +1286,40 @@ nodeGetCPUCount(const char *sysfs_prefix 
ATTRIBUTE_UNUSED)
 }
 
 virBitmapPtr
-nodeGetPresentCPUBitmap(const char *sysfs_prefix)
+nodeGetPresentCPUBitmap(const char *sysfs_prefix ATTRIBUTE_UNUSED)
 {
-int max_present;
 #ifdef __linux__
+virBitmapPtr present_cpus = NULL;
 char *present_path = NULL;
-virBitmapPtr bitmap = NULL;
-#endif
+int npresent_cpus;
 
-if ((max_present = nodeGetCPUCount(sysfs_prefix)) < 0)
-return NULL;
+if ((npresent_cpus = nodeGetCPUCount(sysfs_prefix)) < 0)
+goto cleanup;
 
-#ifdef __linux__
 if (!(present_path = linuxGetCPUPresentPath(sysfs_prefix)))
-return NULL;
-if (virFileExists(present_path))
-bitmap = linuxParseCPUmap(max_present, present_path);
+goto cleanup;
+
+/* If the cpu/present file is available, parse it and exit */
+if (virFileExists(present_path)) {
+present_cpus = linuxParseCPUmap(npresent_cpus, present_path);
+goto cleanup;
+}
+
+/* If the file is not available, we can assume that the kernel is
+ * too old to support non-consecutive CPU ids and just mark all
+ * possible CPUs as present */
+if (!(present_cpus = virBitmapNew(npresent_cpus)))
+goto cleanup;
+
+virBitmapSetAll(present_cpus);
+
+ cleanup:
 VIR_FREE(present_path);
-return bitmap;
+
+return present_cpus;
 #endif
 virReportError(VIR_ERR_NO_SUPPORT, "%s",
-   _("non-continuous host cpu numbers not implemented on this 
platform"));
+   _("node present CPU map not implemented on this platform"));
 return NULL;
 }
 
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 00/10] nodeinfo: Various cleanups

[Andrea Bolognani]

This should take care of Peter's remarks, except the ones
that I've addressed separately.

Andrea Bolognani (10):
  nodeinfo: Introduce linuxGetCPUGlobalPath()
  nodeinfo: Introduce linuxGetCPUOnlinePath()
  nodeinfo: Rename linuxParseCPUmax() to linuxParseCPUCount()
  nodeinfo: Add old kernel compatibility to nodeGetPresentCPUBitmap()
  nodeinfo: Remove out parameter from nodeGetCPUBitmap()
  nodeinfo: Rename nodeGetCPUBitmap() to nodeGetOnlineCPUBitmap()
  nodeinfo: Phase out cpu_set_t usage
  nodeinfo: Use nodeGetOnlineCPUBitmap() when parsing node
  nodeinfo: Use a bitmap to keep track of node CPUs
  nodeinfo: Calculate present and online CPUs only once

 src/libvirt_private.syms |   2 +-
 src/nodeinfo.c   | 214 +--
 src/nodeinfo.h   |   2 +-
 3 files changed, 136 insertions(+), 82 deletions(-)

-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 05/10] nodeinfo: Remove out parameter from nodeGetCPUBitmap()

[Andrea Bolognani]

Not all users of this API will need the size of the returned
bitmap; those who do can simply call virBitmapSize() themselves.
---
 src/nodeinfo.c | 12 +---
 src/nodeinfo.h |  2 +-
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index ceb517a..69c15fc 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -1324,8 +1324,7 @@ nodeGetPresentCPUBitmap(const char *sysfs_prefix 
ATTRIBUTE_UNUSED)
 }
 
 virBitmapPtr
-nodeGetCPUBitmap(const char *sysfs_prefix ATTRIBUTE_UNUSED,
- int *max_id ATTRIBUTE_UNUSED)
+nodeGetCPUBitmap(const char *sysfs_prefix ATTRIBUTE_UNUSED)
 {
 #ifdef __linux__
 const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SYSTEM_PATH;
@@ -1363,8 +1362,7 @@ nodeGetCPUBitmap(const char *sysfs_prefix 
ATTRIBUTE_UNUSED,
 ignore_value(virBitmapSetBit(cpumap, i));
 }
 }
-if (max_id && cpumap)
-*max_id = present;
+
  cleanup:
 VIR_FREE(online_path);
 VIR_FREE(cpudir);
@@ -1685,7 +1683,6 @@ nodeGetCPUMap(const char *sysfs_prefix,
   unsigned int flags)
 {
 virBitmapPtr cpus = NULL;
-int maxpresent;
 int ret = -1;
 int dummy;
 
@@ -1694,7 +1691,7 @@ nodeGetCPUMap(const char *sysfs_prefix,
 if (!cpumap && !online)
 return nodeGetCPUCount(sysfs_prefix);
 
-if (!(cpus = nodeGetCPUBitmap(sysfs_prefix, &maxpresent)))
+if (!(cpus = nodeGetCPUBitmap(sysfs_prefix)))
 goto cleanup;
 
 if (cpumap && virBitmapToData(cpus, cpumap, &dummy) < 0)
@@ -1702,7 +1699,8 @@ nodeGetCPUMap(const char *sysfs_prefix,
 if (online)
 *online = virBitmapCountBits(cpus);
 
-ret = maxpresent;
+ret = virBitmapSize(cpus);
+
  cleanup:
 if (ret < 0 && cpumap)
 VIR_FREE(*cpumap);
diff --git a/src/nodeinfo.h b/src/nodeinfo.h
index 4f983c2..02af9c5 100644
--- a/src/nodeinfo.h
+++ b/src/nodeinfo.h
@@ -45,7 +45,7 @@ int nodeGetMemory(unsigned long long *mem,
   unsigned long long *freeMem);
 
 virBitmapPtr nodeGetPresentCPUBitmap(const char *sysfs_prefix);
-virBitmapPtr nodeGetCPUBitmap(const char *sysfs_prefix, int *max_id);
+virBitmapPtr nodeGetCPUBitmap(const char *sysfs_prefix);
 int nodeGetCPUCount(const char *sysfs_prefix);
 
 int nodeGetMemoryParameters(virTypedParameterPtr params,
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 07/10] nodeinfo: Phase out cpu_set_t usage

[Andrea Bolognani]

Swap out all instances of cpu_set_t and replace them with virBitmap,
which some of the code was already using anyway.

The changes are pretty mechanical, with one notable exception: an
assumption has been added on the max value we can run into while
reading either socket_it or core_id.

While this specific assumption was not in place before, we were
using cpu_set_t improperly by not making sure not to set any bit
past CPU_SETSIZE or explicitly allocating bigger bitmaps; in fact
the default size of a cpu_set_t, 1024, is way too low to run our
testsuite, which includes core_id values in the 2000s.
---
 src/nodeinfo.c | 65 ++
 1 file changed, 38 insertions(+), 27 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 3550e24..7d0e6b0 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -30,7 +30,6 @@
 #include 
 #include 
 #include 
-#include 
 #include "conf/domain_conf.h"
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
@@ -388,19 +387,6 @@ virNodeParseSocket(const char *dir,
 return ret;
 }
 
-# ifndef CPU_COUNT
-static int
-CPU_COUNT(cpu_set_t *set)
-{
-size_t i, count = 0;
-
-for (i = 0; i < CPU_SETSIZE; i++)
-if (CPU_ISSET(i, set))
-count++;
-return count;
-}
-# endif /* !CPU_COUNT */
-
 /* parses a node entry, returning number of processors in the node and
  * filling arguments */
 static int
@@ -415,15 +401,18 @@ virNodeParseNode(const char *sysfs_prefix,
  int *threads,
  int *offline)
 {
+/* Biggest value we can expect to be used as either socket id
+ * or core id. Bitmaps will need to be sized accordingly */
+const int ID_MAX = 4095;
 int ret = -1;
 int processors = 0;
 DIR *cpudir = NULL;
 struct dirent *cpudirent = NULL;
 virBitmapPtr present_cpumap = NULL;
+virBitmapPtr sockets_map = NULL;
+virBitmapPtr *cores_maps = NULL;
 int sock_max = 0;
-cpu_set_t sock_map;
 int sock;
-cpu_set_t *core_maps = NULL;
 int core;
 size_t i;
 int siblings;
@@ -445,7 +434,9 @@ virNodeParseNode(const char *sysfs_prefix,
 goto cleanup;
 
 /* enumerate sockets in the node */
-CPU_ZERO(&sock_map);
+if (!(sockets_map = virBitmapNew(ID_MAX + 1)))
+goto cleanup;
+
 while ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0) {
 if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
 continue;
@@ -462,7 +453,15 @@ virNodeParseNode(const char *sysfs_prefix,
 /* Parse socket */
 if ((sock = virNodeParseSocket(node, arch, cpu)) < 0)
 goto cleanup;
-CPU_SET(sock, &sock_map);
+if (sock > ID_MAX) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Socket %d can't be handled (max socket is %d)"),
+   sock, ID_MAX);
+goto cleanup;
+}
+
+if (virBitmapSetBit(sockets_map, sock) < 0)
+goto cleanup;
 
 if (sock > sock_max)
 sock_max = sock;
@@ -473,12 +472,13 @@ virNodeParseNode(const char *sysfs_prefix,
 
 sock_max++;
 
-/* allocate cpu maps for each socket */
-if (VIR_ALLOC_N(core_maps, sock_max) < 0)
+/* allocate cores maps for each socket */
+if (VIR_ALLOC_N(cores_maps, sock_max) < 0)
 goto cleanup;
 
 for (i = 0; i < sock_max; i++)
-CPU_ZERO(&core_maps[i]);
+if (!(cores_maps[i] = virBitmapNew(ID_MAX + 1)))
+goto cleanup;
 
 /* iterate over all CPU's in the node */
 rewinddir(cpudir);
@@ -502,7 +502,7 @@ virNodeParseNode(const char *sysfs_prefix,
 /* Parse socket */
 if ((sock = virNodeParseSocket(node, arch, cpu)) < 0)
 goto cleanup;
-if (!CPU_ISSET(sock, &sock_map)) {
+if (!virBitmapIsBitSet(sockets_map, sock)) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("CPU socket topology has changed"));
 goto cleanup;
@@ -515,8 +515,15 @@ virNodeParseNode(const char *sysfs_prefix,
 } else {
 core = virNodeGetCpuValue(node, cpu, "topology/core_id", 0);
 }
+if (core > ID_MAX) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Core %d can't be handled (max core is %d)"),
+   core, ID_MAX);
+goto cleanup;
+}
 
-CPU_SET(core, &core_maps[sock]);
+if (virBitmapSetBit(cores_maps[sock], core) < 0)
+goto cleanup;
 
 if (!(siblings = virNodeCountThreadSiblings(node, cpu)))
 goto cleanup;
@@ -529,13 +536,13 @@ virNodeParseNode(const char *sysfs_prefix,
 goto cleanup;
 
 /* finalize the returned data */
-*sockets = CPU_COUNT(&sock_map);
+*sockets = virBitmapCountBits(sockets_map);
 
 for (i = 0; i < sock_max; i++) {
-if (!CPU_ISSET(i, &sock_map))
+if (!virBitmapIsBitSet(sockets_map, i))

[libvirt] [PATCH v2 06/10] nodeinfo: Rename nodeGetCPUBitmap() to nodeGetOnlineCPUBitmap()

[Andrea Bolognani]

The new name makes it clear that the returned bitmap contains the
information about which CPUs are online, not eg. which CPUs are
present.

No behavioral change.
---
 src/libvirt_private.syms | 2 +-
 src/nodeinfo.c   | 6 +++---
 src/nodeinfo.h   | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b6347de..7f42e40 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -999,7 +999,6 @@ virLockManagerRelease;
 nodeAllocPages;
 nodeCapsInitNUMA;
 nodeGetCellsFreeMemory;
-nodeGetCPUBitmap;
 nodeGetCPUCount;
 nodeGetCPUMap;
 nodeGetCPUStats;
@@ -1008,6 +1007,7 @@ nodeGetInfo;
 nodeGetMemory;
 nodeGetMemoryParameters;
 nodeGetMemoryStats;
+nodeGetOnlineCPUBitmap;
 nodeGetPresentCPUBitmap;
 nodeSetMemoryParameters;
 
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 69c15fc..3550e24 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -1324,7 +1324,7 @@ nodeGetPresentCPUBitmap(const char *sysfs_prefix 
ATTRIBUTE_UNUSED)
 }
 
 virBitmapPtr
-nodeGetCPUBitmap(const char *sysfs_prefix ATTRIBUTE_UNUSED)
+nodeGetOnlineCPUBitmap(const char *sysfs_prefix ATTRIBUTE_UNUSED)
 {
 #ifdef __linux__
 const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SYSTEM_PATH;
@@ -1369,7 +1369,7 @@ nodeGetCPUBitmap(const char *sysfs_prefix 
ATTRIBUTE_UNUSED)
 return cpumap;
 #else
 virReportError(VIR_ERR_NO_SUPPORT, "%s",
-   _("node cpumap not implemented on this platform"));
+   _("node online CPU map not implemented on this platform"));
 return NULL;
 #endif
 }
@@ -1691,7 +1691,7 @@ nodeGetCPUMap(const char *sysfs_prefix,
 if (!cpumap && !online)
 return nodeGetCPUCount(sysfs_prefix);
 
-if (!(cpus = nodeGetCPUBitmap(sysfs_prefix)))
+if (!(cpus = nodeGetOnlineCPUBitmap(sysfs_prefix)))
 goto cleanup;
 
 if (cpumap && virBitmapToData(cpus, cpumap, &dummy) < 0)
diff --git a/src/nodeinfo.h b/src/nodeinfo.h
index 02af9c5..1810c1c 100644
--- a/src/nodeinfo.h
+++ b/src/nodeinfo.h
@@ -45,7 +45,7 @@ int nodeGetMemory(unsigned long long *mem,
   unsigned long long *freeMem);
 
 virBitmapPtr nodeGetPresentCPUBitmap(const char *sysfs_prefix);
-virBitmapPtr nodeGetCPUBitmap(const char *sysfs_prefix);
+virBitmapPtr nodeGetOnlineCPUBitmap(const char *sysfs_prefix);
 int nodeGetCPUCount(const char *sysfs_prefix);
 
 int nodeGetMemoryParameters(virTypedParameterPtr params,
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 09/10] nodeinfo: Use a bitmap to keep track of node CPUs

[Andrea Bolognani]

Keep track of what CPUs belong to the current node while walking
through the sysfs node entry, so we don't need to do it a second
time immediately afterwards.

This also allows us to loop through all CPUs that are part of a
node in guaranteed ascending order, which is something that is
required for some upcoming changes.
---
 src/nodeinfo.c | 28 ++--
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index c97dece..2328a86 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -410,8 +410,10 @@ virNodeParseNode(const char *sysfs_prefix,
 struct dirent *cpudirent = NULL;
 virBitmapPtr present_cpumap = NULL;
 virBitmapPtr online_cpus_map = NULL;
+virBitmapPtr node_cpus_map = NULL;
 virBitmapPtr sockets_map = NULL;
 virBitmapPtr *cores_maps = NULL;
+int npresent_cpus;
 int sock_max = 0;
 int sock;
 int core;
@@ -432,10 +434,16 @@ virNodeParseNode(const char *sysfs_prefix,
 present_cpumap = nodeGetPresentCPUBitmap(sysfs_prefix);
 if (!present_cpumap)
 goto cleanup;
-online_cpus_map = nodeGetOnlineCPUBitmap(sysfs_prefix, NULL);
+online_cpus_map = nodeGetOnlineCPUBitmap(sysfs_prefix);
 if (!online_cpus_map)
 goto cleanup;
 
+npresent_cpus = virBitmapSize(present_cpumap);
+
+/* Keep track of the CPUs that belong to the current node */
+if (!(node_cpus_map = virBitmapNew(npresent_cpus)))
+goto cleanup;
+
 /* enumerate sockets in the node */
 if (!(sockets_map = virBitmapNew(ID_MAX + 1)))
 goto cleanup;
@@ -447,6 +455,10 @@ virNodeParseNode(const char *sysfs_prefix,
 if (!virBitmapIsBitSet(present_cpumap, cpu))
 continue;
 
+/* Mark this CPU as part of the current node */
+if (virBitmapSetBit(node_cpus_map, cpu) < 0)
+goto cleanup;
+
 if (!virBitmapIsBitSet(online_cpus_map, cpu))
 continue;
 
@@ -480,13 +492,11 @@ virNodeParseNode(const char *sysfs_prefix,
 if (!(cores_maps[i] = virBitmapNew(ID_MAX + 1)))
 goto cleanup;
 
-/* iterate over all CPU's in the node */
-rewinddir(cpudir);
-while ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0) {
-if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
-continue;
+/* Iterate over all CPUs in the node, in ascending order */
+for (cpu = 0; cpu < npresent_cpus; cpu++) {
 
-if (!virBitmapIsBitSet(present_cpumap, cpu))
+/* Skip CPUs that are not part of the current node */
+if (!virBitmapIsBitSet(node_cpus_map, cpu))
 continue;
 
 if (!virBitmapIsBitSet(online_cpus_map, cpu)) {
@@ -529,9 +539,6 @@ virNodeParseNode(const char *sysfs_prefix,
 *threads = siblings;
 }
 
-if (direrr < 0)
-goto cleanup;
-
 /* finalize the returned data */
 *sockets = virBitmapCountBits(sockets_map);
 
@@ -557,6 +564,7 @@ virNodeParseNode(const char *sysfs_prefix,
 virBitmapFree(cores_maps[i]);
 VIR_FREE(cores_maps);
 virBitmapFree(sockets_map);
+virBitmapFree(node_cpus_map);
 virBitmapFree(online_cpus_map);
 virBitmapFree(present_cpumap);
 
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v6 0/5] nodeinfo: Add support for subcores

[Andrea Bolognani]

Changes from v5 to v6:

  * updated to work on top of

  [PATCH v2 00/10] nodeinfo: Various cleanups

I'm only sending patch 1/5 to the list, because all the
other commits haven't changed since v5 and because
this way there's no need to perform moderation due to
the message size.

For the same reason, I've heavily edited the following
summary by cutting out everything that belonged to a
test case.

Cheers.


Andrea Bolognani (3):
  tests: Add subcores-default nodeinfo test
  tests: Add subcores-partial nodeinfo test
  tests: Add subcores-invalid nodeinfo test

Shivaprasad G Bhat (2):
  nodeinfo: Fix output on PPC64 KVM hosts
  tests: Prepare for subcore tests

 src/libvirt_private.syms   |   1 +
 src/nodeinfo.c | 144 -
 src/nodeinfo.h |   1 +
 tests/Makefile.am  |   6 +
 [...]
 tests/nodeinfomock.c   |  35 +
 tests/nodeinfotest.c   |   5 +-
 1348 files changed, 2122 insertions(+), 6 deletions(-)
 [...]
 create mode 100644 tests/nodeinfomock.c

-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v6 1/5] nodeinfo: Fix output on PPC64 KVM hosts

[Andrea Bolognani]

From: Shivaprasad G Bhat 

The nodeinfo is reporting incorrect number of cpus and incorrect host
topology on PPC64 KVM hosts. The KVM hypervisor on PPC64 needs only
the primary thread in a core to be online, and the secondaries offlined.
While scheduling a guest in, the kvm scheduler wakes up the secondaries to
run in guest context.

The host scheduling of the guests happen at the core level(as only primary
thread is online). The kvm scheduler exploits as many threads of the core
as needed by guest. Further, starting POWER8, the processor allows splitting
a physical core into multiple subcores with 2 or 4 threads each. Again, only
the primary thread in a subcore is online in the host. The KVM-PPC
scheduler allows guests to exploit all the offline threads in the subcore,
by bringing them online when needed.
(Kernel patches on split-core 
http://www.spinics.net/lists/kvm-ppc/msg09121.html)

Recently with dynamic micro-threading changes in ppc-kvm, makes sure
to utilize all the offline cpus across guests, and across guests with
different cpu topologies.
(https://www.mail-archive.com/kvm@vger.kernel.org/msg115978.html)

Since the offline cpus are brought online in the guest context, it is safe
to count them as online. Nodeinfo today discounts these offline cpus from
cpu count/topology calclulation, and the nodeinfo output is not of any help
and the host appears overcommited when it is actually not.

The patch carefully counts those offline threads whose primary threads are
online. The host topology displayed by the nodeinfo is also fixed when the
host is in valid kvm state.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Andrea Bolognani 
---
 src/libvirt_private.syms |   1 +
 src/nodeinfo.c   | 144 +--
 src/nodeinfo.h   |   1 +
 3 files changed, 142 insertions(+), 4 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7f42e40..46e535e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1009,6 +1009,7 @@ nodeGetMemoryParameters;
 nodeGetMemoryStats;
 nodeGetOnlineCPUBitmap;
 nodeGetPresentCPUBitmap;
+nodeGetThreadsPerSubcore;
 nodeSetMemoryParameters;
 
 
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 31095a4..0fedc0e 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -31,6 +31,12 @@
 #include 
 #include 
 #include "conf/domain_conf.h"
+#include 
+#include 
+
+#if HAVE_LINUX_KVM_H
+# include 
+#endif
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
 # include 
@@ -391,14 +397,15 @@ virNodeParseSocket(const char *dir,
  * filling arguments */
 static int
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3)
-ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6)
-ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(8)
-ATTRIBUTE_NONNULL(9)
+ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(7)
+ATTRIBUTE_NONNULL(8) ATTRIBUTE_NONNULL(9)
+ATTRIBUTE_NONNULL(10)
 virNodeParseNode(const char *node,
  virArch arch,
  virBitmapPtr present_cpus_map,
  int npresent_cpus,
  virBitmapPtr online_cpus_map,
+ int threads_per_subcore,
  int *sockets,
  int *cores,
  int *threads,
@@ -491,7 +498,18 @@ virNodeParseNode(const char *node,
 continue;
 
 if (!virBitmapIsBitSet(online_cpus_map, cpu)) {
-(*offline)++;
+if (threads_per_subcore > 0 &&
+cpu % threads_per_subcore != 0 &&
+virBitmapIsBitSet(online_cpus_map,
+  cpu - (cpu % threads_per_subcore))) {
+/* Secondary offline threads are counted as online when
+ * subcores are in use and the corresponding primary
+ * thread is online */
+processors++;
+} else {
+/* But they are counted as offline otherwise */
+(*offline)++;
+}
 continue;
 }
 
@@ -542,6 +560,12 @@ virNodeParseNode(const char *node,
 *cores = core;
 }
 
+if (threads_per_subcore > 0) {
+/* The thread count ignores offline threads, which means that only
+ * only primary threads have been considered so far. If subcores
+ * are in use, we need to also account for secondary threads */
+*threads *= threads_per_subcore;
+}
 ret = processors;
 
  cleanup:
@@ -560,6 +584,48 @@ virNodeParseNode(const char *node,
 return ret;
 }
 
+/* Check whether the host subcore configuration is valid.
+ *
+ * A valid configuration is one where no secondary thread is online;
+ * the primary thread in a subcore is always the first one */
+static bool
+nodeHasValidSubcoreConfiguration(const char *sysfs_prefix,
+ int threads_per_subcore)
+{
+virBitmapPtr online_cpus = NULL;
+int nonline_cpus;
+int cpu;
+bool ret = false;
+
+/* No point in checking if su

Re: [libvirt] [PATCH v2 08/10] nodeinfo: Use nodeGetOnlineCPUBitmap() when parsing node

[Andrea Bolognani]

On Wed, 2015-07-22 at 10:34 +0200, Peter Krempa wrote:
> 
> > @@ -432,6 +432,9 @@ virNodeParseNode(const char *sysfs_prefix,
> >  present_cpumap = nodeGetPresentCPUBitmap(sysfs_prefix);
> >  if (!present_cpumap)
> >  goto cleanup;
> > +online_cpus_map = nodeGetOnlineCPUBitmap(sysfs_prefix, NULL);
> 
> This can't be compiled since nodeGetOnlineCPUBitmap doesn't have the
> second parameter. I see that you've fixed it in next commit so I'll 
> move
> it here before pushing.

Sorry, must have slipped through during one of the rebases.
Thanks for catching it.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 10/10] nodeinfo: Calculate present and online CPUs only once

[Andrea Bolognani]

On Wed, 2015-07-22 at 10:44 +0200, Peter Krempa wrote:
> 
> > -present_cpumap = nodeGetPresentCPUBitmap(sysfs_prefix);
> > -if (!present_cpumap)
> > -goto cleanup;
> > -online_cpus_map = nodeGetOnlineCPUBitmap(sysfs_prefix);
> > -if (!online_cpus_map)
> > -goto cleanup;
> > -
> > -npresent_cpus = virBitmapSize(present_cpumap);
> 
> This still can be extracted here since the caller doesn't care about 
> the
> npreset count so you are basically passing redundant info.
> 
> ACK to the rest. I'll revert this part and adjust the patch 
> accordingly
> before pushing.

Sure, it was intended as an optimization but then again
we already agreed that it doesn't really make sense
because virBitmapSize() does so little, plus it indeed
looks cleaner if the caller is passing one less parameter.

Thanks.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] Bump tar format?

[Andrea Bolognani]

Hi everyone,

commit cc3d52e[1] has apparently broken 'make dist' for at least some
platforms: Fedora 20 is fine[2], but both Fedora 21 and rawhide[3][4]
are currently failing. My laptop is running Fedora 22 and can make
dist just fine, which is of course very confusing.

The issue is caused by the following tar error:

  tar: libvirt-1.2.18/tests/nodeinfodata/linux-deconfigured-cpus/ \
   node/node0/cpu16/topology/core_siblings_list:  \
  link name is too long; not dumped

There seem to be basically two ways to solve the failure:

  1. rename the test case, making it shorter;

  2. bump tar format from ustar, which has limitations on the length
 of file names, to posix/pax, which has no such limitations.

I'm not a fan of the former option because short names are usually very
opaque, see tests/nodeinfodata/linux-test[0-8] for some perfect
examples.

On the other hand, I'm not sure whether bumping the tar version would
affect portability in a significant way: the automake documentation[5]
reports that

  this format is very young and should probably be restricted to
  packages that target only very modern platforms

but the GNU tar documentation[6], while containing some warnings as
well, suggest that this format is going to become the default.

Any suggestions on how to proceed?

Cheers.


[1] https://www.redhat.com/archives/libvir-list/2015-July/msg00662.html
[2] https://ci.centos.org/view/libvirt-project/job/libvirt-daemon
-rpm/systems=libvirt-fedora-20/327/
[3] https://ci.centos.org/view/libvirt-project/job/libvirt-daemon
-rpm/systems=libvirt-fedora-21/327/
[4] https://ci.centos.org/view/libvirt-project/job/libvirt-daemon
-rpm/systems=libvirt-fedora-rawhide/327/
[5] https://www.gnu.org/software/automake/manual/html_node/List-of
-Automake-options.html
[6] http://www.gnu.org/software/tar/manual/html_chapter/tar_8.html
-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v7 0/5] nodeinfo: Add support for subcores

[Andrea Bolognani]

Changes from v6 to v7:

  * rebased on top of master now that the series this one
builds on have been merged

Again, I'm only sending patch 1/5 to the list because the
rest of the series hasn't changed from v5. Here are
pointers for your convenience:

  2/5 http://www.redhat.com/archives/libvir-list/2015-July/msg00676.html
  3/5 http://www.redhat.com/archives/libvir-list/2015-July/msg00680.html
  4/5 http://www.redhat.com/archives/libvir-list/2015-July/msg00681.html
  5/5 http://www.redhat.com/archives/libvir-list/2015-July/msg00682.html

Cheers.


Andrea Bolognani (3):
  tests: Add subcores-default nodeinfo test
  tests: Add subcores-partial nodeinfo test
  tests: Add subcores-invalid nodeinfo test

Shivaprasad G Bhat (2):
  nodeinfo: Fix output on PPC64 KVM hosts
  tests: Prepare for subcore tests

 src/libvirt_private.syms   |   1 +
 src/nodeinfo.c | 144 -
 src/nodeinfo.h |   1 +
 tests/Makefile.am  |   6 +
 [...]
 tests/nodeinfomock.c   |  35 +
 tests/nodeinfotest.c   |   5 +-
 1348 files changed, 2122 insertions(+), 6 deletions(-)

-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v7 1/5] nodeinfo: Fix output on PPC64 KVM hosts

[Andrea Bolognani]

From: Shivaprasad G Bhat 

The nodeinfo is reporting incorrect number of cpus and incorrect host
topology on PPC64 KVM hosts. The KVM hypervisor on PPC64 needs only
the primary thread in a core to be online, and the secondaries offlined.
While scheduling a guest in, the kvm scheduler wakes up the secondaries to
run in guest context.

The host scheduling of the guests happen at the core level(as only primary
thread is online). The kvm scheduler exploits as many threads of the core
as needed by guest. Further, starting POWER8, the processor allows splitting
a physical core into multiple subcores with 2 or 4 threads each. Again, only
the primary thread in a subcore is online in the host. The KVM-PPC
scheduler allows guests to exploit all the offline threads in the subcore,
by bringing them online when needed.
(Kernel patches on split-core 
http://www.spinics.net/lists/kvm-ppc/msg09121.html)

Recently with dynamic micro-threading changes in ppc-kvm, makes sure
to utilize all the offline cpus across guests, and across guests with
different cpu topologies.
(https://www.mail-archive.com/kvm@vger.kernel.org/msg115978.html)

Since the offline cpus are brought online in the guest context, it is safe
to count them as online. Nodeinfo today discounts these offline cpus from
cpu count/topology calclulation, and the nodeinfo output is not of any help
and the host appears overcommited when it is actually not.

The patch carefully counts those offline threads whose primary threads are
online. The host topology displayed by the nodeinfo is also fixed when the
host is in valid kvm state.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Andrea Bolognani 
---
 src/libvirt_private.syms |   1 +
 src/nodeinfo.c   | 144 +--
 src/nodeinfo.h   |   1 +
 3 files changed, 142 insertions(+), 4 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ff322d6..0517c24 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1010,6 +1010,7 @@ nodeGetMemoryParameters;
 nodeGetMemoryStats;
 nodeGetOnlineCPUBitmap;
 nodeGetPresentCPUBitmap;
+nodeGetThreadsPerSubcore;
 nodeSetMemoryParameters;
 
 
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index fb932c8..5fbe374 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -31,6 +31,12 @@
 #include 
 #include 
 #include "conf/domain_conf.h"
+#include 
+#include 
+
+#if HAVE_LINUX_KVM_H
+# include 
+#endif
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
 # include 
@@ -392,13 +398,14 @@ virNodeParseSocket(const char *dir,
  * filling arguments */
 static int
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3)
-ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5)
-ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(7)
-ATTRIBUTE_NONNULL(8)
+ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(6)
+ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(8)
+ATTRIBUTE_NONNULL(9)
 virNodeParseNode(const char *node,
  virArch arch,
  virBitmapPtr present_cpus_map,
  virBitmapPtr online_cpus_map,
+ int threads_per_subcore,
  int *sockets,
  int *cores,
  int *threads,
@@ -492,7 +499,18 @@ virNodeParseNode(const char *node,
 continue;
 
 if (!virBitmapIsBitSet(online_cpus_map, cpu)) {
-(*offline)++;
+if (threads_per_subcore > 0 &&
+cpu % threads_per_subcore != 0 &&
+virBitmapIsBitSet(online_cpus_map,
+  cpu - (cpu % threads_per_subcore))) {
+/* Secondary offline threads are counted as online when
+ * subcores are in use and the corresponding primary
+ * thread is online */
+processors++;
+} else {
+/* But they are counted as offline otherwise */
+(*offline)++;
+}
 continue;
 }
 
@@ -543,6 +561,12 @@ virNodeParseNode(const char *node,
 *cores = core;
 }
 
+if (threads_per_subcore > 0) {
+/* The thread count ignores offline threads, which means that only
+ * only primary threads have been considered so far. If subcores
+ * are in use, we need to also account for secondary threads */
+*threads *= threads_per_subcore;
+}
 ret = processors;
 
  cleanup:
@@ -561,6 +585,48 @@ virNodeParseNode(const char *node,
 return ret;
 }
 
+/* Check whether the host subcore configuration is valid.
+ *
+ * A valid configuration is one where no secondary thread is online;
+ * the primary thread in a subcore is always the first one */
+static bool
+nodeHasValidSubcoreConfiguration(const char *sysfs_prefix,
+ int threads_per_subcore)
+{
+virBitmapPtr online_cpus = NULL;
+int nonline_cpus;
+int cpu;
+bool ret = false;
+
+/* No point in checking if subcores are not in use */
+if (threads

Re: [libvirt] Bump tar format?

[Andrea Bolognani]

On Wed, 2015-07-22 at 15:58 +0100, Daniel P. Berrange wrote:
> 
> > Any suggestions on how to proceed?
> 
> We've hit this many times before and always just shorten the file
> names in GIT to avoid it.
> 
> Really we could do with adding a check for filename length to
> make check to avoid this again.

Might not be trivial, as limits are apparently different for symbolic
links and regular files, nevermind the fact that very similar versions
of tar (eg. Fedora 22 and Fedora rawhide) don't behave the same.

Anyway, if bumping the tar format is not acceptable, I guess the only
option is to shorten the name of the test case.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Bump tar format?

[Andrea Bolognani]

On Wed, 2015-07-22 at 17:01 +0100, Daniel P. Berrange wrote:
> 
> > Might not be trivial, as limits are apparently different for
> > symbolic
> > links and regular files, nevermind the fact that very similar
> > versions
> > of tar (eg. Fedora 22 and Fedora rawhide) don't behave the same.
> 
> If we assume 'libvirt-X.Y.Z/' (with double-digits for each version
> field)
> as a prefix, that's upto 17 characters consumed. With a limit of 100
> chars
> for symlinks, we should simply make sure all our file names are 83
> characters
> or less
> 
> $ ./build-aux/vc-list-files | perl -e 'while (<>) { die "filename $_
> too long" if length($_) > 83 }'
> filename tests/cputestdata/x86-host-Haswell-noTSX+Haswell
> -noTSX,haswell,Haswell-noTSX-result.xml
>  too long at -e line 1, <> line 1280.

Unfortunately, that script[1] doesn't catch any of the files
that are breaking the build, and prints out a bunch of false
positives instead.

That's because vc-list-files explicitly filters out symbolic
links; even if it didn't, you would only get the name of the
actual symbolic link in the output, for example

  tests/nodeinfodata/linux-deconfigured-cpus/node/node17/cpu120

which is still way shorter than 83 characters and as such
wouldn't trigger the error condition.

The problem is that, IIUC, all symbolic links are converted
to hard links when creating the tarball and if the path to
the file requiring this conversion has length >100, like

  libvirt-1.2.18/tests/nodeinfodata/linux-deconfigured-cpus/ \
  node/node17/cpu120/topology/thread_siblings

then we get into trouble.

Cheers.


[1] with the call to die() replaced with a print() so that it
doesn't stop at the first error 
-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] nodeinfo: Check for errors when reading core_id

[Andrea Bolognani]

---
 src/nodeinfo.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index fb932c8..ba633a1 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -512,7 +512,9 @@ virNodeParseNode(const char *node,
 /* logical cpu is equivalent to a core on s390 */
 core = cpu;
 } else {
-core = virNodeGetCpuValue(node, cpu, "topology/core_id", 0);
+if ((core = virNodeGetCpuValue(node, cpu,
+   "topology/core_id", 0)) < 0)
+goto cleanup;
 }
 if (core > ID_MAX) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Plan for next release

[Andrea Bolognani]

On Wed, 2015-07-22 at 23:50 +0800, Daniel Veillard wrote:
>   We are getting close to the end of the month, so we may need to 
> freeze
> next Monday or Tuesday.

I guess a requirement for freeze, or at least for release, is
deciding how we want to handle symlinks with long paths[1] so
that we can, you know, make dist :)

Cheers.


[1] https://www.redhat.com/archives/libvir-list/2015-July/msg00869.html
-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Plan for next release

[Andrea Bolognani]

On Fri, 2015-07-24 at 09:35 -0400, Laine Stump wrote:
> On 07/24/2015 07:28 AM, Andrea Bolognani wrote:
> > On Wed, 2015-07-22 at 23:50 +0800, Daniel Veillard wrote:
> > >   We are getting close to the end of the month, so we may need to
> > > 
> > > freeze
> > > next Monday or Tuesday.
> > I guess a requirement for freeze, or at least for release, is
> > deciding how we want to handle symlinks with long paths[1] so
> > that we can, you know, make dist :)
> 
> It needs to be fixed far before the release - make rpm is broken too,
> and that's my preferred method of testing.
> 
> I've fixed it locally with a patch to change "deconfigured-cpus" to
> "down-cpus", but didn't want to push it unilaterally, even though the
> build is broken.

I just sent a patch renaming it to "nonpresent", which is still
short enough not to run into the lenght limit.

It's certainly going to be stuck in the moderation queue, so if
you have the permissions to do so please let it through :)

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v8 1/5] nodeinfo: Fix output on PPC64 KVM hosts

[Andrea Bolognani]

From: Shivaprasad G Bhat 

The nodeinfo is reporting incorrect number of cpus and incorrect host
topology on PPC64 KVM hosts. The KVM hypervisor on PPC64 needs only
the primary thread in a core to be online, and the secondaries offlined.
While scheduling a guest in, the kvm scheduler wakes up the secondaries to
run in guest context.

The host scheduling of the guests happen at the core level(as only primary
thread is online). The kvm scheduler exploits as many threads of the core
as needed by guest. Further, starting POWER8, the processor allows splitting
a physical core into multiple subcores with 2 or 4 threads each. Again, only
the primary thread in a subcore is online in the host. The KVM-PPC
scheduler allows guests to exploit all the offline threads in the subcore,
by bringing them online when needed.
(Kernel patches on split-core 
http://www.spinics.net/lists/kvm-ppc/msg09121.html)

Recently with dynamic micro-threading changes in ppc-kvm, makes sure
to utilize all the offline cpus across guests, and across guests with
different cpu topologies.
(https://www.mail-archive.com/kvm@vger.kernel.org/msg115978.html)

Since the offline cpus are brought online in the guest context, it is safe
to count them as online. Nodeinfo today discounts these offline cpus from
cpu count/topology calclulation, and the nodeinfo output is not of any help
and the host appears overcommited when it is actually not.

The patch carefully counts those offline threads whose primary threads are
online. The host topology displayed by the nodeinfo is also fixed when the
host is in valid kvm state.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Andrea Bolognani 
---
 src/libvirt_private.syms |   1 +
 src/nodeinfo.c   | 144 +--
 src/nodeinfo.h   |   1 +
 3 files changed, 142 insertions(+), 4 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ff322d6..0517c24 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1010,6 +1010,7 @@ nodeGetMemoryParameters;
 nodeGetMemoryStats;
 nodeGetOnlineCPUBitmap;
 nodeGetPresentCPUBitmap;
+nodeGetThreadsPerSubcore;
 nodeSetMemoryParameters;
 
 
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index ba633a1..53bbc54 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -31,6 +31,12 @@
 #include 
 #include 
 #include "conf/domain_conf.h"
+#include 
+#include 
+
+#if HAVE_LINUX_KVM_H
+# include 
+#endif
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
 # include 
@@ -392,13 +398,14 @@ virNodeParseSocket(const char *dir,
  * filling arguments */
 static int
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3)
-ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5)
-ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(7)
-ATTRIBUTE_NONNULL(8)
+ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(6)
+ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(8)
+ATTRIBUTE_NONNULL(9)
 virNodeParseNode(const char *node,
  virArch arch,
  virBitmapPtr present_cpus_map,
  virBitmapPtr online_cpus_map,
+ int threads_per_subcore,
  int *sockets,
  int *cores,
  int *threads,
@@ -492,7 +499,18 @@ virNodeParseNode(const char *node,
 continue;
 
 if (!virBitmapIsBitSet(online_cpus_map, cpu)) {
-(*offline)++;
+if (threads_per_subcore > 0 &&
+cpu % threads_per_subcore != 0 &&
+virBitmapIsBitSet(online_cpus_map,
+  cpu - (cpu % threads_per_subcore))) {
+/* Secondary offline threads are counted as online when
+ * subcores are in use and the corresponding primary
+ * thread is online */
+processors++;
+} else {
+/* But they are counted as offline otherwise */
+(*offline)++;
+}
 continue;
 }
 
@@ -545,6 +563,12 @@ virNodeParseNode(const char *node,
 *cores = core;
 }
 
+if (threads_per_subcore > 0) {
+/* The thread count ignores offline threads, which means that only
+ * only primary threads have been considered so far. If subcores
+ * are in use, we need to also account for secondary threads */
+*threads *= threads_per_subcore;
+}
 ret = processors;
 
  cleanup:
@@ -563,6 +587,48 @@ virNodeParseNode(const char *node,
 return ret;
 }
 
+/* Check whether the host subcore configuration is valid.
+ *
+ * A valid configuration is one where no secondary thread is online;
+ * the primary thread in a subcore is always the first one */
+static bool
+nodeHasValidSubcoreConfiguration(const char *sysfs_prefix,
+ int threads_per_subcore)
+{
+virBitmapPtr online_cpus = NULL;
+int nonline_cpus;
+int cpu;
+bool ret = false;
+
+/* No point in checking if subcores are not in use */
+if (threads

[libvirt] [PATCH v8 2/5] tests: Prepare for subcore tests

[Andrea Bolognani]

From: Shivaprasad G Bhat 

The nodeGetThreadsPerSubcore() function is mocked to return 8 for
ppc64 tests, which corresponds to the default subcore mode.

Update the expected output for the deconfigured-cpus nodeinfo
test to account for this change.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Andrea Bolognani 
---
 tests/Makefile.am  |  6 
 .../nodeinfodata/linux-ppc64-deconf-cpus.expected  |  2 +-
 tests/nodeinfomock.c   | 35 ++
 tests/nodeinfotest.c   |  2 +-
 4 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100644 tests/nodeinfomock.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index b202195..bde7f5b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -412,6 +412,7 @@ test_libraries = libshunload.la \
vircgroupmock.la \
virpcimock.la \
virnetdevmock.la \
+   nodeinfomock.la \
$(NULL)
 if WITH_QEMU
 test_libraries += libqemumonitortestutils.la \
@@ -1048,6 +1049,11 @@ virpcimock_la_LIBADD = $(GNULIB_LIBS) \
   ../src/libvirt.la
 virpcimock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
 
+nodeinfomock_la_SOURCES = \
+   nodeinfomock.c
+nodeinfomock_la_CFLAGS = $(AM_CFLAGS)
+nodeinfomock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
+
 virnetdevtest_SOURCES = \
virnetdevtest.c testutils.h testutils.c
 virnetdevtest_CFLAGS = $(AM_CFLAGS) $(LIBNL_CFLAGS)
diff --git a/tests/nodeinfodata/linux-ppc64-deconf-cpus.expected 
b/tests/nodeinfodata/linux-ppc64-deconf-cpus.expected
index 304f423..113bfa8 100644
--- a/tests/nodeinfodata/linux-ppc64-deconf-cpus.expected
+++ b/tests/nodeinfodata/linux-ppc64-deconf-cpus.expected
@@ -1 +1 @@
-CPUs: 10/80, MHz: 3690, Nodes: 1, Sockets: 1, Cores: 80, Threads: 1
+CPUs: 80/80, MHz: 3690, Nodes: 1, Sockets: 1, Cores: 80, Threads: 1
diff --git a/tests/nodeinfomock.c b/tests/nodeinfomock.c
new file mode 100644
index 000..b9c0152
--- /dev/null
+++ b/tests/nodeinfomock.c
@@ -0,0 +1,35 @@
+/*
+ * 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/>.
+ *
+ */
+
+#include 
+
+#include "internal.h"
+#include "nodeinfo.h"
+
+int
+nodeGetThreadsPerSubcore(virArch arch)
+{
+int threads_per_subcore = 0;
+
+// Emulate SMT=8 on POWER hardware
+if (ARCH_IS_PPC64(arch))
+threads_per_subcore = 8;
+
+return threads_per_subcore;
+}
diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c
index 3fffdb2..ccef4a9 100644
--- a/tests/nodeinfotest.c
+++ b/tests/nodeinfotest.c
@@ -256,6 +256,6 @@ mymain(void)
 return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
-VIRT_TEST_MAIN(mymain)
+VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/nodeinfomock.so")
 
 #endif /* __linux__ */
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] tests: Shorten nodeinfo test name

[Andrea Bolognani]

On Mon, 2015-07-27 at 09:33 +0100, Daniel P. Berrange wrote:
> On Fri, Jul 24, 2015 at 04:16:26PM +0200, Andrea Bolognani wrote:
> > The long name was causing make dist to fail, at least on certain
> > operating systems.
> 
> ACK

Thanks, but Daniel already pushed an equivalent fix
so this is not needed anymore.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v8 1/5] nodeinfo: Fix output on PPC64 KVM hosts

[Andrea Bolognani]

On Wed, 2015-07-29 at 13:12 +0530, Shivaprasad bhat wrote:
> 
> > +#if HAVE_LINUX_KVM_H && defined(KVM_CAP_PPC_SMT)
> > +if (ARCH_IS_PPC64(arch)) {
> > +
> > +kvmfd = open("/dev/kvm", O_RDONLY);
> > +if (kvmfd < 0) {
> > +threads_per_subcore = -1;
> 
> Its okay for a guest to not have kvm/qemu packages installed and 
> open()
> would fail.
> The caller goes to cleanup because of -1 and we get this error
> 
> error: failed to get node information
> error: An error occurred, but the cause is unknown
> 
> If we remove the -1 assignment we should be good. Even on a host,
> user might not want to use kvm that should also be treated with the 
> usual
> cpu counting.

What about checking for the file's existence before trying
to open it?

That way we can ignore the cases where we're okay with not
having /dev/kvm, eg. the kvm modules are not installed or
loaded, and we can still error out when something else is
wrong, eg. permission error.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v9 0/5] nodeinfo: Add support for subcores

[Andrea Bolognani]

Only patch 1/5 has been updated, all the other patches
are the same as v8:

  2/5 https://www.redhat.com/archives/libvir-list/2015-July/msg01045.html
  3/5 https://www.redhat.com/archives/libvir-list/2015-July/msg01048.html
  4/5 https://www.redhat.com/archives/libvir-list/2015-July/msg01049.html
  5/5 https://www.redhat.com/archives/libvir-list/2015-July/msg01050.html

I'm including the full history below.

John, this should be finally ready to be merged, so if you'd
like to have another go at reviewing it... :)

Cheers.


Changes in v9:

  * take into account the fact that kvm might not be loaded
or even installed

Changes in v8:

  * shortened test names so that make dist doesn't
stop working again

Changes in v7:

  * rebased on top of master now that the series this one
builds on have been merged

Changes in v6:

  * updated to work on top of

  [PATCH v2 00/10] nodeinfo: Various cleanups

Changes in v5:

  * streamlined the logic used to decide whether the subcore
configuration is valid and moved it to a separate function

  * split the tests into separate commits for easier review and
to hopefully avoid having trouble with the list due to the
message size

Changes in v4:

  * removed a printf() statement

  * fixed typo in a commit message

Changes in v3:

  * the function to get the number of threads per subcore
has been moved to the from virarch.c, which deals with
architecture names only and is therefore not the right
place to read host configuration, to nodeinfo.c where
the rest of this stuff lives

  * said function has also been given a shorter name

  * the "valid subcore mode" boolean has been removed:
threads_per_subcore will be a positive number if
subcores should be taken into account, and if that's
not the case (x86 host, tainted configuration) it
will simply be zero, so now the code needs to keep
track of a single variable instead of two

  * the test case has been renamed to be more
descriptive

  * the test data has been cleaned up by removing all
cpu/cpu*/node* links, which prevented 'make dist'
from working due to recursive linking


Andrea Bolognani (3):
  tests: Add subcores1 nodeinfo test
  tests: Add subcores2 nodeinfo test
  tests: Add subcores3 nodeinfo test

Shivaprasad G Bhat (2):
  nodeinfo: Fix output on PPC64 KVM hosts
  tests: Prepare for subcore tests

 src/libvirt_private.syms   |   1 +
 src/nodeinfo.c | 156 -
 src/nodeinfo.h |   1 +
 tests/Makefile.am  |   6 +
 [...]
 tests/nodeinfomock.c   |  35 +
 tests/nodeinfotest.c   |   8 +-
 1348 files changed, 2137 insertions(+), 6 deletions(-)
 [...]
 create mode 100644 tests/nodeinfomock.c

-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v9 1/5] nodeinfo: Fix output on PPC64 KVM hosts

[Andrea Bolognani]

From: Shivaprasad G Bhat 

The nodeinfo is reporting incorrect number of cpus and incorrect host
topology on PPC64 KVM hosts. The KVM hypervisor on PPC64 needs only
the primary thread in a core to be online, and the secondaries offlined.
While scheduling a guest in, the kvm scheduler wakes up the secondaries to
run in guest context.

The host scheduling of the guests happen at the core level(as only primary
thread is online). The kvm scheduler exploits as many threads of the core
as needed by guest. Further, starting POWER8, the processor allows splitting
a physical core into multiple subcores with 2 or 4 threads each. Again, only
the primary thread in a subcore is online in the host. The KVM-PPC
scheduler allows guests to exploit all the offline threads in the subcore,
by bringing them online when needed.
(Kernel patches on split-core 
http://www.spinics.net/lists/kvm-ppc/msg09121.html)

Recently with dynamic micro-threading changes in ppc-kvm, makes sure
to utilize all the offline cpus across guests, and across guests with
different cpu topologies.
(https://www.mail-archive.com/kvm@vger.kernel.org/msg115978.html)

Since the offline cpus are brought online in the guest context, it is safe
to count them as online. Nodeinfo today discounts these offline cpus from
cpu count/topology calclulation, and the nodeinfo output is not of any help
and the host appears overcommited when it is actually not.

The patch carefully counts those offline threads whose primary threads are
online. The host topology displayed by the nodeinfo is also fixed when the
host is in valid kvm state.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Andrea Bolognani 
---
 src/libvirt_private.syms |   1 +
 src/nodeinfo.c   | 156 +--
 src/nodeinfo.h   |   1 +
 3 files changed, 154 insertions(+), 4 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ff322d6..0517c24 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1010,6 +1010,7 @@ nodeGetMemoryParameters;
 nodeGetMemoryStats;
 nodeGetOnlineCPUBitmap;
 nodeGetPresentCPUBitmap;
+nodeGetThreadsPerSubcore;
 nodeSetMemoryParameters;
 
 
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index ba633a1..9ede6dc 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -31,6 +31,12 @@
 #include 
 #include 
 #include "conf/domain_conf.h"
+#include 
+#include 
+
+#if HAVE_LINUX_KVM_H
+# include 
+#endif
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
 # include 
@@ -392,13 +398,14 @@ virNodeParseSocket(const char *dir,
  * filling arguments */
 static int
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3)
-ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5)
-ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(7)
-ATTRIBUTE_NONNULL(8)
+ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(6)
+ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(8)
+ATTRIBUTE_NONNULL(9)
 virNodeParseNode(const char *node,
  virArch arch,
  virBitmapPtr present_cpus_map,
  virBitmapPtr online_cpus_map,
+ int threads_per_subcore,
  int *sockets,
  int *cores,
  int *threads,
@@ -492,7 +499,18 @@ virNodeParseNode(const char *node,
 continue;
 
 if (!virBitmapIsBitSet(online_cpus_map, cpu)) {
-(*offline)++;
+if (threads_per_subcore > 0 &&
+cpu % threads_per_subcore != 0 &&
+virBitmapIsBitSet(online_cpus_map,
+  cpu - (cpu % threads_per_subcore))) {
+/* Secondary offline threads are counted as online when
+ * subcores are in use and the corresponding primary
+ * thread is online */
+processors++;
+} else {
+/* But they are counted as offline otherwise */
+(*offline)++;
+}
 continue;
 }
 
@@ -545,6 +563,12 @@ virNodeParseNode(const char *node,
 *cores = core;
 }
 
+if (threads_per_subcore > 0) {
+/* The thread count ignores offline threads, which means that only
+ * only primary threads have been considered so far. If subcores
+ * are in use, we need to also account for secondary threads */
+*threads *= threads_per_subcore;
+}
 ret = processors;
 
  cleanup:
@@ -563,6 +587,48 @@ virNodeParseNode(const char *node,
 return ret;
 }
 
+/* Check whether the host subcore configuration is valid.
+ *
+ * A valid configuration is one where no secondary thread is online;
+ * the primary thread in a subcore is always the first one */
+static bool
+nodeHasValidSubcoreConfiguration(const char *sysfs_prefix,
+ int threads_per_subcore)
+{
+virBitmapPtr online_cpus = NULL;
+int nonline_cpus;
+int cpu;
+bool ret = false;
+
+/* No point in checking if subcores are not in use */
+if (threads

[libvirt] [PATCH v10 0/5] nodeinfo: Add support for subcores

[Andrea Bolognani]

Only patch 1/5 has been updated, all the other patches
are the same as v8:

  2/5 https://www.redhat.com/archives/libvir-list/2015-July/msg01045.html
  3/5 https://www.redhat.com/archives/libvir-list/2015-July/msg01048.html
  4/5 https://www.redhat.com/archives/libvir-list/2015-July/msg01049.html
  5/5 https://www.redhat.com/archives/libvir-list/2015-July/msg01050.html

I'm including the full history below.

Cheers.


Changes in v10:

  * don't attempt to close a file that wasn't opened

  * report a detailed error to help with diagnosis

Changes in v9:

  * take into account the fact that kvm might not be loaded
or even installed

Changes in v8:

  * shortened test names so that make dist doesn't
stop working again

Changes in v7:

  * rebased on top of master now that the series this one
builds on have been merged

Changes in v6:

  * updated to work on top of

  [PATCH v2 00/10] nodeinfo: Various cleanups

Changes in v5:

  * streamlined the logic used to decide whether the subcore
configuration is valid and moved it to a separate function

  * split the tests into separate commits for easier review and
to hopefully avoid having trouble with the list due to the
message size

Changes in v4:

  * removed a printf() statement

  * fixed typo in a commit message

Changes in v3:

  * the function to get the number of threads per subcore
has been moved to the from virarch.c, which deals with
architecture names only and is therefore not the right
place to read host configuration, to nodeinfo.c where
the rest of this stuff lives

  * said function has also been given a shorter name

  * the "valid subcore mode" boolean has been removed:
threads_per_subcore will be a positive number if
subcores should be taken into account, and if that's
not the case (x86 host, tainted configuration) it
will simply be zero, so now the code needs to keep
track of a single variable instead of two

  * the test case has been renamed to be more
descriptive

  * the test data has been cleaned up by removing all
cpu/cpu*/node* links, which prevented 'make dist'
from working due to recursive linking


Andrea Bolognani (3):
  tests: Add subcores1 nodeinfo test
  tests: Add subcores2 nodeinfo test
  tests: Add subcores3 nodeinfo test

Shivaprasad G Bhat (2):
  nodeinfo: Fix output on PPC64 KVM hosts
  tests: Prepare for subcore tests

 src/libvirt_private.syms   |   1 +
 src/nodeinfo.c | 159 -
 src/nodeinfo.h |   1 +
 tests/Makefile.am  |   6 +
 [...]
 tests/nodeinfomock.c   |  35 +
 tests/nodeinfotest.c   |   8 +-
 1348 files changed, 2140 insertions(+), 6 deletions(-)
 [...]
 create mode 100644 tests/nodeinfomock.c

-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v10 1/5] nodeinfo: Fix output on PPC64 KVM hosts

[Andrea Bolognani]

From: Shivaprasad G Bhat 

The nodeinfo is reporting incorrect number of cpus and incorrect host
topology on PPC64 KVM hosts. The KVM hypervisor on PPC64 needs only
the primary thread in a core to be online, and the secondaries offlined.
While scheduling a guest in, the kvm scheduler wakes up the secondaries to
run in guest context.

The host scheduling of the guests happen at the core level(as only primary
thread is online). The kvm scheduler exploits as many threads of the core
as needed by guest. Further, starting POWER8, the processor allows splitting
a physical core into multiple subcores with 2 or 4 threads each. Again, only
the primary thread in a subcore is online in the host. The KVM-PPC
scheduler allows guests to exploit all the offline threads in the subcore,
by bringing them online when needed.
(Kernel patches on split-core 
http://www.spinics.net/lists/kvm-ppc/msg09121.html)

Recently with dynamic micro-threading changes in ppc-kvm, makes sure
to utilize all the offline cpus across guests, and across guests with
different cpu topologies.
(https://www.mail-archive.com/kvm@vger.kernel.org/msg115978.html)

Since the offline cpus are brought online in the guest context, it is safe
to count them as online. Nodeinfo today discounts these offline cpus from
cpu count/topology calclulation, and the nodeinfo output is not of any help
and the host appears overcommited when it is actually not.

The patch carefully counts those offline threads whose primary threads are
online. The host topology displayed by the nodeinfo is also fixed when the
host is in valid kvm state.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Andrea Bolognani 
---
 src/libvirt_private.syms |   1 +
 src/nodeinfo.c   | 159 +--
 src/nodeinfo.h   |   1 +
 3 files changed, 157 insertions(+), 4 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ff322d6..0517c24 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1010,6 +1010,7 @@ nodeGetMemoryParameters;
 nodeGetMemoryStats;
 nodeGetOnlineCPUBitmap;
 nodeGetPresentCPUBitmap;
+nodeGetThreadsPerSubcore;
 nodeSetMemoryParameters;
 
 
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index ba633a1..3300439 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -31,6 +31,12 @@
 #include 
 #include 
 #include "conf/domain_conf.h"
+#include 
+#include 
+
+#if HAVE_LINUX_KVM_H
+# include 
+#endif
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
 # include 
@@ -392,13 +398,14 @@ virNodeParseSocket(const char *dir,
  * filling arguments */
 static int
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3)
-ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5)
-ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(7)
-ATTRIBUTE_NONNULL(8)
+ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(6)
+ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(8)
+ATTRIBUTE_NONNULL(9)
 virNodeParseNode(const char *node,
  virArch arch,
  virBitmapPtr present_cpus_map,
  virBitmapPtr online_cpus_map,
+ int threads_per_subcore,
  int *sockets,
  int *cores,
  int *threads,
@@ -492,7 +499,18 @@ virNodeParseNode(const char *node,
 continue;
 
 if (!virBitmapIsBitSet(online_cpus_map, cpu)) {
-(*offline)++;
+if (threads_per_subcore > 0 &&
+cpu % threads_per_subcore != 0 &&
+virBitmapIsBitSet(online_cpus_map,
+  cpu - (cpu % threads_per_subcore))) {
+/* Secondary offline threads are counted as online when
+ * subcores are in use and the corresponding primary
+ * thread is online */
+processors++;
+} else {
+/* But they are counted as offline otherwise */
+(*offline)++;
+}
 continue;
 }
 
@@ -545,6 +563,12 @@ virNodeParseNode(const char *node,
 *cores = core;
 }
 
+if (threads_per_subcore > 0) {
+/* The thread count ignores offline threads, which means that only
+ * only primary threads have been considered so far. If subcores
+ * are in use, we need to also account for secondary threads */
+*threads *= threads_per_subcore;
+}
 ret = processors;
 
  cleanup:
@@ -563,6 +587,48 @@ virNodeParseNode(const char *node,
 return ret;
 }
 
+/* Check whether the host subcore configuration is valid.
+ *
+ * A valid configuration is one where no secondary thread is online;
+ * the primary thread in a subcore is always the first one */
+static bool
+nodeHasValidSubcoreConfiguration(const char *sysfs_prefix,
+ int threads_per_subcore)
+{
+virBitmapPtr online_cpus = NULL;
+int nonline_cpus;
+int cpu;
+bool ret = false;
+
+/* No point in checking if subcores are not in use */
+if (threads

Re: [libvirt] [PATCH 0/2] Disable floppy disk for PowerPC VMs

[Andrea Bolognani]

On Fri, 2015-07-24 at 15:29 -0400, Kothapally Madhu Pavan wrote:
> This is an attempt to fix:
> Libvirt BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1180486
> 
> Libvirt currently assumes ISA_based floppy disks to be available
> across all
> architectures and machine types. However, PowerPC Book 3S compatible
> ('ie pseries)
> virtual machines do not support Floppy disks.
> 
> This patch series prevents libvirt from launching ppc64[le] -based
> 'pseries'
> VMs with floppy devices.

Hi,

sorry for not replying right away.

I started looking into that bug a while ago but I got sidetracked
shortly afterwards, so I don't have much to show for it. I'd like
to share my opinion on the matter anyway.

I believe you're basically following the right approach, eg. avoid
setting floppy-related capabilities and erroring out afterwards if
attempts are made to use devices that require such capabilites.

However, I think the implementation should be a little more
generic: ideally, the code would contain no references to the
ppc64 architecture and whether or not floppy disks are be allowed
would be determined by probing the QEMU binary.

Writing the code this way would allow us to handle automatically
other architectures where floppy disks do not make sense[1] and
situations where floppy support is not available[2].

If that turns out not to be possible or practical, at the very
least the check should be performed in one single spot, and not
replicated a dozen times thorough the library.

Cheers.


[1] I expect ARM to be one such architecture
[2] Such as stripped down / hardened QEMU binaries
-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: fix no error when pass a count <= 0 to setvcpus

[Andrea Bolognani]

On Thu, 2015-07-30 at 10:57 +0800, Luyao Huang wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1248277
> 
> When count <= 0, the client exit without set an error.
> 
> Signed-off-by: Luyao Huang 
> ---
>  tools/virsh-domain.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index f7edeeb..b6da684 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -6744,8 +6744,12 @@ cmdSetvcpus(vshControl *ctl, const vshCmd 
> *cmd)
>  if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
>  return false;
>  
> -if (vshCommandOptInt(ctl, cmd, "count", &count) < 0 || count <= 
> 0)
> +if (vshCommandOptInt(ctl, cmd, "count", &count) < 0)
>  goto cleanup;
> +if (count <= 0) {
> +vshError(ctl, _("Invalid value '%d' for number of virtual 
> CPUs"), count);
> +goto cleanup;
> +}
>  
>  /* none of the options were specified */
>  if (!current && flags == 0) {

Nice catch, but I would go for the following diff instead:

-8<-
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a61656f..4b8ebbd 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6819,7 +6819,7 @@ static bool
 cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
 {
 virDomainPtr dom;
-int count = 0;
+unsigned int count = 0;
 bool ret = false;
 bool maximum = vshCommandOptBool(cmd, "maximum");
 bool config = vshCommandOptBool(cmd, "config");
@@ -6846,8 +6846,15 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
 if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
 return false;
 
-if (vshCommandOptInt(ctl, cmd, "count", &count) < 0 || count <= 0)
+if (vshCommandOptUInt(ctl, cmd, "count", &count) < 0)
+goto cleanup;
+
+if (count == 0) {
+vshError(ctl,
+ _("Numeric value '%s' for <%s> option is malformed or
out of range"),
+ "0", "count");
 goto cleanup;
+}
 
 /* none of the options were specified */
 if (!current && flags == 0) {
----->8-----

It's slightly more awkward, but his way we make sure the
error message is the same whether the value used for the
count option is "foo", "0" or "-20". vshCommandOptUInt()
already uses that error message internally.

Does it look good to you?

Cheers.


-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: fix no error when pass a count <= 0 to setvcpus

[Andrea Bolognani]

On Thu, 2015-07-30 at 09:45 +0200, Peter Krempa wrote:
> 
> Since we know that here we are trying to set 0 cpus which is wrong we
> can also say that explicitly in the error message rather than copying
> the generic one.

Using the generic one means we don't have to add yet another
translatable string, though. Or did you mean to use a
completely different error message?

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: fix no error when pass a count <= 0 to setvcpus

[Andrea Bolognani]

On Thu, 2015-07-30 at 10:05 +0200, Peter Krempa wrote:
> On Thu, Jul 30, 2015 at 09:51:15 +0200, Andrea Bolognani wrote:
> > On Thu, 2015-07-30 at 09:45 +0200, Peter Krempa wrote:
> > > 
> > > Since we know that here we are trying to set 0 cpus which is 
> > > wrong we
> > > can also say that explicitly in the error message rather than 
> > > copying
> > > the generic one.
> > 
> > Using the generic one means we don't have to add yet another
> > translatable string, though. Or did you mean to use a
> > completely different error message?
> 
> Completely different. Something along: "Can't set 0 processors for a 
> VM"

I see. I don't feel strongly either way, I just want to avoid
having a bunch of very similar but non-identical messages; if
you feel a custom error message is warranted in this situation
then go right ahead.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v11 0/5] nodeinfo: Add support for subcores

[Andrea Bolognani]

Only patch 1/5 has been updated, all the other patches
are the same as v8:

  2/5 https://www.redhat.com/archives/libvir-list/2015-July/msg01045.html
  3/5 https://www.redhat.com/archives/libvir-list/2015-July/msg01048.html
  4/5 https://www.redhat.com/archives/libvir-list/2015-July/msg01049.html
  5/5 https://www.redhat.com/archives/libvir-list/2015-July/msg01050.html

I'm including the full history below.

Cheers.


Changes in v11:

  * don't declare variables only used inside a code block
guarded by #ifdef outside of said code block

  * use virBitmapNextSetBit() instead of going through
all possible index values and calling
virBitmapIsBitSet() for each one

Changes in v10:

  * don't attempt to close a file that wasn't opened

  * report a detailed error to help with diagnosis

Changes in v9:

  * take into account the fact that kvm might not be loaded
or even installed

Changes in v8:

  * shortened test names so that make dist doesn't
stop working again

Changes in v7:

  * rebased on top of master now that the series this one
builds on have been merged

Changes in v6:

  * updated to work on top of

  [PATCH v2 00/10] nodeinfo: Various cleanups

Changes in v5:

  * streamlined the logic used to decide whether the subcore
configuration is valid and moved it to a separate function

  * split the tests into separate commits for easier review and
to hopefully avoid having trouble with the list due to the
message size

Changes in v4:

  * removed a printf() statement

  * fixed typo in a commit message

Changes in v3:

  * the function to get the number of threads per subcore
has been moved to the from virarch.c, which deals with
architecture names only and is therefore not the right
place to read host configuration, to nodeinfo.c where
the rest of this stuff lives

  * said function has also been given a shorter name

  * the "valid subcore mode" boolean has been removed:
threads_per_subcore will be a positive number if
subcores should be taken into account, and if that's
not the case (x86 host, tainted configuration) it
will simply be zero, so now the code needs to keep
track of a single variable instead of two

  * the test case has been renamed to be more
descriptive

  * the test data has been cleaned up by removing all
cpu/cpu*/node* links, which prevented 'make dist'
    from working due to recursive linking


Andrea Bolognani (3):
  tests: Add subcores1 nodeinfo test
  tests: Add subcores2 nodeinfo test
  tests: Add subcores3 nodeinfo test

Shivaprasad G Bhat (2):
  nodeinfo: Fix output on PPC64 KVM hosts
  tests: Prepare for subcore tests

 src/libvirt_private.syms   |   1 +
 src/nodeinfo.c | 153 -
 src/nodeinfo.h |   1 +
 tests/Makefile.am  |   6 +
 [...]
 tests/nodeinfomock.c   |  35 +
 tests/nodeinfotest.c   |   8 +-
 1348 files changed, 2134 insertions(+), 6 deletions(-)
 [...]
 create mode 100644 tests/nodeinfomock.c

-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v11 1/5] nodeinfo: Fix output on PPC64 KVM hosts

[Andrea Bolognani]

From: Shivaprasad G Bhat 

The nodeinfo is reporting incorrect number of cpus and incorrect host
topology on PPC64 KVM hosts. The KVM hypervisor on PPC64 needs only
the primary thread in a core to be online, and the secondaries offlined.
While scheduling a guest in, the kvm scheduler wakes up the secondaries to
run in guest context.

The host scheduling of the guests happen at the core level(as only primary
thread is online). The kvm scheduler exploits as many threads of the core
as needed by guest. Further, starting POWER8, the processor allows splitting
a physical core into multiple subcores with 2 or 4 threads each. Again, only
the primary thread in a subcore is online in the host. The KVM-PPC
scheduler allows guests to exploit all the offline threads in the subcore,
by bringing them online when needed.
(Kernel patches on split-core 
http://www.spinics.net/lists/kvm-ppc/msg09121.html)

Recently with dynamic micro-threading changes in ppc-kvm, makes sure
to utilize all the offline cpus across guests, and across guests with
different cpu topologies.
(https://www.mail-archive.com/kvm@vger.kernel.org/msg115978.html)

Since the offline cpus are brought online in the guest context, it is safe
to count them as online. Nodeinfo today discounts these offline cpus from
cpu count/topology calclulation, and the nodeinfo output is not of any help
and the host appears overcommited when it is actually not.

The patch carefully counts those offline threads whose primary threads are
online. The host topology displayed by the nodeinfo is also fixed when the
host is in valid kvm state.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Andrea Bolognani 
---
 src/libvirt_private.syms |   1 +
 src/nodeinfo.c   | 153 +--
 src/nodeinfo.h   |   1 +
 3 files changed, 151 insertions(+), 4 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ff322d6..0517c24 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1010,6 +1010,7 @@ nodeGetMemoryParameters;
 nodeGetMemoryStats;
 nodeGetOnlineCPUBitmap;
 nodeGetPresentCPUBitmap;
+nodeGetThreadsPerSubcore;
 nodeSetMemoryParameters;
 
 
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index ba633a1..7da055c 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -31,6 +31,12 @@
 #include 
 #include 
 #include "conf/domain_conf.h"
+#include 
+#include 
+
+#if HAVE_LINUX_KVM_H
+# include 
+#endif
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
 # include 
@@ -392,13 +398,14 @@ virNodeParseSocket(const char *dir,
  * filling arguments */
 static int
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3)
-ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5)
-ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(7)
-ATTRIBUTE_NONNULL(8)
+ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(6)
+ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(8)
+ATTRIBUTE_NONNULL(9)
 virNodeParseNode(const char *node,
  virArch arch,
  virBitmapPtr present_cpus_map,
  virBitmapPtr online_cpus_map,
+ int threads_per_subcore,
  int *sockets,
  int *cores,
  int *threads,
@@ -492,7 +499,18 @@ virNodeParseNode(const char *node,
 continue;
 
 if (!virBitmapIsBitSet(online_cpus_map, cpu)) {
-(*offline)++;
+if (threads_per_subcore > 0 &&
+cpu % threads_per_subcore != 0 &&
+virBitmapIsBitSet(online_cpus_map,
+  cpu - (cpu % threads_per_subcore))) {
+/* Secondary offline threads are counted as online when
+ * subcores are in use and the corresponding primary
+ * thread is online */
+processors++;
+} else {
+/* But they are counted as offline otherwise */
+(*offline)++;
+}
 continue;
 }
 
@@ -545,6 +563,12 @@ virNodeParseNode(const char *node,
 *cores = core;
 }
 
+if (threads_per_subcore > 0) {
+/* The thread count ignores offline threads, which means that only
+ * only primary threads have been considered so far. If subcores
+ * are in use, we need to also account for secondary threads */
+*threads *= threads_per_subcore;
+}
 ret = processors;
 
  cleanup:
@@ -563,6 +587,41 @@ virNodeParseNode(const char *node,
 return ret;
 }
 
+/* Check whether the host subcore configuration is valid.
+ *
+ * A valid configuration is one where no secondary thread is online;
+ * the primary thread in a subcore is always the first one */
+static bool
+nodeHasValidSubcoreConfiguration(const char *sysfs_prefix,
+ int threads_per_subcore)
+{
+virBitmapPtr online_cpus = NULL;
+int cpu = -1;
+bool ret = false;
+
+/* No point in checking if subcores are not in use */
+if (threads_per_sub

Re: [libvirt] [PATCH v10 1/5] nodeinfo: Fix output on PPC64 KVM hosts

[Andrea Bolognani]

On Wed, 2015-07-29 at 22:30 -0400, John Ferlan wrote:
> 
> > +online_cpus = nodeGetOnlineCPUBitmap(sysfs_prefix);
> > +if (online_cpus == NULL)
> > +goto cleanup;
> > +
> > +nonline_cpus = virBitmapSize(online_cpus);
> > +
> > +for (cpu = 0; cpu < nonline_cpus; cpu++) {
> > +
> > +/* Skip primary threads */
> > +if (cpu % threads_per_subcore == 0)
> > +continue;
> > +
> > +/* A single online subthread is enough to make the
> > + * configuration invalid */
> > +if (virBitmapIsBitSet(online_cpus, cpu))
> > +goto cleanup;
> > +}
> 
> Could virBitmapNextSetBit be used?  Where if the returned pos (cpu) %
> threads_per_subcore != 0, then jump to cleanup?
> 
> I think both work, I just didn't know how large nonline_cpus could be
> and since the bitmap code has a way to return 'next' bit set, we 
> could
> use it.

It works, and it looks nicer too! Thanks for the tip :)

> > +int
> > +nodeGetThreadsPerSubcore(virArch arch)
> > +{
> > +const char *kvmpath = "/dev/kvm";
> > +int kvmfd;
> 
> These could move inside the if below.
> 
> I'm thinking of one particular architecture where we consistently get
> compilation errors when there's variables that aren't used (which 
> would
> be the case if HAVE_LINUX_KVM_H or KVM_CAP_PPC_SMT were not true

Done.

> > +if ((kvmfd = open(kvmpath, O_RDONLY)) < 0) {
> > +/* This can happen when running as a regular user if
> > + * permissions are tight enough, in which case 
> > erroring out
> > + * is better than silently falling back and reporting
> > + * different nodeinfo depending on the user */
> > +virReportSystemError(errno,
> > + _("Failed to open '%s'"),
> > + kvmpath);
> > +threads_per_subcore = kvmfd;
> 
> I would just set to -1 here directly rather than the return failure 
> from
> open

Done.

> Beyond those - things look OK to me.

Just to be sure, did you check the rest of the series as well?

> Let me know if you want to make
> changes... Probably should wait until DV generates the release before
> pushing though...

I've just sent out v11 which addresses all your remarks. It
should definitely only be pushed once the new release is out.

Thanks for the review :)

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/2] qemu: Improve memory alignment handling and fix upcoming powerpc

[Andrea Bolognani]

On Fri, 2015-07-31 at 16:49 +0200, Peter Krempa wrote:
> 
> Peter Krempa (2):
>   qemu: Make memory alignment helper more universal
>   qemu: ppc64: Align memory sizes to 256MiB
> 
>  src/qemu/qemu_domain.c | 33 
> --
>  src/qemu/qemu_domain.h |  3 +-
>  src/qemu/qemu_hotplug.c|  4 +--
>  .../qemuxml2argv-pseries-cpu-compat.args   |  2 +-
>  4 files changed, 30 insertions(+), 12 deletions(-)

Looks okay overall and works as expected from my limited
testing on ppc64.

One thing, though: the domain XML is not updated with the
rounded-up value, which I think might be a little bit
confusing to the user. Any chance the rounding up could
be reflected there as well?

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/2] qemu: Improve memory alignment handling and fix upcoming powerpc

[Andrea Bolognani]

On Mon, 2015-08-03 at 15:13 +0200, Peter Krempa wrote:
> 
> > Looks okay overall and works as expected from my limited
> > testing on ppc64.
> > 
> > One thing, though: the domain XML is not updated with the
> > rounded-up value, which I think might be a little bit
> > confusing to the user. Any chance the rounding up could
> > be reflected there as well?
> 
> The rounding will be reflected once you start the VM in the live XML.
> This is also the case with the 1MiB rounding that is currently used, 
> but
> it's not as prominent as the 256MiB granularity.

Let's file updating the on-disk XML under "would be nice to
have" then.

ACK series.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] nodeinfo: Fix build failure when KVM headers are not available

[Andrea Bolognani]

Compiler error:

  ../../src/nodeinfo.c: In function 'nodeGetThreadsPerSubcore':
  ../../src/nodeinfo.c:2393: error: label 'out' defined but not used 
[-Wunused-label]
  ../../src/nodeinfo.c:2352: error: unused parameter 'arch' [-Wunused-parameter]
---

 src/nodeinfo.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 7da055c..6ccada5 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -2341,6 +2341,8 @@ nodeAllocPages(unsigned int npages,
 return ret;
 }
 
+#if HAVE_LINUX_KVM_H && defined(KVM_CAP_PPC_SMT)
+
 /* Get the number of threads per subcore.
  *
  * This will be 2, 4 or 8 on POWER hosts, depending on the current
@@ -2352,8 +2354,6 @@ int
 nodeGetThreadsPerSubcore(virArch arch)
 {
 int threads_per_subcore = 0;
-
-#if HAVE_LINUX_KVM_H && defined(KVM_CAP_PPC_SMT)
 const char *kvmpath = "/dev/kvm";
 int kvmfd;
 
@@ -2388,8 +2388,19 @@ nodeGetThreadsPerSubcore(virArch arch)
 
 VIR_FORCE_CLOSE(kvmfd);
 }
-#endif /* HAVE_LINUX_KVM_H && defined(KVM_CAP_PPC_SMT) */
 
  out:
 return threads_per_subcore;
 }
+
+#else
+
+/* Fallback for nodeGetThreadsPerSubcore() used when KVM headers
+ * are not available on the system */
+int
+nodeGetThreadsPerSubcore(virArch arch ATTRIBUTE_UNUSED)
+{
+return 0;
+}
+
+#endif /* HAVE_LINUX_KVM_H && defined(KVM_CAP_PPC_SMT) */
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v11 0/5] nodeinfo: Add support for subcores

[Andrea Bolognani]

On Mon, 2015-08-03 at 09:27 -0400, John Ferlan wrote:
> 
> Pulled in 2-5 from v8 series, rebuilt, checked, and pushed.

The new code caused the build to fail[1] if the KVM headers
were not available on the system, but Martin has already
pushed the fix so we're good now.

Thanks.


[1] https://ci.centos.org/view/libvirt-project/job/libvirt-daemon-build
/478/systems=libvirt-centos-6/
-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 12/18] cpu: Align ppc64 CPU data with x86

[Andrea Bolognani]

Use a typedef instead of the plain struct and heap allocation. This
will make it easier to extend the ppc64 specific CPU data later on.
---
 src/cpu/cpu.h|  2 +-
 src/cpu/cpu_ppc64.c  | 81 ++--
 src/cpu/cpu_ppc64_data.h |  3 +-
 3 files changed, 67 insertions(+), 19 deletions(-)

diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 49d4226..7375876 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -38,7 +38,7 @@ struct _virCPUData {
 virArch arch;
 union {
 virCPUx86Data *x86;
-struct cpuPPC64Data ppc64;
+virCPUppc64Data *ppc64;
 /* generic driver needs no data */
 } data;
 };
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 4428a55..d186d4c 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -48,7 +48,7 @@ struct ppc64_vendor {
 struct ppc64_model {
 char *name;
 const struct ppc64_vendor *vendor;
-struct cpuPPC64Data data;
+virCPUppc64Data *data;
 struct ppc64_model *next;
 };
 
@@ -58,6 +58,31 @@ struct ppc64_map {
 };
 
 static void
+ppc64DataFree(virCPUppc64Data *data)
+{
+if (!data)
+return;
+
+VIR_FREE(data);
+}
+
+static virCPUppc64Data *
+ppc64DataCopy(virCPUppc64Data *data)
+{
+virCPUppc64Data *copy;
+
+if (!data)
+return NULL;
+
+if (VIR_ALLOC(copy) < 0)
+return NULL;
+
+copy->pvr = data->pvr;
+
+return copy;
+}
+
+static void
 ppc64VendorFree(struct ppc64_vendor *vendor)
 {
 if (!vendor)
@@ -90,6 +115,7 @@ ppc64ModelFree(struct ppc64_model *model)
 if (!model)
 return;
 
+ppc64DataFree(model->data);
 VIR_FREE(model->name);
 VIR_FREE(model);
 }
@@ -102,16 +128,22 @@ ppc64ModelCopy(const struct ppc64_model *model)
 if (!model)
 return NULL;
 
-if (VIR_ALLOC(copy) < 0 ||
-VIR_STRDUP(copy->name, model->name) < 0) {
-ppc64ModelFree(copy);
-return NULL;
-}
+if (VIR_ALLOC(copy) < 0)
+goto error;
+
+if (VIR_STRDUP(copy->name, model->name) < 0)
+goto error;
+
+if (!(copy->data = ppc64DataCopy(model->data)))
+goto error;
 
-copy->data.pvr = model->data.pvr;
 copy->vendor = model->vendor;
 
 return copy;
+
+ error:
+ppc64ModelFree(copy);
+return NULL;
 }
 
 static struct ppc64_model *
@@ -139,7 +171,7 @@ ppc64ModelFindPVR(const struct ppc64_map *map,
 
 model = map->models;
 while (model) {
-if (model->data.pvr == pvr)
+if (model->data->pvr == pvr)
 return model;
 
 model = model->next;
@@ -248,6 +280,11 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
 if (VIR_ALLOC(model) < 0)
 return -1;
 
+if (VIR_ALLOC(model->data) < 0) {
+ppc64ModelFree(model);
+return -1;
+}
+
 model->name = virXPathString("string(@name)", ctxt);
 if (!model->name) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -285,7 +322,7 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
model->name);
 goto ignore;
 }
-model->data.pvr = pvr;
+model->data->pvr = pvr;
 
 if (!map->models) {
 map->models = model;
@@ -329,7 +366,7 @@ ppc64LoadMap(void)
 struct ppc64_map *map;
 
 if (VIR_ALLOC(map) < 0)
-return NULL;
+goto error;
 
 if (cpuMapLoad("ppc64", ppc64MapLoadCallback, map) < 0)
 goto error;
@@ -343,7 +380,7 @@ ppc64LoadMap(void)
 
 static virCPUDataPtr
 ppc64MakeCPUData(virArch arch,
- struct cpuPPC64Data *data)
+ virCPUppc64Data *data)
 {
 virCPUDataPtr cpuData;
 
@@ -351,7 +388,9 @@ ppc64MakeCPUData(virArch arch,
 return NULL;
 
 cpuData->arch = arch;
-cpuData->data.ppc64 = *data;
+
+if (!(cpuData->data.ppc64 = ppc64DataCopy(data)))
+VIR_FREE(cpuData);
 
 return cpuData;
 }
@@ -432,7 +471,7 @@ ppc64Compute(virCPUDefPtr host,
 }
 
 if (guestData)
-if (!(*guestData = ppc64MakeCPUData(arch, &guest_model->data)))
+if (!(*guestData = ppc64MakeCPUData(arch, guest_model->data)))
 goto cleanup;
 
 ret = VIR_CPU_COMPARE_IDENTICAL;
@@ -484,10 +523,10 @@ ppc64DriverDecode(virCPUDefPtr cpu,
 if (!data || !(map = ppc64LoadMap()))
 return -1;
 
-if (!(model = ppc64ModelFindPVR(map, data->data.ppc64.pvr))) {
+if (!(model = ppc64ModelFindPVR(map, data->data.ppc64->pvr))) {
 virReportError(VIR_ERR_OPERATION_FAILED,
_("Cannot find CPU model with PVR 0x%08x"),
-   data->data.ppc64.pvr);
+   data->data.ppc64->pvr);
 goto cleanup;
 }
 
@@ -517,6 +556,7 @@ ppc64DriverFree(virCPUDataPtr data)
 if (!data)
 return;
 
+ppc64DataFree(data->data.ppc64);
 VIR_FREE(data);
 }
 
@@ -526,16 +566,23 @@ ppc64DriverNodeData(virArch arch)
 virCPUDataPtr cpuData;
 
 if (VIR_ALLOC(cpuData) < 0)
-return NULL;
+goto error;
+
+if (VIR_ALLO

[libvirt] [PATCH 07/18] tests: Remove unused file

[Andrea Bolognani]

---
 tests/cputestdata/ppc64-baseline-1-result.xml | 3 ---
 1 file changed, 3 deletions(-)
 delete mode 100644 tests/cputestdata/ppc64-baseline-1-result.xml

diff --git a/tests/cputestdata/ppc64-baseline-1-result.xml 
b/tests/cputestdata/ppc64-baseline-1-result.xml
deleted file mode 100644
index cbdd9bc..000
--- a/tests/cputestdata/ppc64-baseline-1-result.xml
+++ /dev/null
@@ -1,3 +0,0 @@
-
-  POWER7+_v2.1
-
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 02/18] cpu: Simplify NULL handling in ppc64 driver

[Andrea Bolognani]

Use briefer checks, eg. (!model) instead of (model == NULL), and
avoid initializing to NULL a pointer that would be assigned in
the first line of the function anyway.

Also remove a pointless NULL assignment.

No functional changes.
---
 src/cpu/cpu_ppc64.c | 33 -
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 5140297..05ff8f2 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -61,7 +61,7 @@ struct ppc64_map {
 static void
 ppc64ModelFree(struct ppc64_model *model)
 {
-if (model == NULL)
+if (!model)
 return;
 
 VIR_FREE(model->name);
@@ -75,7 +75,7 @@ ppc64ModelFind(const struct ppc64_map *map,
 struct ppc64_model *model;
 
 model = map->models;
-while (model != NULL) {
+while (model) {
 if (STREQ(model->name, name))
 return model;
 
@@ -92,7 +92,7 @@ ppc64ModelFindPVR(const struct ppc64_map *map,
 struct ppc64_model *model;
 
 model = map->models;
-while (model != NULL) {
+while (model) {
 if (model->data.pvr == pvr)
 return model;
 
@@ -158,15 +158,15 @@ static struct ppc64_model *
 ppc64ModelFromCPU(const virCPUDef *cpu,
   const struct ppc64_map *map)
 {
-struct ppc64_model *model = NULL;
+struct ppc64_model *model;
 
-if ((model = ppc64ModelFind(map, cpu->model)) == NULL) {
+if (!(model = ppc64ModelFind(map, cpu->model))) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown CPU model %s"), cpu->model);
 goto error;
 }
 
-if ((model = ppc64ModelCopy(model)) == NULL)
+if (!(model = ppc64ModelCopy(model)))
 goto error;
 
 return model;
@@ -181,7 +181,7 @@ static int
 ppc64VendorLoad(xmlXPathContextPtr ctxt,
 struct ppc64_map *map)
 {
-struct ppc64_vendor *vendor = NULL;
+struct ppc64_vendor *vendor;
 
 if (VIR_ALLOC(vendor) < 0)
 return -1;
@@ -264,7 +264,7 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
 }
 model->data.pvr = pvr;
 
-if (map->models == NULL) {
+if (!map->models) {
 map->models = model;
 } else {
 model->next = map->models;
@@ -303,16 +303,16 @@ ppc64MapLoadCallback(cpuMapElement element,
 static void
 ppc64MapFree(struct ppc64_map *map)
 {
-if (map == NULL)
+if (!map)
 return;
 
-while (map->models != NULL) {
+while (map->models) {
 struct ppc64_model *model = map->models;
 map->models = model->next;
 ppc64ModelFree(model);
 }
 
-while (map->vendors != NULL) {
+while (map->vendors) {
 struct ppc64_vendor *vendor = map->vendors;
 map->vendors = vendor->next;
 ppc64VendorFree(vendor);
@@ -350,7 +350,6 @@ ppc64MakeCPUData(virArch arch,
 
 cpuData->arch = arch;
 cpuData->data.ppc64 = *data;
-data = NULL;
 
 return cpuData;
 }
@@ -417,7 +416,7 @@ ppc64Compute(virCPUDefPtr host,
 !(guest_model = ppc64ModelFromCPU(cpu, map)))
 goto cleanup;
 
-if (guestData != NULL) {
+if (guestData) {
 if (cpu->type == VIR_CPU_TYPE_GUEST &&
 cpu->match == VIR_CPU_MATCH_STRICT &&
 STRNEQ(guest_model->name, host_model->name)) {
@@ -478,7 +477,7 @@ ppc64DriverDecode(virCPUDefPtr cpu,
 
 virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1);
 
-if (data == NULL || (map = ppc64LoadMap()) == NULL)
+if (!data || !(map = ppc64LoadMap()))
 return -1;
 
 if (!(model = ppc64ModelFindPVR(map, data->data.ppc64.pvr))) {
@@ -512,7 +511,7 @@ ppc64DriverDecode(virCPUDefPtr cpu,
 static void
 ppc64DriverFree(virCPUDataPtr data)
 {
-if (data == NULL)
+if (!data)
 return;
 
 VIR_FREE(data);
@@ -575,7 +574,7 @@ ppc64DriverBaseline(virCPUDefPtr *cpus,
 unsigned int nmodels ATTRIBUTE_UNUSED,
 unsigned int flags)
 {
-struct ppc64_map *map = NULL;
+struct ppc64_map *map;
 const struct ppc64_model *model;
 const struct ppc64_vendor *vendor = NULL;
 virCPUDefPtr cpu = NULL;
@@ -667,7 +666,7 @@ ppc64DriverGetModels(char ***models)
 goto error;
 
 model = map->models;
-while (model != NULL) {
+while (model) {
 if (models) {
 if (VIR_STRDUP(name, model->name) < 0)
 goto error;
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 14/18] cpu: Simplify ppc64 part of CPU map XML

[Andrea Bolognani]

Use multiple PVRs per CPU model to reduce the number of models we
need to keep track of.

Remove specific CPU models (eg. POWER7+_v2.1): the corresponding
generic CPU model (eg. POWER7) should be used instead to ensure
the guest can be booted on any compatible host.

Get rid of all the entries that did not match any of the CPU
models supported by QEMU, like power8 and power8e.
---
 src/cpu/cpu_map.xml| 39 ++
 tests/cputest.c|  4 +--
 .../ppc64-baseline-incompatible-vendors.xml|  4 +--
 .../ppc64-baseline-no-vendor-result.xml|  2 +-
 tests/cputestdata/ppc64-baseline-no-vendor.xml |  2 +-
 tests/cputestdata/ppc64-exact.xml  |  2 +-
 tests/cputestdata/ppc64-guest-nofallback.xml   |  2 +-
 tests/cputestdata/ppc64-guest.xml  |  2 +-
 .../ppc64-host+guest,ppc_models-result.xml |  2 +-
 tests/cputestdata/ppc64-host.xml   |  2 +-
 tests/cputestdata/ppc64-strict.xml |  4 +--
 11 files changed, 16 insertions(+), 49 deletions(-)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 6387ce4..b3c4477 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -1358,53 +1358,20 @@
 
 
 
-
-  
-  
-
-
-
-  
-  
-
-
-
-  
-  
-
-
-
-  
-  
-
-
-
-  
-  
-
-
-
+
   
   
 
 
-
+
   
   
-
-
-
-  
   
 
 
-
+
   
   
-
-
-
-  
   
 
 
diff --git a/tests/cputest.c b/tests/cputest.c
index ec867a7..82999f8 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -493,7 +493,7 @@ static const char *model486[]   = { "486" };
 static const char *nomodel[]= { "nomodel" };
 static const char *models[] = { "qemu64", "core2duo", "Nehalem" };
 static const char *haswell[]= { "SandyBridge", "Haswell" };
-static const char *ppc_models[] = { "POWER7", "POWER7_v2.1", "POWER7_v2.3", 
"POWER8_v1.0"};
+static const char *ppc_models[] = { "POWER6", "POWER7", "POWER8" };
 
 static int
 mymain(void)
@@ -654,7 +654,7 @@ mymain(void)
   NULL, "Haswell-noTSX", 0);
 
 DO_TEST_GUESTDATA("ppc64", "host", "guest", ppc_models, NULL, 0);
-DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, 
"POWER7_v2.1", -1);
+DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, 
"POWER6", -1);
 
 return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
diff --git a/tests/cputestdata/ppc64-baseline-incompatible-vendors.xml 
b/tests/cputestdata/ppc64-baseline-incompatible-vendors.xml
index 97d3c9c..9e67e9d 100644
--- a/tests/cputestdata/ppc64-baseline-incompatible-vendors.xml
+++ b/tests/cputestdata/ppc64-baseline-incompatible-vendors.xml
@@ -1,13 +1,13 @@
 
 
   ppc64
-  POWER7+_v2.1
+  POWER7
   Intel
   
 
 
   ppc64
-  POWER8_v1.0
+  POWER8
   Intel
   
 
diff --git a/tests/cputestdata/ppc64-baseline-no-vendor-result.xml 
b/tests/cputestdata/ppc64-baseline-no-vendor-result.xml
index 36bae52..7fac4b7 100644
--- a/tests/cputestdata/ppc64-baseline-no-vendor-result.xml
+++ b/tests/cputestdata/ppc64-baseline-no-vendor-result.xml
@@ -1,3 +1,3 @@
 
-  POWER7_v2.3
+  POWER7
 
diff --git a/tests/cputestdata/ppc64-baseline-no-vendor.xml 
b/tests/cputestdata/ppc64-baseline-no-vendor.xml
index 5e69a62..6d8dd0d 100644
--- a/tests/cputestdata/ppc64-baseline-no-vendor.xml
+++ b/tests/cputestdata/ppc64-baseline-no-vendor.xml
@@ -1,7 +1,7 @@
 
 
   ppc64
-  POWER7_v2.3
+  POWER7
   
 
 
diff --git a/tests/cputestdata/ppc64-exact.xml 
b/tests/cputestdata/ppc64-exact.xml
index c84f16a..f416a59 100644
--- a/tests/cputestdata/ppc64-exact.xml
+++ b/tests/cputestdata/ppc64-exact.xml
@@ -1,3 +1,3 @@
 
-  POWER8_v1.0
+  POWER8
 
diff --git a/tests/cputestdata/ppc64-guest-nofallback.xml 
b/tests/cputestdata/ppc64-guest-nofallback.xml
index 42026b4..603a3ab 100644
--- a/tests/cputestdata/ppc64-guest-nofallback.xml
+++ b/tests/cputestdata/ppc64-guest-nofallback.xml
@@ -1,4 +1,4 @@
 
-  POWER7_v2.1
+  POWER6
   
 
diff --git a/tests/cputestdata/ppc64-guest.xml 
b/tests/cputestdata/ppc64-guest.xml
index 9e91501..210f96d 100644
--- a/tests/cputestdata/ppc64-guest.xml
+++ b/tests/cputestdata/ppc64-guest.xml
@@ -1,4 +1,4 @@
 
-  POWER7_v2.3
+  POWER7
   
 
diff --git a/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml 
b/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml
index 3e55f68..3548c0e 100644
--- a/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml
+++ b/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml
@@ -1,5 +1,5 @@
 
   ppc64
-  POWER7_v2.3
+  POWER7
   IBM
 
diff --git a/tests/cputestdata/ppc64-host.xml b/tests/cputestdata/ppc64-host.xml
index 39cb741..0ac5c4e 100644
--- a/tests/cputestdata/ppc64-host.xml
+++ b/tests/cputestdata/ppc64-host.xml
@@ -1,6 +1,6 @@
 
   ppc64
-  POWER7_v2.3
+  POWER7
   IBM
   
 
diff --git a/test

[libvirt] [PATCH 09/18] cpu: Don't skip CPU model name check in ppc64 driver

[Andrea Bolognani]

ppc64Compute(), called by cpuNodeData(), is used not only to retrieve
the driver-specific data associated to a guest CPU definition, but
also to check whether said guest CPU is compatible with the host CPU.

If the user is not interested in the CPU data, it's perfectly fine
to pass a NULL pointer instead of a return location, and the
compatibility data returned should not be affected by this. One of
the checks, specifically the one on CPU model name, was however
only performed if the return location was non-NULL.
---
 src/cpu/cpu_ppc64.c | 31 +++
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 20c68ca..633515f 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -418,26 +418,25 @@ ppc64Compute(virCPUDefPtr host,
 !(guest_model = ppc64ModelFromCPU(cpu, map)))
 goto cleanup;
 
-if (guestData) {
-if (cpu->type == VIR_CPU_TYPE_GUEST &&
-cpu->match == VIR_CPU_MATCH_STRICT &&
-STRNEQ(guest_model->name, host_model->name)) {
-VIR_DEBUG("host CPU model does not match required CPU model %s",
-  guest_model->name);
-if (message &&
-virAsprintf(message,
-_("host CPU model does not match required "
-"CPU model %s"),
-guest_model->name) < 0)
-goto cleanup;
-
-ret = VIR_CPU_COMPARE_INCOMPATIBLE;
+if (cpu->type == VIR_CPU_TYPE_GUEST &&
+cpu->match == VIR_CPU_MATCH_STRICT &&
+STRNEQ(guest_model->name, host_model->name)) {
+VIR_DEBUG("host CPU model does not match required CPU model %s",
+  guest_model->name);
+if (message &&
+virAsprintf(message,
+_("host CPU model does not match required "
+"CPU model %s"),
+guest_model->name) < 0)
 goto cleanup;
-}
 
+ret = VIR_CPU_COMPARE_INCOMPATIBLE;
+goto cleanup;
+}
+
+if (guestData)
 if (!(*guestData = ppc64MakeCPUData(arch, &guest_model->data)))
 goto cleanup;
-}
 
 ret = VIR_CPU_COMPARE_IDENTICAL;
 
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list