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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list