On 09/06/2013 05:11 PM, Giuseppe Scrivano wrote:
The new function virConnectGetCPUModelNames allows to retrieve the list of CPU models known by the hypervisor for a specific architecture.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- include/libvirt/libvirt.h.in | 18 +++++++++++++ python/generator.py | 1 + src/cpu/cpu.c | 64 ++++++++++++++++++++++++++++++++++++++++++++ src/cpu/cpu.h | 3 +++ src/driver.h | 7 +++++ src/libvirt.c | 47 ++++++++++++++++++++++++++++++++ src/libvirt_private.syms | 1 + src/libvirt_public.syms | 5 ++++ tools/virsh-host.c | 48 +++++++++++++++++++++++++++++++++ tools/virsh.pod | 5 ++++ 10 files changed, 199 insertions(+)
In addition to Daniel's review,
+++ b/include/libvirt/libvirt.h.in @@ -4006,6 +4006,24 @@ int virConnectCompareCPU(virConnectPtr conn, const char *xmlDesc, unsigned int flags);
+/** + * virConnectGetCPUModelNames: + * + * @conn: virConnect connection + * @arch: Architecture + * @models: NULL terminated array of the CPU models supported for the specified + * architecture. Each element and the array itself must be freed by the caller + * with free. + * @flags: extra flags; not used yet, so callers should always pass 0. + * + * Get the list of supported CPU models for a specific architecture. + * + * Returns -1 on error, 0 on success. + */ +int virConnectGetCPUModelNames(virConnectPtr conn, + const char *arch, + char ***models, + unsigned int flags);
Typically we have not documented functions in the .h, just in src/libvirt.c.
+static int +cpuGetArchModelsCb(enum cpuMapElement element, + xmlXPathContextPtr ctxt, + void *cbdata) +{ + char *name; + struct cpuGetModelsData *data = cbdata; + if (element != CPU_MAP_ELEMENT_MODEL) + return 0; + + name = virXPathString("string(@name)", ctxt); + if (name == NULL) + return -1; + + if (VIR_EXPAND_N(data->data, data->len, 1) < 0) + return -1; + + data->data[data->len - 2] = name; + data->data[data->len - 1] = NULL; + return 0;
VIR_INSERT_ELEMENT may be simpler to use than VIR_EXPAND_N. Furthermore, VIR_EXPAND_N guarantees zero-initialization of the growth, so the data->data[data->len - 1] = NULL; line is redundant.
+int +virConnectGetCPUModelNames(virConnectPtr conn, const char *arch, char ***models, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, arch=%s, flags=%x", conn, arch, flags); + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (arch == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + }
virCheckNonNullArgReturn(arch, -1)
+ + if (conn->driver->connectGetCPUModelNames) { + if (conn->driver->connectGetCPUModelNames(conn, arch, models, flags) < 0) + goto error; + + return 0;
I agree with Dan that this should return the number of non-NULL entries (so setting the array to { "x86_64", "i686", NULL } would return 2, even though the array has allocated room for [at least] 3 pointers).
+++ b/tools/virsh-host.c @@ -92,6 +92,25 @@ static const vshCmdOptDef opts_freecell[] = { {.name = NULL} };
+static const vshCmdInfo info_cpu_models[] = { + {.name = "help", + .data = N_("CPU models.")
We generally don't have trailing '.' in the short help.
+ }, + {.name = "desc", + .data = N_("Get the CPU models for an arch.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_cpu_models[] = { + {.name = "arch", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("architecture") + }, + {.name = NULL} +}; +
These new [] data should be closer...
static bool cmdFreecell(vshControl *ctl, const vshCmd *cmd) { @@ -626,6 +645,29 @@ cmdURI(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) return true; }
+static bool +cmdCPUModelNames(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
...to the function they are associated with. (That is, don't split cmdFreecell from its options).
+{ + char **models, **it; + const char *arch = NULL; + + if (vshCommandOptStringReq(ctl, cmd, "arch", &arch) < 0) + return false; + + if (virConnectGetCPUModelNames(ctl->conn, arch, &models, 0) < 0) { + vshError(ctl, "%s", _("failed to get CPU Models Names"));
Sounds awkward; maybe "failed to get CPU model names"
@@ -851,6 +893,12 @@ const vshCmdDef hostAndHypervisorCmds[] = { .info = info_capabilities, .flags = 0 }, + {.name = "cpu-models", + .handler = cmdCPUModelNames, + .opts = opts_cpu_models, + .info = info_cpu_models, + .flags = 0
Not your fault, but I'd like trailing commas in all of these initializers (as a separate cleanup), so that future additions can be strict additions (thinking forward to Tomas' work on tab-completers). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org