diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 22d53e5..ba2f793 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -468,7 +468,8 @@ nodeGetCellsFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED, long long mem; if (numa_node_size64(n, &mem) < 0) { nodeReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Failed to query NUMA free memory")); + "%s: %d", _("Failed to query NUMA free memory " + "for node"), n);
This is hard to translate and it doesn't work well if the number needs to be somewhere else than at the end in some language. It should rather look like _("Failed to query NUMA free memory for node: %d"), n
goto cleanup; } freeMems[numCells++] = mem; diff --git a/tools/virsh.c b/tools/virsh.c index 50d5e33..165a791 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2301,32 +2307,60 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) }
if (all_given) { - if (virNodeGetInfo(ctl->conn, &info) < 0) { - vshError(ctl, "%s", _("failed to get NUMA nodes count")); + cap_xml = virConnectGetCapabilities(ctl->conn); + if (!cap_xml) { + vshError(ctl, "%s", _("unable to get node capabilities")); goto cleanup; }
- if (!info.nodes) { - vshError(ctl, "%s", _("no NUMA nodes present")); + xml = xmlReadDoc((const xmlChar *) cap_xml, "node.xml", NULL, + XML_PARSE_NOENT | XML_PARSE_NONET |
+ XML_PARSE_NOWARNING); + + if (!xml) { + vshError(ctl, "%s", _("unable to get node capabilities")); goto cleanup; }
- if (VIR_ALLOC_N(nodes, info.nodes) < 0) { - vshError(ctl, "%s", _("could not allocate memory")); + ctxt = xmlXPathNewContext(xml); + nodes_cnt = virXPathNodeSet("/capabilities/host/topology/cells/cell", + ctxt, &nodes); + + if (nodes_cnt == -1) { + vshError(ctl, "%s", _("could not get information about " + "NUMA topology")); goto cleanup; }
- ret = virNodeGetCellsFreeMemory(ctl->conn, nodes, 0, info.nodes); - if (ret != info.nodes) { - vshError(ctl, "%s", _("could not get information about " - "all NUMA nodes")); + if ((VIR_ALLOC_N(nodes_free, nodes_cnt) < 0) || + (VIR_ALLOC_N(nodes_id, nodes_cnt) < 0)){ + vshError(ctl, "%s", _("could not allocate memory")); goto cleanup; } You can use vshCalloc, which already prints an error and fails. Yeah, virsh code is a mess and doesn't really fit in general libvirt coding standards. But
Space at the end of the line above. that's a different story.
+ for (i = 0; i < nodes_cnt; i++) { + unsigned long id; + char *val = virXMLPropString(nodes[i], "id"); + char *conv = NULL; + id = strtoul(val, &conv, 10); + if (*conv !='\0') { + vshError(ctl, "%s", _("conversion from string failed")); + goto cleanup; + }
Apart from the funky indentation, you can use virStrToLong_ul() to do the job. Also val is leaked here, you should free it just after converting it to integer.
+ nodes_id[i]=id; + ret = virNodeGetCellsFreeMemory(ctl->conn, &(nodes_free[i]), id, 1); + if (ret != 1) { + vshError(ctl, "%s %lu", _("failed to get free memory for " + "NUMA node nuber:"), id); Move %lu inside the translatable string here as well.
+ goto cleanup; + } + } + memory = 0; - for (cell = 0; cell < info.nodes; cell++) { - vshPrint(ctl, "%5d: %10llu kB\n", cell, (nodes[cell]/1024)); - memory += nodes[cell]; + for (cell = 0; cell < nodes_cnt; cell++) { + vshPrint(ctl, "%5lu: %10llu kB\n", nodes_id[cell], + (nodes_free[cell]/1024)); + memory += nodes_free[cell]; }
vshPrintExtra(ctl, "--------------------\n");
Otherwise it looks good. Jirka