On 09/11/2013 08:12 AM, Giuseppe Scrivano wrote: Minor tweak: @models: Pointer to a variable to store the NULL-terminated array... Can models usefully be NULL, just for the return value without actually getting the number of names? I'll decide that based on the patch to the .x file... /me goes and looks Hmm, patch 3/7 blindly stores into *models, meaning you require a non-NULL models, and are missing a virCheckNonNullArgReturn(models, -1) here. On the other hand, it is inconsistent with all our other *ListAll functions, which allow a NULL pointer for the sake of using just the return value to grab a count of valid information (and implemented in the .x file by a side channel need_results in the _args, and ret in the _ret struct). I'm leaning towards consistency, so I'll support that here, too. Everything else looks good. ACK with this squashed in: diff --git i/src/libvirt.c w/src/libvirt.c index 159d922..b31b561 100644 --- i/src/libvirt.c +++ w/src/libvirt.c @@ -18528,9 +18528,10 @@ error: * * @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. + * @models: Pointer to a variable to store the 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. Pass + * NULL if only the list length is needed. * @flags: extra flags; not used yet, so callers should always pass 0. * * Get the list of supported CPU models for a specific architecture. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 09/13/2013 01:25 PM, Eric Blake wrote: And yet another tweak squashed in - we must log ALL parameters. diff --git i/src/libvirt.c w/src/libvirt.c index b9b9459..5831f84 100644 --- i/src/libvirt.c +++ w/src/libvirt.c @@ -18542,7 +18542,8 @@ int virConnectGetCPUModelNames(virConnectPtr conn, const char *arch, char ***models, unsigned int flags) { - VIR_DEBUG("conn=%p, arch=%s, flags=%x", conn, arch, flags); + VIR_DEBUG("conn=%p, arch=%s, models=%p, flags=%x", + conn, arch, models, flags); virResetLastError(); if (models) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 09/11/2013 08:12 AM, Giuseppe Scrivano wrote: Blind dereference of models; but since I argued in patch 1 that passing NULL makes sense for the return value count, this needs a tweak... ACK with this squashed in. diff --git i/src/cpu/cpu.c w/src/cpu/cpu.c index 7011b3d..dc0c474 100644 --- i/src/cpu/cpu.c +++ w/src/cpu/cpu.c @@ -1,7 +1,7 @@ /* * cpu.c: internal functions for CPU manipulation * - * Copyright (C) 2009-2012 Red Hat, Inc. + * Copyright (C) 2009-2013 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 @@ -479,6 +479,11 @@ cpuGetArchModelsCb(enum cpuMapElement element, if (name == NULL) return -1; + if (!data->data) { + VIR_FREE(name); + data->len++; + return 0; + } return VIR_INSERT_ELEMENT(data->data, data->len - 1, data->len, name); } @@ -495,16 +500,17 @@ cpuGetModels(const char *arch, char ***models) { struct cpuGetModelsData data; - *models = data.data = NULL; + data.data = NULL; data.len = 1; - if (VIR_ALLOC_N(data.data, data.len) < 0) + if (models && VIR_ALLOC_N(data.data, data.len) < 0) goto error; if (cpuGetArchModels(arch, &data) < 0) goto error; - *models = data.data; + if (models) + *models = data.data; return data.len - 1; -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com&gt; --- daemon/remote.c | 43 +++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 57 ++++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 20 +++++++++++++++- src/remote_protocol-structs | 11 +++++++++ 4 files changed, 130 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index 2aff7c1..d357016 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -5037,6 +5037,49 @@ cleanup: static int +remoteDispatchConnectGetCPUModelNames(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_connect_get_cpu_model_names_args *args, + remote_connect_get_cpu_model_names_ret *ret) +{ + int len, rv = -1; + char **models; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + len = virConnectGetCPUModelNames(priv->conn, args->arch, &models, + args->flags); + if (len < 0) + goto cleanup; + + if (ret->models.models_len > REMOTE_CONNECT_CPU_MODELS_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many CPU models '%d' for limit '%d'"), + ret->models.models_len, REMOTE_CONNECT_CPU_MODELS_MAX); + virStringFreeList(models); + goto cleanup; + } + + ret->models.models_val = models; + ret->models.models_len = len; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + return rv; +} + + +static int remoteDispatchDomainCreateXMLWithFiles(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 62e77a5..aa3762a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5541,6 +5541,62 @@ done: static int +remoteConnectGetCPUModelNames(virConnectPtr conn, + const char *arch, + char ***models, + unsigned int flags) +{ + int rv = -1; + size_t i; + char **retmodels; + remote_connect_get_cpu_model_names_args args; + remote_connect_get_cpu_model_names_ret ret; + struct private_data *priv = conn->privateData; + remoteDriverLock(priv); + + memset(&args, 0, sizeof(args)); + memset(&ret, 0, sizeof(ret)); + + args.arch = (char *) arch; + args.flags = flags; + + if (call(conn, priv, 0, REMOTE_PROC_CONNECT_GET_CPU_MODEL_NAMES, + (xdrproc_t) xdr_remote_connect_get_cpu_model_names_args, + (char *) &args, + (xdrproc_t) xdr_remote_connect_get_cpu_model_names_ret, + (char *) &ret) < 0) + goto error; + + /* Check the length of the returned list carefully. */ + if (ret.models.models_len > REMOTE_CONNECT_CPU_MODELS_MAX) { + virReportError(VIR_ERR_RPC, "%s", + _("remoteConnectGetCPUModelNames: " + "returned number of CPU models exceeds limit")); + goto error; + } + + if (VIR_ALLOC_N(retmodels, ret.models.models_len + 1) < 0) + goto error; + + for (i = 0; i < ret.models.models_len; i++) + retmodels[i] = ret.models.models_val[i]; + + /* Caller frees MODELS. */ + *models = retmodels; + rv = ret.models.models_len; + +done: + remoteDriverUnlock(priv); + return rv; + +error: + rv = -1; + virStringFreeList(ret.models.models_val); + goto done; +} + + +static int remoteDomainOpenGraphics(virDomainPtr dom, unsigned int idx, int fd, @@ -6933,6 +6989,7 @@ static virDriver remote_driver = { .domainMigratePerform3Params = remoteDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = remoteDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = remoteDomainMigrateConfirm3Params, /* 1.1.0 */ + .connectGetCPUModelNames = remoteConnectGetCPUModelNames, /* 1.1.3 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index a1c23da..a1d90ad 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -232,6 +232,9 @@ const REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX = 64; /* Upper limit on number of job stats */ const REMOTE_DOMAIN_JOB_STATS_MAX = 16; +/* Upper limit on number of CPU models */ +const REMOTE_CONNECT_CPU_MODELS_MAX = 8192; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN]; @@ -2835,6 +2838,15 @@ struct remote_domain_event_device_removed_msg { remote_nonnull_string devAlias; }; +struct remote_connect_get_cpu_model_names_args { + remote_nonnull_string arch; + unsigned int flags; +}; + +struct remote_connect_get_cpu_model_names_ret { + remote_nonnull_string models<REMOTE_CONNECT_CPU_MODELS_MAX>; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -4998,5 +5010,11 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311 + REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311, + + /** + * @generate: none + * @acl: connect:read + */ + REMOTE_PROC_CONNECT_GET_CPU_MODEL_NAMES = 312 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 4e27aae..9148202 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2316,6 +2316,16 @@ struct remote_domain_event_device_removed_msg { remote_nonnull_domain dom; remote_nonnull_string devAlias; }; +struct remote_connect_get_cpu_model_names_args { + remote_nonnull_string arch; + u_int flags; +}; +struct remote_connect_get_cpu_model_names_ret { + struct { + u_int models_len; + remote_nonnull_string * models_val; + } models; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -2628,4 +2638,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_CREATE_XML_WITH_FILES = 309, REMOTE_PROC_DOMAIN_CREATE_WITH_FILES = 310, REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311, + REMOTE_PROC_CONNECT_GET_CPU_MODEL_NAMES = 312, }; -- 1.8.3.1

Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bbf2d23..c139daa 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15880,6 +15880,19 @@ qemuNodeSuspendForDuration(virConnectPtr conn, return nodeSuspendForDuration(target, duration, flags); } +static int +qemuConnectGetCPUModelNames(virConnectPtr conn, + const char *arch, + char ***models, + unsigned int flags) +{ + virCheckFlags(0, -1); + if (virConnectGetCPUModelNamesEnsureACL(conn) < 0) + return -1; + + return cpuGetModels(arch, models); +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, @@ -16067,6 +16080,7 @@ static virDriver qemuDriver = { .domainMigratePerform3Params = qemuDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = qemuDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = qemuDomainMigrateConfirm3Params, /* 1.1.0 */ + .connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */ }; -- 1.8.3.1

Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com&gt; --- src/test/test_driver.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index f9eee44..fb4c1d9 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -54,6 +54,7 @@ #include "virtypedparam.h" #include "virrandom.h" #include "virstring.h" +#include "cpu/cpu.h" #define VIR_FROM_THIS VIR_FROM_TEST @@ -5801,6 +5802,15 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED, return ret; } +static int +testConnectGetCPUModelNames(virConnectPtr conn ATTRIBUTE_UNUSED, + const char *arch, + char ***models, + unsigned int flags) +{ + virCheckFlags(0, -1); + return cpuGetModels(arch, models); +} static virDriver testDriver = { .no = VIR_DRV_TEST, @@ -5872,6 +5882,7 @@ static virDriver testDriver = { .connectIsAlive = testConnectIsAlive, /* 0.9.8 */ .nodeGetCPUMap = testNodeGetCPUMap, /* 1.0.0 */ .domainScreenshot = testDomainScreenshot, /* 1.0.5 */ + .connectGetCPUModelNames = testConnectGetCPUModelNames, /* 1.1.3 */ }; static virNetworkDriver testNetworkDriver = { -- 1.8.3.1

Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com&gt; --- tools/virsh-host.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 5 +++++ 2 files changed, 59 insertions(+) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 880ae4b..5d47428 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -627,6 +627,54 @@ cmdURI(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } /* + * "cpu-models" command + */ +static const vshCmdInfo info_cpu_models[] = { + {.name = "help", + .data = N_("CPU models") + }, + {.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} +}; + +static bool +cmdCPUModelNames(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + char **models; + size_t i; + int nmodels; + const char *arch = NULL; + + if (vshCommandOptStringReq(ctl, cmd, "arch", &arch) < 0) + return false; + + nmodels = virConnectGetCPUModelNames(ctl->conn, arch, &models, 0); + if (nmodels < 0) { + vshError(ctl, "%s", _("failed to get CPU model names")); + return false; + } + + for (i = 0; i < nmodels; i++) { + vshPrint(ctl, "%s\n", models[i]); + VIR_FREE(models[i]); + } + VIR_FREE(models); + + return true; +} + +/* * "version" command */ static const vshCmdInfo info_version[] = { @@ -851,6 +899,12 @@ const vshCmdDef hostAndHypervisorCmds[] = { .info = info_capabilities, .flags = 0 }, + {.name = "cpu-models", + .handler = cmdCPUModelNames, + .opts = opts_cpu_models, + .info = info_cpu_models, + .flags = 0 + }, {.name = "freecell", .handler = cmdFreecell, .opts = opts_freecell, diff --git a/tools/virsh.pod b/tools/virsh.pod index 0ae5178..72555e7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -163,6 +163,7 @@ group as an option. For example: Host and Hypervisor (help keyword 'host'): capabilities capabilities + cpu-models show the CPU models for an architecture connect (re)connect to hypervisor freecell NUMA free memory hostname print the hypervisor hostname @@ -358,6 +359,10 @@ current domain is in. =over 4 +=item B<cpu-models> I<arch> + +Print the list of CPU models known for the specified architecture. + =item B<running> The domain is currently running on a CPU -- 1.8.3.1

Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com&gt; --- python/generator.py | 2 +- python/libvirt-override-api.xml | 7 ++++++ python/libvirt-override.c | 52 +++++++++++++++++++++++++++++++++++++++++ python/libvirt-override.py | 11 +++++++++ 4 files changed, 71 insertions(+), 1 deletion(-) diff --git a/python/generator.py b/python/generator.py index 73107d7..12c14f1 100755 --- a/python/generator.py +++ b/python/generator.py @@ -250,6 +250,7 @@ lxc_functions_failed = [] qemu_functions_failed = [] functions_skipped = [ "virConnectListDomains", + "virConnectGetCPUModelNames", ] lxc_functions_skipped = [] qemu_functions_skipped = [] @@ -366,7 +367,6 @@ foreign_encoding_args = ( # Class methods which are written by hand in libvirt.c but the Python-level # code is still automatically generated (so they are not in skip_function()). skip_impl = ( - "virConnectGetCPUModelNames", 'virConnectGetVersion', 'virConnectGetLibVersion', 'virConnectListDomainsID', diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 9a88215..1bceb05 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -476,6 +476,13 @@ <arg name='xmlCPUs' type='const char **' info='array of XML descriptions of host CPUs'/> <arg name='flags' type='unsigned int' info='fine-tuning flags, currently unused, pass 0.'/> </function> + <function name='virConnectGetCPUModelNames' file='python'> + <info>Get the list of supported CPU models.</info> + <return type='char *' info='list of supported CPU models'/> + <arg name='conn' type='virConnectPtr' info='virConnect connection'/> + <arg name='arch' type='const char *' info='arch'/> + <arg name='flags' type='unsigned int' info='fine-tuning flags, currently unused, pass 0.'/> + </function> <function name='virDomainSnapshotListNames' file='python'> <info>collect the list of snapshot names for the given domain</info> <arg name='dom' type='virDomainPtr' info='pointer to the domain'/> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index d16b9a2..63c2566 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2276,6 +2276,58 @@ libvirt_virConnectGetVersion(PyObject *self ATTRIBUTE_UNUSED, return PyInt_FromLong(hvVersion); } +PyObject * +libvirt_virConnectGetCPUModelNames(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + int c_retval; + virConnectPtr conn; + PyObject *rv = NULL, *pyobj_conn; + char **models = NULL; + size_t i; + int flags = 0; + const char *arch = NULL; + + if (!PyArg_ParseTuple(args, (char *)"Osi:virConnectGetCPUModelNames", + &pyobj_conn, &arch, &flags)) + return NULL; + conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + + LIBVIRT_BEGIN_ALLOW_THREADS; + + c_retval = virConnectGetCPUModelNames(conn, arch, &models, flags); + + LIBVIRT_END_ALLOW_THREADS; + + if (c_retval == -1) + return VIR_PY_INT_FAIL; + + if ((rv = PyList_New(c_retval)) == NULL) + goto error; + + for (i = 0; i < c_retval; i++) { + PyObject *str; + if ((str = PyString_FromString(models[i])) == NULL) + goto error; + + PyList_SET_ITEM(rv, i, str); + } + +done: + if (models) { + for (i = 0; i < c_retval; i++) + VIR_FREE(models[i]); + VIR_FREE(models); + } + + return rv; + +error: + Py_XDECREF(rv); + rv = VIR_PY_INT_FAIL; + goto done; +} + static PyObject * libvirt_virConnectGetLibVersion(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) diff --git a/python/libvirt-override.py b/python/libvirt-override.py index ccfec48..3471a43 100644 --- a/python/libvirt-override.py +++ b/python/libvirt-override.py @@ -207,3 +207,14 @@ def virEventAddTimeout(timeout, cb, opaque): ret = libvirtmod.virEventAddTimeout(timeout, cbData) if ret == -1: raise libvirtError ('virEventAddTimeout() failed') return ret + +def getCPUModelNames(conn, arch, flags=0): + """ + get the list of supported CPU models. + @conn: virConnect connection + @arch: Architecture + @flags: extra flags; not used yet, so callers should always pass 0. + """ + ret = libvirtmod.virConnectGetCPUModelNames(conn._o, arch, flags) + if ret == None: raise libvirtError ('virConnectGetCPUModelNames() failed', conn=self) + return ret -- 1.8.3.1