[libvirt] [PATCH 0/3] Use virNodeGetCPUMap where appropriate
This series concludes the introduction of the virNodeGetCPUMap API by replacing calls to virNodeGetInfo used only for the purpose of computing the maximum number of node CPUs (which has the potential to yield the incorrect number). Most prominently, with patch 3/3 the output of virsh vcpuinfo will now be correct for domains on hosts with offline CPUs Viktor Mihajlovski (3): qemu, lxc: Change host CPU detection logic. python: Use virNodeGetCPUMap where possible virsh: Use virNodeGetCPUMap if possible python/libvirt-override.c | 100 ++++++++++++++++++++++++++++++++++------------ src/lxc/lxc_controller.c | 8 ++-- src/qemu/qemu_driver.c | 12 ++---- src/qemu/qemu_process.c | 10 ++--- tools/virsh-domain.c | 36 +++++++++++------ 5 files changed, 108 insertions(+), 58 deletions(-) -- 1.7.12.4
The drivers for QEMU and LXC use virNodeGetInfo to determine the number of host CPUs. This approach can lead to a wrong (too small) number if one or more CPUs are offline. It is better to use virNodeGetCPUMap if available, which is the case here. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/lxc/lxc_controller.c | 8 +++----- src/qemu/qemu_driver.c | 12 +++--------- src/qemu/qemu_process.c | 10 ++++------ 3 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index a41c903..e9720bc 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -492,17 +492,15 @@ static int virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl) static int virLXCControllerSetupCpuAffinity(virLXCControllerPtr ctrl) { int hostcpus, maxcpu = CPU_SETSIZE; - virNodeInfo nodeinfo; virBitmapPtr cpumap, cpumapToSet; VIR_DEBUG("Setting CPU affinity"); - if (nodeGetInfo(NULL, &nodeinfo) < 0) - return -1; - /* setaffinity fails if you set bits for CPUs which * aren't present, so we have to limit ourselves */ - hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); + if ((hostcpus = nodeGetCPUMap(NULL, NULL, NULL, 0)) < 0) + return -1; + if (maxcpu > hostcpus) maxcpu = hostcpus; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 18be7d9..84f015f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4089,7 +4089,6 @@ qemudDomainGetVcpuPinInfo(virDomainPtr dom, struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm = NULL; - virNodeInfo nodeinfo; virDomainDefPtr targetDef = NULL; int ret = -1; int maxcpu, hostcpus, vcpu, pcpu; @@ -4125,9 +4124,8 @@ qemudDomainGetVcpuPinInfo(virDomainPtr dom, /* Coverity didn't realize that targetDef must be set if we got here. */ sa_assert(targetDef); - if (nodeGetInfo(dom->conn, &nodeinfo) < 0) + if ((hostcpus = nodeGetCPUMap(dom->conn, NULL, NULL, 0)) < 0) goto cleanup; - hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); maxcpu = maplen * 8; if (maxcpu > hostcpus) maxcpu = hostcpus; @@ -4340,7 +4338,6 @@ qemudDomainGetEmulatorPinInfo(virDomainPtr dom, { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm = NULL; - virNodeInfo nodeinfo; virDomainDefPtr targetDef = NULL; int ret = -1; int maxcpu, hostcpus, pcpu; @@ -4373,9 +4370,8 @@ qemudDomainGetEmulatorPinInfo(virDomainPtr dom, /* Coverity didn't realize that targetDef must be set if we got here. */ sa_assert(targetDef); - if (nodeGetInfo(dom->conn, &nodeinfo) < 0) + if ((hostcpus = nodeGetCPUMap(dom->conn, NULL, NULL, 0)) < 0) goto cleanup; - hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); maxcpu = maplen * 8; if (maxcpu > hostcpus) maxcpu = hostcpus; @@ -4417,7 +4413,6 @@ qemudDomainGetVcpus(virDomainPtr dom, int maplen) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - virNodeInfo nodeinfo; int i, v, maxcpu, hostcpus; int ret = -1; qemuDomainObjPrivatePtr priv; @@ -4443,10 +4438,9 @@ qemudDomainGetVcpus(virDomainPtr dom, priv = vm->privateData; - if (nodeGetInfo(dom->conn, &nodeinfo) < 0) + if ((hostcpus = nodeGetCPUMap(dom->conn, NULL, NULL, 0)) < 0) goto cleanup; - hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); maxcpu = maplen * 8; if (maxcpu > hostcpus) maxcpu = hostcpus; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5004e9b..64206cb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1899,15 +1899,13 @@ qemuPrepareCpumap(struct qemud_driver *driver, virBitmapPtr nodemask) { int i, hostcpus, maxcpu = QEMUD_CPUMASK_LEN; - virNodeInfo nodeinfo; virBitmapPtr cpumap = NULL; - if (nodeGetInfo(NULL, &nodeinfo) < 0) - return NULL; - /* setaffinity fails if you set bits for CPUs which * aren't present, so we have to limit ourselves */ - hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); + if ((hostcpus = nodeGetCPUMap(NULL, NULL, NULL, 0)) < 0) + return NULL; + if (maxcpu > hostcpus) maxcpu = hostcpus; @@ -1923,7 +1921,7 @@ qemuPrepareCpumap(struct qemud_driver *driver, bool result; if (virBitmapGetBit(nodemask, i, &result) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to covert nodeset to cpuset")); + _("Failed to convert nodeset to cpuset")); virBitmapFree(cpumap); return NULL; } -- 1.7.12.4
On 10/26/2012 07:19 AM, Viktor Mihajlovski wrote:
The drivers for QEMU and LXC use virNodeGetInfo to determine the number of host CPUs. This approach can lead to a wrong (too small) number if one or more CPUs are offline. It is better to use virNodeGetCPUMap if available, which is the case here.
Hmm - ever since commit 4fbf322 added the simpler nodeGetCPUCount, that function seems like a better option than calling virNodeGetCPUMap with NULL arguments if all we need is the number of available (and possibly offline) CPUs.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/lxc/lxc_controller.c | 8 +++----- src/qemu/qemu_driver.c | 12 +++--------- src/qemu/qemu_process.c | 10 ++++------ 3 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index a41c903..e9720bc 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -492,17 +492,15 @@ static int virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl) static int virLXCControllerSetupCpuAffinity(virLXCControllerPtr ctrl) { int hostcpus, maxcpu = CPU_SETSIZE; - virNodeInfo nodeinfo; virBitmapPtr cpumap, cpumapToSet;
VIR_DEBUG("Setting CPU affinity");
- if (nodeGetInfo(NULL, &nodeinfo) < 0) - return -1; - /* setaffinity fails if you set bits for CPUs which * aren't present, so we have to limit ourselves */ - hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); + if ((hostcpus = nodeGetCPUMap(NULL, NULL, NULL, 0)) < 0) + return -1; + if (maxcpu > hostcpus) maxcpu = hostcpus;
But the idea of using the simplest function possible instead of calling nodeGetInfo() and discarding the bulk of the struct makes sense. Looking forward to v2.
@@ -1923,7 +1921,7 @@ qemuPrepareCpumap(struct qemud_driver *driver, bool result; if (virBitmapGetBit(nodemask, i, &result) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to covert nodeset to cpuset")); + _("Failed to convert nodeset to cpuset")); virBitmapFree(cpumap);
I noticed this one independently, and fixed it in commit dd0a704 before even seeing this thread from you. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Modified the places where virNodeGetInfo was used for the purpose of obtaining the maximum node CPU number. Transparently falling back to virNodeGetInfo in case of failure. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- python/libvirt-override.c | 100 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 74 insertions(+), 26 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index cd48227..f4e8e9a 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -1343,18 +1343,30 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, virVcpuInfoPtr cpuinfo = NULL; unsigned char *cpumap = NULL; size_t cpumaplen, i; - int i_retval; + int i_retval, cpunum; if (!PyArg_ParseTuple(args, (char *)"O:virDomainGetVcpus", &pyobj_domain)) return NULL; domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + /* try to determine the host cpu number directly */ LIBVIRT_BEGIN_ALLOW_THREADS; - i_retval = virNodeGetInfo(virDomainGetConnect(domain), &nodeinfo); + i_retval = virNodeGetCPUMap(virDomainGetConnect(domain), NULL, NULL, 0); LIBVIRT_END_ALLOW_THREADS; - if (i_retval < 0) - return VIR_PY_INT_FAIL; + + if (i_retval < 0) { + /* fallback: use nodeinfo */ + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virNodeGetInfo(virDomainGetConnect(domain), &nodeinfo); + LIBVIRT_END_ALLOW_THREADS; + if (i_retval < 0) + return VIR_PY_INT_FAIL; + + cpunum = VIR_NODEINFO_MAXCPUS(nodeinfo); + } else { + cpunum = i_retval; + } LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetInfo(domain, &dominfo); @@ -1365,7 +1377,7 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, if (VIR_ALLOC_N(cpuinfo, dominfo.nrVirtCpu) < 0) return PyErr_NoMemory(); - cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo)); + cpumaplen = VIR_CPU_MAPLEN(cpunum); if (xalloc_oversized(dominfo.nrVirtCpu, cpumaplen) || VIR_ALLOC_N(cpumap, dominfo.nrVirtCpu * cpumaplen) < 0) { error = PyErr_NoMemory(); @@ -1423,11 +1435,11 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, goto cleanup; } for (i = 0 ; i < dominfo.nrVirtCpu ; i++) { - PyObject *info = PyTuple_New(VIR_NODEINFO_MAXCPUS(nodeinfo)); + PyObject *info = PyTuple_New(cpunum); int j; if (info == NULL) goto cleanup; - for (j = 0 ; j < VIR_NODEINFO_MAXCPUS(nodeinfo) ; j++) { + for (j = 0 ; j < cpunum ; j++) { PyObject *item = NULL; if ((item = PyBool_FromLong(VIR_CPU_USABLE(cpumap, cpumaplen, i, j))) == NULL || PyTuple_SetItem(info, j, item) < 0) { @@ -1469,7 +1481,7 @@ libvirt_virDomainPinVcpu(PyObject *self ATTRIBUTE_UNUSED, PyObject *ret = NULL; virNodeInfo nodeinfo; unsigned char *cpumap; - int cpumaplen, i, vcpu, tuple_size; + int cpumaplen, i, vcpu, tuple_size, cpunum; int i_retval; if (!PyArg_ParseTuple(args, (char *)"OiO:virDomainPinVcpu", @@ -1477,11 +1489,23 @@ libvirt_virDomainPinVcpu(PyObject *self ATTRIBUTE_UNUSED, return NULL; domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + /* try to determine the host cpu number directly */ LIBVIRT_BEGIN_ALLOW_THREADS; - i_retval = virNodeGetInfo(virDomainGetConnect(domain), &nodeinfo); + i_retval = virNodeGetCPUMap(virDomainGetConnect(domain), NULL, NULL, 0); LIBVIRT_END_ALLOW_THREADS; - if (i_retval < 0) - return VIR_PY_INT_FAIL; + + if (i_retval < 0) { + /* fallback: use nodeinfo */ + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virNodeGetInfo(virDomainGetConnect(domain), &nodeinfo); + LIBVIRT_END_ALLOW_THREADS; + if (i_retval < 0) + return VIR_PY_INT_FAIL; + + cpunum = VIR_NODEINFO_MAXCPUS(nodeinfo); + } else { + cpunum = i_retval; + } if (PyTuple_Check(pycpumap)) { tuple_size = PyTuple_Size(pycpumap); @@ -1492,7 +1516,7 @@ libvirt_virDomainPinVcpu(PyObject *self ATTRIBUTE_UNUSED, return ret; } - cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo)); + cpumaplen = VIR_CPU_MAPLEN(cpunum); if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) return PyErr_NoMemory(); @@ -1509,7 +1533,7 @@ libvirt_virDomainPinVcpu(PyObject *self ATTRIBUTE_UNUSED, VIR_UNUSE_CPU(cpumap, i); } - for (; i < VIR_NODEINFO_MAXCPUS(nodeinfo); i++) + for (; i < cpunum; i++) VIR_UNUSE_CPU(cpumap, i); LIBVIRT_BEGIN_ALLOW_THREADS; @@ -1536,7 +1560,7 @@ libvirt_virDomainPinVcpuFlags(PyObject *self ATTRIBUTE_UNUSED, PyObject *ret = NULL; virNodeInfo nodeinfo; unsigned char *cpumap; - int cpumaplen, i, vcpu, tuple_size; + int cpumaplen, i, vcpu, tuple_size, cpunum; unsigned int flags; int i_retval; @@ -1545,11 +1569,23 @@ libvirt_virDomainPinVcpuFlags(PyObject *self ATTRIBUTE_UNUSED, return NULL; domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + /* try to determine the host cpu number directly */ LIBVIRT_BEGIN_ALLOW_THREADS; - i_retval = virNodeGetInfo(virDomainGetConnect(domain), &nodeinfo); + i_retval = virNodeGetCPUMap(virDomainGetConnect(domain), NULL, NULL, 0); LIBVIRT_END_ALLOW_THREADS; - if (i_retval < 0) - return VIR_PY_INT_FAIL; + + if (i_retval < 0) { + /* fallback: use nodeinfo */ + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virNodeGetInfo(virDomainGetConnect(domain), &nodeinfo); + LIBVIRT_END_ALLOW_THREADS; + if (i_retval < 0) + return VIR_PY_INT_FAIL; + + cpunum = VIR_NODEINFO_MAXCPUS(nodeinfo); + } else { + cpunum = i_retval; + } if (PyTuple_Check(pycpumap)) { tuple_size = PyTuple_Size(pycpumap); @@ -1560,7 +1596,7 @@ libvirt_virDomainPinVcpuFlags(PyObject *self ATTRIBUTE_UNUSED, return ret; } - cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo)); + cpumaplen = VIR_CPU_MAPLEN(cpunum); if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) return PyErr_NoMemory(); @@ -1577,7 +1613,7 @@ libvirt_virDomainPinVcpuFlags(PyObject *self ATTRIBUTE_UNUSED, VIR_UNUSE_CPU(cpumap, i); } - for (; i < VIR_NODEINFO_MAXCPUS(nodeinfo); i++) + for (; i < cpunum; i++) VIR_UNUSE_CPU(cpumap, i); LIBVIRT_BEGIN_ALLOW_THREADS; @@ -1604,18 +1640,30 @@ libvirt_virDomainGetVcpuPinInfo(PyObject *self ATTRIBUTE_UNUSED, unsigned char *cpumaps; size_t cpumaplen, vcpu, pcpu; unsigned int flags; - int i_retval; + int i_retval, cpunum; if (!PyArg_ParseTuple(args, (char *)"Oi:virDomainGetVcpuPinInfo", &pyobj_domain, &flags)) return NULL; domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + /* try to determine the host cpu number directly */ LIBVIRT_BEGIN_ALLOW_THREADS; - i_retval = virNodeGetInfo(virDomainGetConnect(domain), &nodeinfo); + i_retval = virNodeGetCPUMap(virDomainGetConnect(domain), NULL, NULL, 0); LIBVIRT_END_ALLOW_THREADS; - if (i_retval < 0) - return VIR_PY_NONE; + + if (i_retval < 0) { + /* fallback: use nodeinfo */ + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virNodeGetInfo(virDomainGetConnect(domain), &nodeinfo); + LIBVIRT_END_ALLOW_THREADS; + if (i_retval < 0) + return VIR_PY_NONE; + + cpunum = VIR_NODEINFO_MAXCPUS(nodeinfo); + } else { + cpunum = i_retval; + } LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetInfo(domain, &dominfo); @@ -1623,7 +1671,7 @@ libvirt_virDomainGetVcpuPinInfo(PyObject *self ATTRIBUTE_UNUSED, if (i_retval < 0) return VIR_PY_NONE; - cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo)); + cpumaplen = VIR_CPU_MAPLEN(cpunum); if (xalloc_oversized(dominfo.nrVirtCpu, cpumaplen) || VIR_ALLOC_N(cpumaps, dominfo.nrVirtCpu * cpumaplen) < 0) goto cleanup; @@ -1639,11 +1687,11 @@ libvirt_virDomainGetVcpuPinInfo(PyObject *self ATTRIBUTE_UNUSED, goto cleanup; for (vcpu = 0; vcpu < dominfo.nrVirtCpu; vcpu++) { - PyObject *mapinfo = PyTuple_New(VIR_NODEINFO_MAXCPUS(nodeinfo)); + PyObject *mapinfo = PyTuple_New(cpunum); if (mapinfo == NULL) goto cleanup; - for (pcpu = 0; pcpu < VIR_NODEINFO_MAXCPUS(nodeinfo); pcpu++) { + for (pcpu = 0; pcpu < cpunum; pcpu++) { PyTuple_SetItem(mapinfo, pcpu, PyBool_FromLong(VIR_CPU_USABLE(cpumaps, cpumaplen, vcpu, pcpu))); } -- 1.7.12.4
On 10/26/2012 07:19 AM, Viktor Mihajlovski wrote:
Modified the places where virNodeGetInfo was used for the purpose of obtaining the maximum node CPU number. Transparently falling back to virNodeGetInfo in case of failure.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- python/libvirt-override.c | 100 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 74 insertions(+), 26 deletions(-)
Unlike patch 1/3/, this is the correct approach of using the only public API for getting at the full number, and properly falling back to the older API. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Modified the places where virNodeGetInfo was used for the purpose of obtaining the maximum node CPU number. Transparently falling back to virNodeGetInfo in case of failure. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- tools/virsh-domain.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0906267..0aa643a 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4508,9 +4508,14 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (virNodeGetInfo(ctl->conn, &nodeinfo) != 0) { - virDomainFree(dom); - return false; + if ((maxcpu = virNodeGetCPUMap(ctl->conn, NULL, NULL, 0)) < 0) { + /* fall back to nodeinfo */ + if (virNodeGetInfo(ctl->conn, &nodeinfo) == 0) { + maxcpu = VIR_NODEINFO_MAXCPUS(nodeinfo); + } else { + virDomainFree(dom); + return false; + } } if (virDomainGetInfo(dom, &info) != 0) { @@ -4519,7 +4524,6 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) } cpuinfo = vshMalloc(ctl, sizeof(virVcpuInfo)*info.nrVirtCpu); - maxcpu = VIR_NODEINFO_MAXCPUS(nodeinfo); cpumaplen = VIR_CPU_MAPLEN(maxcpu); cpumaps = vshMalloc(ctl, info.nrVirtCpu * cpumaplen); @@ -4695,9 +4699,14 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) return false; } - if (virNodeGetInfo(ctl->conn, &nodeinfo) != 0) { - virDomainFree(dom); - return false; + if ((maxcpu = virNodeGetCPUMap(ctl->conn, NULL, NULL, 0)) < 0) { + /* fall back to nodeinfo */ + if (virNodeGetInfo(ctl->conn, &nodeinfo) == 0) { + maxcpu = VIR_NODEINFO_MAXCPUS(nodeinfo); + } else { + virDomainFree(dom); + return false; + } } if (virDomainGetInfo(dom, &info) != 0) { @@ -4712,7 +4721,6 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) return false; } - maxcpu = VIR_NODEINFO_MAXCPUS(nodeinfo); cpumaplen = VIR_CPU_MAPLEN(maxcpu); /* Query mode: show CPU affinity information then exit.*/ @@ -4905,12 +4913,16 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd) } query = !cpulist; - if (virNodeGetInfo(ctl->conn, &nodeinfo) != 0) { - virDomainFree(dom); - return false; + if ((maxcpu = virNodeGetCPUMap(ctl->conn, NULL, NULL, 0)) < 0) { + /* fall back to nodeinfo */ + if (virNodeGetInfo(ctl->conn, &nodeinfo) == 0) { + maxcpu = VIR_NODEINFO_MAXCPUS(nodeinfo); + } else { + virDomainFree(dom); + return false; + } } - maxcpu = VIR_NODEINFO_MAXCPUS(nodeinfo); cpumaplen = VIR_CPU_MAPLEN(maxcpu); /* Query mode: show CPU affinity information then exit.*/ -- 1.7.12.4
On 10/26/2012 07:19 AM, Viktor Mihajlovski wrote:
Modified the places where virNodeGetInfo was used for the purpose of obtaining the maximum node CPU number. Transparently falling back to virNodeGetInfo in case of failure.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- tools/virsh-domain.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0906267..0aa643a 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4508,9 +4508,14 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false;
- if (virNodeGetInfo(ctl->conn, &nodeinfo) != 0) { - virDomainFree(dom); - return false; + if ((maxcpu = virNodeGetCPUMap(ctl->conn, NULL, NULL, 0)) < 0) { + /* fall back to nodeinfo */ + if (virNodeGetInfo(ctl->conn, &nodeinfo) == 0) { + maxcpu = VIR_NODEINFO_MAXCPUS(nodeinfo); + } else { + virDomainFree(dom); + return false; + }
This looks correct. However, it may be easier to write a helper function vshGetCPUCount, rather than copying and pasting this code into multiple places. I'm okay giving this an ACK, but since I've already asked for a v2 of patch 1, you might want to do a v2 of this as well. Oh, and I guess the same comment applies to the python code in 2/3. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 10/31/2012 02:30 AM, Eric Blake wrote:
This looks correct. However, it may be easier to write a helper function vshGetCPUCount, rather than copying and pasting this code into multiple places. I'm okay giving this an ACK, but since I've already asked for a v2 of patch 1, you might want to do a v2 of this as well. Oh, and I guess the same comment applies to the python code in 2/3.
Thanks for the comments, I have to agree in all points. New series underway. -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (2)
-
Eric Blake -
Viktor Mihajlovski