[libvirt] [RFC] Expose cpu_map.xml via API
--- I have started working on: https://bugzilla.redhat.com/show_bug.cgi?id=916786 Before I split it in a series of commits, test it better and then proceed to virt-manager, are you ok with this idea? include/libvirt/libvirt.h.in | 11 +++++++++++ src/driver.h | 4 ++++ src/libvirt.c | 38 ++++++++++++++++++++++++++++++++++++++ src/libvirt_private.syms | 1 + src/libvirt_public.syms | 1 + src/lxc/lxc_driver.c | 11 +++++++++++ src/qemu/qemu_driver.c | 10 ++++++++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++++++++++- src/remote_protocol-structs | 4 ++++ src/test/test_driver.c | 6 ++++++ src/uml/uml_driver.c | 11 +++++++++++ src/util/virutil.c | 23 +++++++++++++++++++++++ src/util/virutil.h | 3 +++ tools/virsh-host.c | 21 +++++++++++++++++++++ tools/virsh.pod | 5 +++++ 16 files changed, 161 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a47e33c..d6e0d9a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4006,6 +4006,17 @@ int virConnectCompareCPU(virConnectPtr conn, const char *xmlDesc, unsigned int flags); +/** + * virConnectGetCPUMapDesc: + * + * @conn: virConnect connection + * + * Get the content of the cpu_map.xml file used by the connection. + * + * Returns XML description of the cpu_map.xml file. + */ +char *virConnectGetCPUMapDesc(virConnectPtr conn); + /** * virConnectBaselineCPUFlags diff --git a/src/driver.h b/src/driver.h index be64333..ab31262 100644 --- a/src/driver.h +++ b/src/driver.h @@ -681,6 +681,9 @@ typedef char * unsigned int ncpus, unsigned int flags); +typedef char * +(*virDrvConnectGetCPUMapDesc)(virConnectPtr conn); + typedef int (*virDrvDomainGetJobInfo)(virDomainPtr domain, virDomainJobInfoPtr info); @@ -1332,6 +1335,7 @@ struct _virDriver { virDrvDomainMigratePerform3Params domainMigratePerform3Params; virDrvDomainMigrateFinish3Params domainMigrateFinish3Params; virDrvDomainMigrateConfirm3Params domainMigrateConfirm3Params; + virDrvConnectGetCPUMapDesc connectGetCPUMapDesc; }; diff --git a/src/libvirt.c b/src/libvirt.c index 07a3fd5..5e5e594 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18519,6 +18519,44 @@ error: /** + * virConnectGetCPUMapDesc: + * + * @conn: virConnect connection + * + * Get the content of the cpu_map.xml file used by the connection. + * + * Returns the content of the cpu_map.xml file. The result must be freed + * by the caller of this function. + */ +char * +virConnectGetCPUMapDesc(virConnectPtr conn) +{ + VIR_DEBUG("conn=%p", conn); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return NULL; + } + + if (conn->driver->connectGetCPUMapDesc) { + char *ret = conn->driver->connectGetCPUMapDesc(conn); + if (ret == NULL) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return NULL; +} + + +/** * virConnectBaselineCPU: * * @conn: virConnect connection diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c25a61f..ed6d2a0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2074,6 +2074,7 @@ virSetUIDGID; virSetUIDGIDWithCaps; virStrIsPrint; virValidateWWN; +virGetCPUMapDesc; # util/viruuid.h diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index bbdf78a..8023c7e 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -629,6 +629,7 @@ LIBVIRT_1.1.0 { LIBVIRT_1.1.1 { global: + virConnectGetCPUMapDesc; virDomainCreateWithFiles; virDomainCreateXMLWithFiles; virDomainSetMemoryStatsPeriod; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 9cb95ff..633248b 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4590,6 +4590,16 @@ lxcNodeSuspendForDuration(virConnectPtr conn, } +static char * +lxcConnectGetCPUMapDesc(virConnectPtr conn) +{ + if (virConnectGetCPUMapDescEnsureACL(conn) < 0) + return NULL; + + return virGetCPUMapDesc(); +} + + /* Function Tables */ static virDriver lxcDriver = { .no = VIR_DRV_LXC, @@ -4671,6 +4681,7 @@ static virDriver lxcDriver = { .domainShutdownFlags = lxcDomainShutdownFlags, /* 1.0.1 */ .domainReboot = lxcDomainReboot, /* 1.0.1 */ .domainLxcOpenNamespace = lxcDomainLxcOpenNamespace, /* 1.0.2 */ + .connectGetCPUMapDesc = lxcConnectGetCPUMapDesc, /* 1.1.0 */ }; static virStateDriver lxcStateDriver = { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2ad236e..4a1d6fa 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16033,6 +16033,15 @@ qemuNodeSuspendForDuration(virConnectPtr conn, return nodeSuspendForDuration(target, duration, flags); } +static char * +qemuConnectGetCPUMapDesc(virConnectPtr conn) +{ + if (virConnectGetCPUMapDescEnsureACL(conn) < 0) + return NULL; + + return virGetCPUMapDesc(); +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, @@ -16220,6 +16229,7 @@ static virDriver qemuDriver = { .domainMigratePerform3Params = qemuDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = qemuDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = qemuDomainMigrateConfirm3Params, /* 1.1.0 */ + .connectGetCPUMapDesc = qemuConnectGetCPUMapDesc, /* 1.1.0 */ }; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 71d0034..017c7af 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6811,6 +6811,7 @@ static virDriver remote_driver = { .domainMigratePerform3Params = remoteDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = remoteDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = remoteDomainMigrateConfirm3Params, /* 1.1.0 */ + .connectGetCPUMapDesc = remoteConnectGetCPUMapDesc, /* 1.1.0 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 7cfebdf..6e17b41 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2837,6 +2837,10 @@ struct remote_domain_event_device_removed_msg { remote_nonnull_string devAlias; }; +struct remote_connect_get_cpu_map_desc_ret { + remote_nonnull_string xml; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -5000,5 +5004,11 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311 + REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311, + + /** + * @generate: both + * @acl: connect:read + */ + REMOTE_PROC_CONNECT_GET_CPU_MAP_DESC = 312 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 4e27aae..ce1e9f5 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2316,6 +2316,9 @@ struct remote_domain_event_device_removed_msg { remote_nonnull_domain dom; remote_nonnull_string devAlias; }; +struct remote_connect_get_cpu_map_desc_ret { + remote_nonnull_string xml; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -2628,4 +2631,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_MAP_DESC = 312, }; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index d7b2e40..82610b3 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5801,6 +5801,11 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED, return ret; } +static char * +testConnectGetCPUMapDesc(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return virGetCPUMapDesc(); +} static virDriver testDriver = { .no = VIR_DRV_TEST, @@ -5872,6 +5877,7 @@ static virDriver testDriver = { .connectIsAlive = testConnectIsAlive, /* 0.9.8 */ .nodeGetCPUMap = testNodeGetCPUMap, /* 1.0.0 */ .domainScreenshot = testDomainScreenshot, /* 1.0.5 */ + .connectGetCPUMapDesc = testConnectGetCPUMapDesc, /* 1.1.0 */ }; static virNetworkDriver testNetworkDriver = { diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 9ca352f..2aa2c08 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2831,6 +2831,16 @@ umlNodeSuspendForDuration(virConnectPtr conn, } +static char * +umlConnectGetCPUMapDesc(virConnectPtr conn) +{ + if (virConnectGetCPUMapDescEnsureACL(conn) < 0) + return NULL; + + return virGetCPUMapDesc(); +} + + static virDriver umlDriver = { .no = VIR_DRV_UML, .name = "UML", @@ -2892,6 +2902,7 @@ static virDriver umlDriver = { .nodeSuspendForDuration = umlNodeSuspendForDuration, /* 0.9.8 */ .nodeGetMemoryParameters = umlNodeGetMemoryParameters, /* 0.10.2 */ .nodeSetMemoryParameters = umlNodeSetMemoryParameters, /* 0.10.2 */ + .connectGetCPUMapDesc = umlConnectGetCPUMapDesc, /* 1.1.0 */ }; static virStateDriver umlStateDriver = { diff --git a/src/util/virutil.c b/src/util/virutil.c index 34f5998..9d71a53 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -80,6 +80,7 @@ #include "virprocess.h" #include "virstring.h" #include "virutil.h" +#include "configmake.h" #ifndef NSIG # define NSIG 32 @@ -2116,3 +2117,25 @@ cleanup: return rc; } + +char * +virGetCPUMapDesc(void) +{ + char *data, *ret = NULL; + int len; + const char *cpumapfile = PKGDATADIR "/cpu_map.xml"; + if ((len = virFileReadAll(cpumapfile, 1024 * 1024, &data)) < 0) { + goto cleanup; + } + + if (VIR_ALLOC_N(ret, len + 1) < 0) { + goto cleanup; + } + + memcpy(ret, data, len); + ret[len] = '\0'; + +cleanup: + VIR_FREE(data); + return ret; +} diff --git a/src/util/virutil.h b/src/util/virutil.h index 4b06992..11030c5 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -172,4 +172,7 @@ int virCompareLimitUlong(unsigned long long a, unsigned long b); int virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr); + +char *virGetCPUMapDesc(void); + #endif /* __VIR_UTIL_H__ */ diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 880ae4b..3bc4f73 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -626,6 +626,21 @@ cmdURI(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) return true; } +static bool +cmdCPUMapDesc(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + char *xml = virConnectGetCPUMapDesc(ctl->conn); + if (xml == NULL) { + vshError(ctl, "%s", _("failed to get CPU Map XML file")); + return false; + } + + vshPrint(ctl, "%s\n", xml); + VIR_FREE(xml); + + return true; +} + /* * "version" command */ @@ -851,6 +866,12 @@ const vshCmdDef hostAndHypervisorCmds[] = { .info = info_capabilities, .flags = 0 }, + {.name = "cpu-map-desc", + .handler = cmdCPUMapDesc, + .opts = NULL, + .info = info_uri, + .flags = 0 + }, {.name = "freecell", .handler = cmdFreecell, .opts = opts_freecell, diff --git a/tools/virsh.pod b/tools/virsh.pod index 0ae5178..d705a54 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-map-desc print the content of the cpu_map.xml file 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-map-desc> + +Print the content of the cpu_map.xml file used by the hypervisor. + =item B<running> The domain is currently running on a CPU -- 1.8.3.1
On Mon, Aug 19, 2013 at 1:19 PM, Giuseppe Scrivano <gscrivan@redhat.com>wrote:
--- I have started working on:
https://bugzilla.redhat.com/show_bug.cgi?id=916786
Before I split it in a series of commits, test it better and then proceed to virt-manager, are you ok with this idea?
include/libvirt/libvirt.h.in | 11 +++++++++++ src/driver.h | 4 ++++ src/libvirt.c | 38 ++++++++++++++++++++++++++++++++++++++ src/libvirt_private.syms | 1 + src/libvirt_public.syms | 1 + src/lxc/lxc_driver.c | 11 +++++++++++ src/qemu/qemu_driver.c | 10 ++++++++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++++++++++- src/remote_protocol-structs | 4 ++++ src/test/test_driver.c | 6 ++++++ src/uml/uml_driver.c | 11 +++++++++++ src/util/virutil.c | 23 +++++++++++++++++++++++ src/util/virutil.h | 3 +++ tools/virsh-host.c | 21 +++++++++++++++++++++ tools/virsh.pod | 5 +++++ 16 files changed, 161 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a47e33c..d6e0d9a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4006,6 +4006,17 @@ int virConnectCompareCPU(virConnectPtr conn, const char *xmlDesc, unsigned int flags);
+/** + * virConnectGetCPUMapDesc: + * + * @conn: virConnect connection + * + * Get the content of the cpu_map.xml file used by the connection. + * + * Returns XML description of the cpu_map.xml file. + */ +char *virConnectGetCPUMapDesc(virConnectPtr conn); +
So maybe others disagree with me but the naming really doesn't feel good here. I know that's a terrible reason but it really just feels like we're modeling the name of a file to an API. virConnectGetCPUTemplates(virConnectPtr conn) maybe? I don't really know. For hypervisors other than qemu, they won't call that cpu_map.xml.
/** * virConnectBaselineCPUFlags diff --git a/src/driver.h b/src/driver.h index be64333..ab31262 100644 --- a/src/driver.h +++ b/src/driver.h @@ -681,6 +681,9 @@ typedef char * unsigned int ncpus, unsigned int flags);
+typedef char * +(*virDrvConnectGetCPUMapDesc)(virConnectPtr conn); + typedef int (*virDrvDomainGetJobInfo)(virDomainPtr domain, virDomainJobInfoPtr info); @@ -1332,6 +1335,7 @@ struct _virDriver { virDrvDomainMigratePerform3Params domainMigratePerform3Params; virDrvDomainMigrateFinish3Params domainMigrateFinish3Params; virDrvDomainMigrateConfirm3Params domainMigrateConfirm3Params; + virDrvConnectGetCPUMapDesc connectGetCPUMapDesc; };
diff --git a/src/libvirt.c b/src/libvirt.c index 07a3fd5..5e5e594 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18519,6 +18519,44 @@ error:
/** + * virConnectGetCPUMapDesc: + * + * @conn: virConnect connection + * + * Get the content of the cpu_map.xml file used by the connection. + * + * Returns the content of the cpu_map.xml file. The result must be freed + * by the caller of this function. + */ +char * +virConnectGetCPUMapDesc(virConnectPtr conn) +{ + VIR_DEBUG("conn=%p", conn); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return NULL; + } + + if (conn->driver->connectGetCPUMapDesc) { + char *ret = conn->driver->connectGetCPUMapDesc(conn); + if (ret == NULL) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return NULL; +} + + +/** * virConnectBaselineCPU: * * @conn: virConnect connection diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c25a61f..ed6d2a0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2074,6 +2074,7 @@ virSetUIDGID; virSetUIDGIDWithCaps; virStrIsPrint; virValidateWWN; +virGetCPUMapDesc;
# util/viruuid.h diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index bbdf78a..8023c7e 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -629,6 +629,7 @@ LIBVIRT_1.1.0 {
LIBVIRT_1.1.1 { global: + virConnectGetCPUMapDesc; virDomainCreateWithFiles; virDomainCreateXMLWithFiles; virDomainSetMemoryStatsPeriod;
You can't add that to 1.1.1 since this won't actually be in 1.1.1. You need to do something like: LIBVIRT_1.1.2 { global: virConnectGetCPUMapDesc; } LIBVIRT_1.1.1;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 9cb95ff..633248b 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4590,6 +4590,16 @@ lxcNodeSuspendForDuration(virConnectPtr conn, }
+static char * +lxcConnectGetCPUMapDesc(virConnectPtr conn) +{ + if (virConnectGetCPUMapDescEnsureACL(conn) < 0) + return NULL; + + return virGetCPUMapDesc(); +} + + /* Function Tables */ static virDriver lxcDriver = { .no = VIR_DRV_LXC, @@ -4671,6 +4681,7 @@ static virDriver lxcDriver = { .domainShutdownFlags = lxcDomainShutdownFlags, /* 1.0.1 */ .domainReboot = lxcDomainReboot, /* 1.0.1 */ .domainLxcOpenNamespace = lxcDomainLxcOpenNamespace, /* 1.0.2 */ + .connectGetCPUMapDesc = lxcConnectGetCPUMapDesc, /* 1.1.0 */ };
Does this even make sense to wire up LXC? You can't change CPUs for LXC.
static virStateDriver lxcStateDriver = { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2ad236e..4a1d6fa 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16033,6 +16033,15 @@ qemuNodeSuspendForDuration(virConnectPtr conn, return nodeSuspendForDuration(target, duration, flags); }
+static char * +qemuConnectGetCPUMapDesc(virConnectPtr conn) +{ + if (virConnectGetCPUMapDescEnsureACL(conn) < 0) + return NULL; + + return virGetCPUMapDesc(); +} +
static virDriver qemuDriver = { .no = VIR_DRV_QEMU, @@ -16220,6 +16229,7 @@ static virDriver qemuDriver = { .domainMigratePerform3Params = qemuDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = qemuDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = qemuDomainMigrateConfirm3Params, /* 1.1.0 */ + .connectGetCPUMapDesc = qemuConnectGetCPUMapDesc, /* 1.1.0 */ };
1.1.0 is already released. The next version is 1.1.2
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 71d0034..017c7af 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6811,6 +6811,7 @@ static virDriver remote_driver = { .domainMigratePerform3Params = remoteDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = remoteDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = remoteDomainMigrateConfirm3Params, /* 1.1.0 */ + .connectGetCPUMapDesc = remoteConnectGetCPUMapDesc, /* 1.1.0 */ };
Same.
static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 7cfebdf..6e17b41 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2837,6 +2837,10 @@ struct remote_domain_event_device_removed_msg { remote_nonnull_string devAlias; };
+struct remote_connect_get_cpu_map_desc_ret { + remote_nonnull_string xml; +}; + /*----- Protocol. -----*/
/* Define the program number, protocol version and procedure numbers here. */ @@ -5000,5 +5004,11 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311 + REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311, + + /** + * @generate: both + * @acl: connect:read + */ + REMOTE_PROC_CONNECT_GET_CPU_MAP_DESC = 312 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 4e27aae..ce1e9f5 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2316,6 +2316,9 @@ struct remote_domain_event_device_removed_msg { remote_nonnull_domain dom; remote_nonnull_string devAlias; }; +struct remote_connect_get_cpu_map_desc_ret { + remote_nonnull_string xml; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -2628,4 +2631,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_MAP_DESC = 312, }; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index d7b2e40..82610b3 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5801,6 +5801,11 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED, return ret; }
+static char * +testConnectGetCPUMapDesc(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return virGetCPUMapDesc(); +}
static virDriver testDriver = { .no = VIR_DRV_TEST, @@ -5872,6 +5877,7 @@ static virDriver testDriver = { .connectIsAlive = testConnectIsAlive, /* 0.9.8 */ .nodeGetCPUMap = testNodeGetCPUMap, /* 1.0.0 */ .domainScreenshot = testDomainScreenshot, /* 1.0.5 */ + .connectGetCPUMapDesc = testConnectGetCPUMapDesc, /* 1.1.0 */ };
static virNetworkDriver testNetworkDriver = { diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 9ca352f..2aa2c08 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2831,6 +2831,16 @@ umlNodeSuspendForDuration(virConnectPtr conn, }
+static char * +umlConnectGetCPUMapDesc(virConnectPtr conn) +{ + if (virConnectGetCPUMapDescEnsureACL(conn) < 0) + return NULL; + + return virGetCPUMapDesc(); +} + + static virDriver umlDriver = { .no = VIR_DRV_UML, .name = "UML", @@ -2892,6 +2902,7 @@ static virDriver umlDriver = { .nodeSuspendForDuration = umlNodeSuspendForDuration, /* 0.9.8 */ .nodeGetMemoryParameters = umlNodeGetMemoryParameters, /* 0.10.2 */ .nodeSetMemoryParameters = umlNodeSetMemoryParameters, /* 0.10.2 */ + .connectGetCPUMapDesc = umlConnectGetCPUMapDesc, /* 1.1.0 */ };
1.1.2
static virStateDriver umlStateDriver = { diff --git a/src/util/virutil.c b/src/util/virutil.c index 34f5998..9d71a53 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -80,6 +80,7 @@ #include "virprocess.h" #include "virstring.h" #include "virutil.h" +#include "configmake.h"
#ifndef NSIG # define NSIG 32 @@ -2116,3 +2117,25 @@ cleanup:
return rc; } + +char * +virGetCPUMapDesc(void) +{ + char *data, *ret = NULL; + int len; + const char *cpumapfile = PKGDATADIR "/cpu_map.xml"; + if ((len = virFileReadAll(cpumapfile, 1024 * 1024, &data)) < 0) { + goto cleanup; + } + + if (VIR_ALLOC_N(ret, len + 1) < 0) { + goto cleanup; + } + + memcpy(ret, data, len); + ret[len] = '\0'; + +cleanup: + VIR_FREE(data); + return ret; +}
Does this make any sense to wire up for UML? Same case as LXC.
diff --git a/src/util/virutil.h b/src/util/virutil.h index 4b06992..11030c5 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -172,4 +172,7 @@ int virCompareLimitUlong(unsigned long long a, unsigned long b);
int virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr);
+ +char *virGetCPUMapDesc(void); + #endif /* __VIR_UTIL_H__ */ diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 880ae4b..3bc4f73 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -626,6 +626,21 @@ cmdURI(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) return true; }
+static bool +cmdCPUMapDesc(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + char *xml = virConnectGetCPUMapDesc(ctl->conn); + if (xml == NULL) { + vshError(ctl, "%s", _("failed to get CPU Map XML file")); + return false; + } + + vshPrint(ctl, "%s\n", xml); + VIR_FREE(xml); + + return true; +} + /* * "version" command */ @@ -851,6 +866,12 @@ const vshCmdDef hostAndHypervisorCmds[] = { .info = info_capabilities, .flags = 0 }, + {.name = "cpu-map-desc", + .handler = cmdCPUMapDesc, + .opts = NULL, + .info = info_uri, + .flags = 0 + },
Even if we keep the API name you suggested above, we definitely want a better name here. This is way too vague for someone to know what this command name is/does.
{.name = "freecell", .handler = cmdFreecell, .opts = opts_freecell, diff --git a/tools/virsh.pod b/tools/virsh.pod index 0ae5178..d705a54 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-map-desc print the content of the cpu_map.xml file 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-map-desc> + +Print the content of the cpu_map.xml file used by the hypervisor. + =item B<running>
The domain is currently running on a CPU -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
I'm wondering if this could some how be done as an extension to the virConnectBaselineCPU APIs? It would probably have to be another API entirely but at least similar in naming scope. Or potentially generic-ify this a bit more to make it like a virConnectHypervisorCapabilities() where right now it just returns back the CPUs the HV can emulate. In the future it can have support for other things if we need it to. Just trying to step back and see the potential for a bigger picture or expandability in the API in the future, if that's not what's wanted then ignore my comments. But do fix the versioning reviews above. -- Doug Goldstein
On 08/19/2013 01:22 PM, Doug Goldstein wrote:
On Mon, Aug 19, 2013 at 1:19 PM, Giuseppe Scrivano <gscrivan@redhat.com>wrote:
--- I have started working on:
https://bugzilla.redhat.com/show_bug.cgi?id=916786
Before I split it in a series of commits, test it better and then proceed to virt-manager, are you ok with this idea?
I'm wondering if this could some how be done as an extension to the virConnectBaselineCPU APIs? It would probably have to be another API entirely but at least similar in naming scope.
Not just that, but we JUST took a patch that enhanced the virConnectBaselineCPU API to take a flag argument: https://www.redhat.com/archives/libvir-list/2013-August/msg00150.html Looking at it further, it looks like the REAL problem to be solved is "given an emulator, which CPU models can I pass by name, and what CPU features will those models imply". It is completely independent of how the implementation stores it (that is, the fact that our qemu driver stores a cpu_map.xml lookup table should NOT be used in determining the function name).
Or potentially generic-ify this a bit more to make it like a virConnectHypervisorCapabilities() where right now it just returns back the CPUs the HV can emulate. In the future it can have support for other things if we need it to.
Listing all the capabilities of all the emulators in the existing virConnectHypervisorCapabilities may be too much XML for one RPC call; so a new API that lets us focus on one particular emulator is worthwhile. But yes, this is a better scheme to be copying from - we want a command where the user specifies an emulator, and the output is XML describing that emulator's capabilities according to what libvirt can expose. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Eric Blake <eblake@redhat.com> writes:
On 08/19/2013 01:22 PM, Doug Goldstein wrote:
Or potentially generic-ify this a bit more to make it like a virConnectHypervisorCapabilities() where right now it just returns back the CPUs the HV can emulate. In the future it can have support for other things if we need it to.
Listing all the capabilities of all the emulators in the existing virConnectHypervisorCapabilities may be too much XML for one RPC call; so a new API that lets us focus on one particular emulator is worthwhile. But yes, this is a better scheme to be copying from - we want a command where the user specifies an emulator, and the output is XML describing that emulator's capabilities according to what libvirt can expose.
thank you both for the input. I will rename the new function to virConnectGetHypervisorCapabilities and add a flags argument that controls what capabilities are included in the XML; for now both _ALL and _CPUS will return the same XML. Giuseppe
On Mon, Aug 19, 2013 at 2:47 PM, Eric Blake <eblake@redhat.com> wrote:
On 08/19/2013 01:22 PM, Doug Goldstein wrote:
On Mon, Aug 19, 2013 at 1:19 PM, Giuseppe Scrivano <gscrivan@redhat.com wrote:
--- I have started working on:
https://bugzilla.redhat.com/show_bug.cgi?id=916786
Before I split it in a series of commits, test it better and then proceed to virt-manager, are you ok with this idea?
I'm wondering if this could some how be done as an extension to the virConnectBaselineCPU APIs? It would probably have to be another API entirely but at least similar in naming scope.
Not just that, but we JUST took a patch that enhanced the virConnectBaselineCPU API to take a flag argument: https://www.redhat.com/archives/libvir-list/2013-August/msg00150.html
Looking at it further, it looks like the REAL problem to be solved is "given an emulator, which CPU models can I pass by name, and what CPU features will those models imply". It is completely independent of how the implementation stores it (that is, the fact that our qemu driver stores a cpu_map.xml lookup table should NOT be used in determining the function name).
Or potentially generic-ify this a bit more to make it like a virConnectHypervisorCapabilities() where right now it just returns back
CPUs the HV can emulate. In the future it can have support for other
the things
if we need it to.
Listing all the capabilities of all the emulators in the existing virConnectHypervisorCapabilities may be too much XML for one RPC call; so a new API that lets us focus on one particular emulator is worthwhile. But yes, this is a better scheme to be copying from - we want a command where the user specifies an emulator, and the output is XML describing that emulator's capabilities according to what libvirt can expose.
Right. I more meant adding a new API and not overloading the exist virConnectCapabilities() but a virConnectHypervisorCapabilities() that would allow us to query specific instances. I always wanted a way to see if a qemu had qxl support (and by extension spice). I'm just roughing this here: char *virConnectHypervisorCapabilities(virConnectPtr conn, const char *hv, int flags /* future */); where the 2nd arg would take the value from <emulator> from virConnectCapabilities(). or char *virConnectHypervisorCapabilities(virConnectPtr conn, virHypervisorsUghName hv, int flags /* future */); where the prior type is an enum that has VIR_HYPERVISOR_QEMU_ARM, VIR_HYPERVISOR_QEMU_X86. I don't really like the enum simply because there's so many kvm and qemu binaries you can have on the system it'd really be hard to match an enum to a specific one. We could inject another arg in between the 2nd and 3rd and that be the enum for data we're looking for. Like VIR_HYPERVISOR_CPU, VIR_HYPERVISOR_VIDEO, VIR_HYPERVISOR_SYSTEM (would return back 440fx and q35 for x86 based qemu's and something like vexpress-a9 for ARM). In a way I see this complimenting virConnectCapabilities() with virConnectHypervisorCapabilties() with some of the current items in the former appearing in the later (specifically from the <guest> section). The former would primarily have to roll of describing the system as a whole while the later would have the job of describing the specific emulator you're using. -- Doug Goldstein
On Mon, Aug 19, 2013 at 08:19:56PM +0200, Giuseppe Scrivano wrote:
--- I have started working on:
https://bugzilla.redhat.com/show_bug.cgi?id=916786
+/** + * virConnectGetCPUMapDesc: + * + * @conn: virConnect connection + * + * Get the content of the cpu_map.xml file used by the connection. + * + * Returns XML description of the cpu_map.xml file. + */ +char *virConnectGetCPUMapDesc(virConnectPtr conn);
This is really not at all neccessary, or desirable. We now have the ability to query the full CPU flag set from the virConnectBaselineCPU API. All we're missing is thus a way to get a list of supported CPU model names. For that we can just have int virConnectGetCPUModelNames(virConnectPtr conn, char ***models); Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 08/20/2013 05:50 AM, Daniel P. Berrange wrote:
On Mon, Aug 19, 2013 at 08:19:56PM +0200, Giuseppe Scrivano wrote:
--- I have started working on:
https://bugzilla.redhat.com/show_bug.cgi?id=916786
+/** + * virConnectGetCPUMapDesc: + * + * @conn: virConnect connection + * + * Get the content of the cpu_map.xml file used by the connection. + * + * Returns XML description of the cpu_map.xml file. + */ +char *virConnectGetCPUMapDesc(virConnectPtr conn);
This is really not at all neccessary, or desirable.
We now have the ability to query the full CPU flag set from the virConnectBaselineCPU API.
All we're missing is thus a way to get a list of supported CPU model names. For that we can just have
int virConnectGetCPUModelNames(virConnectPtr conn, char ***models);
That API would at least need receive an 'arch' value to limit the returned models. And there's other data in cpu_map than just the model name, though API users likely don't require it (yet). I think the suggestion elsewhere in the thread for a hypervisor capabilities or per-emulator capabilities API would work, and in the future we could extend it with things like available device lists. But we need to think hard about the XML format to make it as future proof as possible, since there's all sorts of crazy interdependencies between arch, machine type, domain type, hvm/pv/exe, etc. - Cole
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> --- I have collected your comments on my RFC patch into this new version. I've replaced "virConnectGetCPUMapDesc" with "virConnectGetCPUModelNames". The new function signature is: int virConnectGetCPUModelNames(virConnectPtr conn, const char *arch, char **models, unsigned int flags); It returns (in MODELS) the list of CPU models formatted as an XML document, like: <models> <arch name='x86'> <model name='486'/> <model name='pentium'/> <model name='pentium2'/> <model name='pentium3'/> <model name='pentiumpro'/> <model name='coreduo'/> ... </arch> </models> The FLAGS attribute is not used for now. daemon/remote.c | 36 +++++++++++++++++++++++++++ include/libvirt/libvirt.h.in | 18 ++++++++++++++ python/generator.py | 1 + python/libvirt-override-api.xml | 7 ++++++ python/libvirt-override.c | 28 +++++++++++++++++++++ python/libvirt-override.py | 11 +++++++++ src/cpu/cpu.c | 55 +++++++++++++++++++++++++++++++++++++++++ src/cpu/cpu.h | 3 +++ src/driver.h | 7 ++++++ src/libvirt.c | 42 +++++++++++++++++++++++++++++++ src/libvirt_private.syms | 1 + src/libvirt_public.syms | 5 ++++ src/qemu/qemu_driver.c | 14 +++++++++++ src/remote/remote_driver.c | 40 ++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 22 ++++++++++++++++- src/remote_protocol-structs | 8 ++++++ src/test/test_driver.c | 17 +++++++++++++ tools/virsh-host.c | 45 +++++++++++++++++++++++++++++++++ tools/virsh.pod | 5 ++++ 19 files changed, 364 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index 03d5557..6fc2d78 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4923,6 +4923,42 @@ 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) +{ + virDomainPtr dom = NULL; + int rv = -1; + char *models = NULL; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (virConnectGetCPUModelNames(priv->conn, args->arch, &models, + args->flags) < 0) + goto cleanup; + + ret->models = models; + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (dom) + virDomainFree(dom); + return rv; +} + + static int remoteDispatchDomainCreateXMLWithFiles( virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client, diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a47e33c..3819606 100644 --- a/include/libvirt/libvirt.h.in +++ 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: XML description containing the CPU models. The string is allocated + * by virConnectGetCPUModelNames and needs to be released using free() by the + * caller. + * @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); /** * virConnectBaselineCPUFlags diff --git a/python/generator.py b/python/generator.py index 427cebc..10d4a49 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 = [] 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..7537cf5 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2276,6 +2276,34 @@ 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 *pyobj_conn; + char *models; + 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; + + return PyString_FromString(models); +} + 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 diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 023ce26..5d05951 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -27,6 +27,7 @@ #include "viralloc.h" #include "virxml.h" #include "cpu.h" +#include "cpu_map.h" #include "cpu_x86.h" #include "cpu_powerpc.h" #include "cpu_s390.h" @@ -456,3 +457,57 @@ cpuModelIsAllowed(const char *model, } return false; } + +static int +cpuGetArchModelsCb(enum cpuMapElement element, + xmlXPathContextPtr ctxt, + void *data) +{ + virBufferPtr buf = (virBufferPtr) data; + if (element == CPU_MAP_ELEMENT_MODEL) { + const char *name = virXPathString("string(@name)", ctxt); + virBufferStrcat(buf, " <model name='", name, "'/>\n", NULL); + VIR_FREE(name); + return virBufferError(buf) ? -1 : 0; + } + return 0; +} + + +static int +cpuGetArchModels(virBufferPtr buf, const char *arch) +{ + return cpuMapLoad(arch, cpuGetArchModelsCb, buf); +} + + +int +cpuGetModels(const char *arch, char **models) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virBufferAddLit(&buf, "<models>\n"); + if (virBufferError(&buf)) + goto no_memory; + + virBufferStrcat(&buf, " <arch name='", arch, "'>\n", NULL); + if (virBufferError(&buf)) + goto no_memory; + + if (cpuGetArchModels(&buf, arch) < 0) + goto error; + + virBufferAddLit(&buf, " </arch>\n</models>\n"); + if (virBufferError(&buf)) + goto no_memory; + + *models = virBufferContentAndReset(&buf); + return 0; + +no_memory: + virReportOOMError(); + +error: + virBufferFreeAndReset(&buf); + return -1; +} diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 7f1d4bd..3e123b2 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -175,4 +175,7 @@ cpuModelIsAllowed(const char *model, const char **models, unsigned int nmodels); +extern int +cpuGetModels(const char *arch, char **models); + #endif /* __VIR_CPU_H__ */ diff --git a/src/driver.h b/src/driver.h index be64333..9cf74f5 100644 --- a/src/driver.h +++ b/src/driver.h @@ -682,6 +682,12 @@ typedef char * unsigned int flags); typedef int +(*virDrvConnectGetCPUModelNames)(virConnectPtr conn, + const char *args, + char **models, + unsigned int flags); + +typedef int (*virDrvDomainGetJobInfo)(virDomainPtr domain, virDomainJobInfoPtr info); @@ -1332,6 +1338,7 @@ struct _virDriver { virDrvDomainMigratePerform3Params domainMigratePerform3Params; virDrvDomainMigrateFinish3Params domainMigrateFinish3Params; virDrvDomainMigrateConfirm3Params domainMigrateConfirm3Params; + virDrvConnectGetCPUModelNames connectGetCPUModelNames; }; diff --git a/src/libvirt.c b/src/libvirt.c index 07a3fd5..56d6d1b 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18519,6 +18519,48 @@ error: /** + * virConnectGetCPUModelNames: + * + * @conn: virConnect connection + * @arch: Architecture + * @models: XML description containing the CPU models. The string is allocated + * by virConnectGetCPUModelNames and needs to be released using free() by the + * caller. + * @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) +{ + VIR_DEBUG("conn=%p", conn); + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (conn->driver->connectGetCPUModelNames) { + if (conn->driver->connectGetCPUModelNames(conn, arch, models, flags) < 0) + goto error; + + return 0; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return -1; +} + + +/** * virConnectBaselineCPU: * * @conn: virConnect connection diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c25a61f..e485bd2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -717,6 +717,7 @@ cpuCompareXML; cpuDataFree; cpuDecode; cpuEncode; +cpuGetModels; cpuGuestData; cpuHasFeature; cpuMapOverride; diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index bbdf78a..547b3ea 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -634,4 +634,9 @@ LIBVIRT_1.1.1 { virDomainSetMemoryStatsPeriod; } LIBVIRT_1.1.0; +LIBVIRT_1.1.2 { + global: + virConnectGetCPUModelNames; +} LIBVIRT_1.1.1; + # .... define new API here using predicted next version number .... diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2ad236e..c212b76 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16033,6 +16033,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, @@ -16220,6 +16233,7 @@ static virDriver qemuDriver = { .domainMigratePerform3Params = qemuDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = qemuDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = qemuDomainMigrateConfirm3Params, /* 1.1.0 */ + .connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.2 */ }; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 71d0034..ebbcc48 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5485,6 +5485,45 @@ done: static int +remoteConnectGetCPUModelNames(virConnectPtr conn, + const char *arch, + char **models, + unsigned int flags) +{ + int rv = -1; + 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; + + *models = ret.models; /* Caller frees. */ + + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; + +error: + rv = -1; + VIR_FREE(ret.models); + goto done; +} + + +static int remoteDomainOpenGraphics(virDomainPtr dom, unsigned int idx, int fd, @@ -6811,6 +6850,7 @@ static virDriver remote_driver = { .domainMigratePerform3Params = remoteDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = remoteDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = remoteDomainMigrateConfirm3Params, /* 1.1.0 */ + .connectGetCPUModelNames = remoteConnectGetCPUModelNames, /* 1.1.2 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 7cfebdf..7ef6c61 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -234,6 +234,11 @@ const REMOTE_DOMAIN_DISK_ERRORS_MAX = 256; */ const REMOTE_NODE_MEMORY_PARAMETERS_MAX = 64; +/* + * Upper limit on lists of CPU models + */ +const REMOTE_INTERFACE_CPU_MODELS_MAX = 16384; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN]; @@ -2837,6 +2842,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; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -5000,5 +5014,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..a2c3b14 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2316,6 +2316,13 @@ 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 { + remote_nonnull_string models; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -2628,4 +2635,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, }; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index d7b2e40..51baf18 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5801,6 +5801,22 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED, return ret; } +static int +testConnectGetCPUModelNames(virConnectPtr conn ATTRIBUTE_UNUSED, + const char *arch, + char **models, + unsigned int flags) +{ + char *ret; + virCheckFlags(0, -1); + if (virAsprintf(&ret, "<models>\n" + " <arch name='%s'/>\n" + "</models>\n", arch) < 0) + return -1; + + *models = ret; + return 0; +} static virDriver testDriver = { .no = VIR_DRV_TEST, @@ -5872,6 +5888,7 @@ static virDriver testDriver = { .connectIsAlive = testConnectIsAlive, /* 0.9.8 */ .nodeGetCPUMap = testNodeGetCPUMap, /* 1.0.0 */ .domainScreenshot = testDomainScreenshot, /* 1.0.5 */ + .connectGetCPUModelNames = testConnectGetCPUModelNames, /* 1.1.2 */ }; static virNetworkDriver testNetworkDriver = { diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 880ae4b..cc210b4 100644 --- a/tools/virsh-host.c +++ 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.") + }, + {.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 cmdFreecell(vshControl *ctl, const vshCmd *cmd) { @@ -626,6 +645,26 @@ cmdURI(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) return true; } +static bool +cmdCPUModelNames(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + char *models; + 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")); + return false; + } + + vshPrint(ctl, "%s\n", models); + VIR_FREE(models); + + return true; +} + /* * "version" command */ @@ -851,6 +890,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
On 08/21/2013 01:02 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> --- I have collected your comments on my RFC patch into this new version. I've replaced "virConnectGetCPUMapDesc" with "virConnectGetCPUModelNames".
Good that the above paragraph is below the ---...
The new function signature is:
int virConnectGetCPUModelNames(virConnectPtr conn, const char *arch, char **models, unsigned int flags);
It returns (in MODELS) the list of CPU models formatted as an XML document, like:
<models> <arch name='x86'> <model name='486'/> <model name='pentium'/> <model name='pentium2'/> <model name='pentium3'/> <model name='pentiumpro'/> <model name='coreduo'/> ... </arch> </models>
...but this is useful 6 months down the road, and should be in the commit message proper, above the ---. I'm not sure whether returning XML or a straight-up list makes more sense. If you used char ***models, then the user would get an array of directly-usable strings, "486", "pentium", ..., instead of a document that has to be parsed. On the other hand, your idea of returning XML lets us return information for multiple arches simultaneously. But do we need that flexibility, since arch is also an input parameter? Is the idea that you pass arch=NULL to get the full list, or arch="x86" to get the x86 subset of the xml? Why not just make arch mandatory and return char ***; but then you have the question of which arches are supported. So, let's get agreement on the best design before worrying about implementation (I'm still 50/50 on whether xml vs. char*** makes more sense, without more discussion to sway me one way or the other).
daemon/remote.c | 36 +++++++++++++++++++++++++++ include/libvirt/libvirt.h.in | 18 ++++++++++++++ python/generator.py | 1 + python/libvirt-override-api.xml | 7 ++++++ python/libvirt-override.c | 28 +++++++++++++++++++++ python/libvirt-override.py | 11 +++++++++ src/cpu/cpu.c | 55 +++++++++++++++++++++++++++++++++++++++++ src/cpu/cpu.h | 3 +++ src/driver.h | 7 ++++++ src/libvirt.c | 42 +++++++++++++++++++++++++++++++ src/libvirt_private.syms | 1 + src/libvirt_public.syms | 5 ++++ src/qemu/qemu_driver.c | 14 +++++++++++ src/remote/remote_driver.c | 40 ++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 22 ++++++++++++++++- src/remote_protocol-structs | 8 ++++++ src/test/test_driver.c | 17 +++++++++++++ tools/virsh-host.c | 45 +++++++++++++++++++++++++++++++++ tools/virsh.pod | 5 ++++
This is a rather big patch. When adding new API, it's best to do it in pieces: 1. use of the API itself (include, tools, src/driver, src/libvirt*) 2. RPC support (daemon, src/remote) 3. qemu support (src/qemu) 4. test support (src/test) 5. python bindings (python) as long as each part builds and passes 'make check'. I did not review the patch itself, on the grounds that getting the design right, and the patch split, will make review easier. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Eric Blake <eblake@redhat.com> writes:
I'm not sure whether returning XML or a straight-up list makes more sense. If you used char ***models, then the user would get an array of directly-usable strings, "486", "pentium", ..., instead of a document that has to be parsed. On the other hand, your idea of returning XML lets us return information for multiple arches simultaneously. But do we need that flexibility, since arch is also an input parameter? Is the idea that you pass arch=NULL to get the full list, or arch="x86" to get the x86 subset of the xml? Why not just make arch mandatory and return char ***; but then you have the question of which arches are supported.
I've thought a bit about XML vs Array and whether specifying the arch in the returned XML snippet or not, and these are the reasons behind my choice: 1) leave room for VIR_CONNECT_GET_CPU_MODEL_NAMES_EXPAND_FEATURES, in the same way as it is done by virConnectBaselineCPU. It might turn useful in future to get all the data in one shot, without requiring additional round-trips for each model trough virConnectBaselineCPU. 2) as you said, support arch=NULL to get the full list (and now that I've thought more about it, I should change the error to VIR_ERR_NO_SUPPORT when arch==NULL instead of raising an internal error). So if these two features will be implemented at some point, it will still be possible trough this new function to get the same functionalities of the "virConnectGetCPUMapDesc" function that I have proposed in my RFC.
So, let's get agreement on the best design before worrying about implementation (I'm still 50/50 on whether xml vs. char*** makes more sense, without more discussion to sway me one way or the other).
Right.
This is a rather big patch. When adding new API, it's best to do it in pieces:
1. use of the API itself (include, tools, src/driver, src/libvirt*) 2. RPC support (daemon, src/remote) 3. qemu support (src/qemu) 4. test support (src/test) 5. python bindings (python)
as long as each part builds and passes 'make check'.
I'll split the patch in a series before resubmitting it. Giuseppe
On Wed, Aug 21, 2013 at 10:46:36PM +0200, Giuseppe Scrivano wrote:
Eric Blake <eblake@redhat.com> writes:
I'm not sure whether returning XML or a straight-up list makes more sense. If you used char ***models, then the user would get an array of directly-usable strings, "486", "pentium", ..., instead of a document that has to be parsed. On the other hand, your idea of returning XML lets us return information for multiple arches simultaneously. But do we need that flexibility, since arch is also an input parameter? Is the idea that you pass arch=NULL to get the full list, or arch="x86" to get the x86 subset of the xml? Why not just make arch mandatory and return char ***; but then you have the question of which arches are supported.
I've thought a bit about XML vs Array and whether specifying the arch in the returned XML snippet or not, and these are the reasons behind my choice:
1) leave room for VIR_CONNECT_GET_CPU_MODEL_NAMES_EXPAND_FEATURES, in the same way as it is done by virConnectBaselineCPU. It might turn useful in future to get all the data in one shot, without requiring additional round-trips for each model trough virConnectBaselineCPU.
I don't think returning a list of all cpus with all features is an operation that will be common, or performance critical. So do not want to see us replicating virConnectBaselineCPU functionality here.
2) as you said, support arch=NULL to get the full list (and now that I've thought more about it, I should change the error to VIR_ERR_NO_SUPPORT when arch==NULL instead of raising an internal error).
I'd strictly forbid arch=NULL in the API and wire protocol (ie use remote_nonnull_string arch). Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
"Daniel P. Berrange" <berrange@redhat.com> writes:
2) as you said, support arch=NULL to get the full list (and now that I've thought more about it, I should change the error to VIR_ERR_NO_SUPPORT when arch==NULL instead of raising an internal error).
I'd strictly forbid arch=NULL in the API and wire protocol (ie use remote_nonnull_string arch).
How do we get the set of supported architectures? I'd like that for completeness sake we take this into account now, even if there is no real use (yet?). Giuseppe
On Thu, Aug 22, 2013 at 12:06:55PM +0200, Giuseppe Scrivano wrote:
"Daniel P. Berrange" <berrange@redhat.com> writes:
2) as you said, support arch=NULL to get the full list (and now that I've thought more about it, I should change the error to VIR_ERR_NO_SUPPORT when arch==NULL instead of raising an internal error).
I'd strictly forbid arch=NULL in the API and wire protocol (ie use remote_nonnull_string arch).
How do we get the set of supported architectures? I'd like that for completeness sake we take this into account now, even if there is no real use (yet?).
The capabilities XML shows you all architectures that libvirt knows about. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Giuseppe Scrivano (5): cpu_models: add new public API cpu_models: implement the remote protocol cpu_models: add the support for qemu cpu_models: add the support for the test protocol cpu_models: add Python bindings daemon/remote.c | 37 ++++++++++++++++++++++ include/libvirt/libvirt.h.in | 18 +++++++++++ python/generator.py | 1 + python/libvirt-override-api.xml | 7 +++++ python/libvirt-override.c | 56 +++++++++++++++++++++++++++++++++ python/libvirt-override.py | 11 +++++++ src/cpu/cpu.c | 69 +++++++++++++++++++++++++++++++++++++++++ src/cpu/cpu.h | 3 ++ src/driver.h | 7 +++++ src/libvirt.c | 47 ++++++++++++++++++++++++++++ src/libvirt_private.syms | 1 + src/libvirt_public.syms | 5 +++ src/qemu/qemu_driver.c | 14 +++++++++ src/remote/remote_driver.c | 51 ++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 17 +++++++++- src/remote_protocol-structs | 11 +++++++ src/test/test_driver.c | 16 ++++++++++ tools/virsh-host.c | 48 ++++++++++++++++++++++++++++ tools/virsh.pod | 5 +++ 19 files changed, 423 insertions(+), 1 deletion(-) -- 1.8.3.1
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> --- It explicitly disables the Python bindings as generator.py doesn't deal correctly with the new API. Patch 5 in this series will implement the Python bindings. include/libvirt/libvirt.h.in | 18 ++++++++++++ python/generator.py | 1 + src/cpu/cpu.c | 69 ++++++++++++++++++++++++++++++++++++++++++++ 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, 204 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a47e33c..43fb738 100644 --- a/include/libvirt/libvirt.h.in +++ 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); /** * virConnectBaselineCPUFlags diff --git a/python/generator.py b/python/generator.py index 427cebc..f8ac100 100755 --- a/python/generator.py +++ b/python/generator.py @@ -366,6 +366,7 @@ 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/src/cpu/cpu.c b/src/cpu/cpu.c index 023ce26..6abb173 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -27,6 +27,7 @@ #include "viralloc.h" #include "virxml.h" #include "cpu.h" +#include "cpu_map.h" #include "cpu_x86.h" #include "cpu_powerpc.h" #include "cpu_s390.h" @@ -456,3 +457,71 @@ cpuModelIsAllowed(const char *model, } return false; } + +struct cpuGetModelsData +{ + char **data; + unsigned int used; + unsigned int len; /* Doesn't count the last element DATA (NULL). */ +}; + +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 (data->used == data->len) { + data->len *= 2; + if (VIR_REALLOC_N(data->data, data->len + 1) < 0) + return -1; + } + + data->data[data->used++] = name; + data->data[data->used] = NULL; + return 0; +} + + +static int +cpuGetArchModels(const char *arch, struct cpuGetModelsData *data) +{ + return cpuMapLoad(arch, cpuGetArchModelsCb, data); +} + + +int +cpuGetModels(const char *arch, char ***models) +{ + struct cpuGetModelsData data; + + *models = data.data = NULL; + data.used = 0; + data.len = 8; + + if (VIR_ALLOC_N(data.data, data.len + 1) < 0) + goto error; + + if (cpuGetArchModels(arch, &data) < 0) + goto error; + + *models = data.data; + return 0; + +error: + if (data.data) { + char **it; + for (it = data.data; *it; it++) + VIR_FREE(*it); + VIR_FREE(data.data); + } + return -1; +} diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 7f1d4bd..ba22919 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -175,4 +175,7 @@ cpuModelIsAllowed(const char *model, const char **models, unsigned int nmodels); +extern int +cpuGetModels(const char *arch, char ***models); + #endif /* __VIR_CPU_H__ */ diff --git a/src/driver.h b/src/driver.h index be64333..8cd164a 100644 --- a/src/driver.h +++ b/src/driver.h @@ -682,6 +682,12 @@ typedef char * unsigned int flags); typedef int +(*virDrvConnectGetCPUModelNames)(virConnectPtr conn, + const char *args, + char ***models, + unsigned int flags); + +typedef int (*virDrvDomainGetJobInfo)(virDomainPtr domain, virDomainJobInfoPtr info); @@ -1332,6 +1338,7 @@ struct _virDriver { virDrvDomainMigratePerform3Params domainMigratePerform3Params; virDrvDomainMigrateFinish3Params domainMigrateFinish3Params; virDrvDomainMigrateConfirm3Params domainMigrateConfirm3Params; + virDrvConnectGetCPUModelNames connectGetCPUModelNames; }; diff --git a/src/libvirt.c b/src/libvirt.c index 07a3fd5..2032519 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18519,6 +18519,53 @@ error: /** + * 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) +{ + VIR_DEBUG("conn=%p", conn); + 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; + } + + if (conn->driver->connectGetCPUModelNames) { + if (conn->driver->connectGetCPUModelNames(conn, arch, models, flags) < 0) + goto error; + + return 0; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return -1; +} + + +/** * virConnectBaselineCPU: * * @conn: virConnect connection diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c25a61f..e485bd2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -717,6 +717,7 @@ cpuCompareXML; cpuDataFree; cpuDecode; cpuEncode; +cpuGetModels; cpuGuestData; cpuHasFeature; cpuMapOverride; diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index bbdf78a..547b3ea 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -634,4 +634,9 @@ LIBVIRT_1.1.1 { virDomainSetMemoryStatsPeriod; } LIBVIRT_1.1.0; +LIBVIRT_1.1.2 { + global: + virConnectGetCPUModelNames; +} LIBVIRT_1.1.1; + # .... define new API here using predicted next version number .... diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 880ae4b..ebab5ae 100644 --- a/tools/virsh-host.c +++ 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.") + }, + {.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 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) +{ + 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")); + return false; + } + + for (it = models; *it; it++) { + vshPrint(ctl, "%s\n", *it); + VIR_FREE(*it); + } + VIR_FREE(models); + + return true; +} + /* * "version" command */ @@ -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 + }, {.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@redhat.com> --- daemon/remote.c | 37 ++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 51 ++++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 17 ++++++++++++++- src/remote_protocol-structs | 11 ++++++++++ 4 files changed, 115 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index 03d5557..e33f616 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4923,6 +4923,43 @@ 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 rv = -1; + char **models; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (virConnectGetCPUModelNames(priv->conn, args->arch, &models, + args->flags) < 0) + goto cleanup; + + ret->models.models_val = models; + ret->models.models_len = 0; + while (*models++) + ret->models.models_len++; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + return rv; +} + + static int remoteDispatchDomainCreateXMLWithFiles( virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 71d0034..eb0bca2 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5485,6 +5485,56 @@ 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; + + 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 = 0; + +done: + remoteDriverUnlock(priv); + return rv; + +error: + rv = -1; + for (i = 0; i < ret.models.models_len; i++) + VIR_FREE(ret.models.models_val[i]); + VIR_FREE(ret.models.models_val); + goto done; +} + + +static int remoteDomainOpenGraphics(virDomainPtr dom, unsigned int idx, int fd, @@ -6811,6 +6861,7 @@ static virDriver remote_driver = { .domainMigratePerform3Params = remoteDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = remoteDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = remoteDomainMigrateConfirm3Params, /* 1.1.0 */ + .connectGetCPUModelNames = remoteConnectGetCPUModelNames, /* 1.1.2 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 7cfebdf..924d629 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2837,6 +2837,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<>; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -5000,5 +5009,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
On Thu, Aug 22, 2013 at 09:19:14PM +0200, Giuseppe Scrivano wrote:
4 files changed, 115 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 7cfebdf..924d629 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2837,6 +2837,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<>;
Use of <> is forbidden - please define a constant for the upper bound of this array and check the length on both client and server side. (Yes, some existing APIs don't do this & they are being fixed). Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
"Daniel P. Berrange" <berrange@redhat.com> writes:
+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<>;
Use of <> is forbidden - please define a constant for the upper bound of this array and check the length on both client and server side.
(Yes, some existing APIs don't do this & they are being fixed).
oops, I had that in the first implementation and then I removed it as I thought other APIs had no arbitrary limit on purpose. I will amend this change in the next version. Thanks, Giuseppe
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- 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 0a8e518..cb61243 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16035,6 +16035,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, @@ -16222,6 +16235,7 @@ static virDriver qemuDriver = { .domainMigratePerform3Params = qemuDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = qemuDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = qemuDomainMigrateConfirm3Params, /* 1.1.0 */ + .connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.2 */ }; -- 1.8.3.1
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- src/test/test_driver.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index d7b2e40..9b73532 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5801,6 +5801,21 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED, return ret; } +static int +testConnectGetCPUModelNames(virConnectPtr conn ATTRIBUTE_UNUSED, + const char *arch ATTRIBUTE_UNUSED, + char ***models, + unsigned int flags) +{ + char **null_list; + + virCheckFlags(0, -1); + if (VIR_ALLOC_N(null_list, 1) < 0) + return -1; + + *models = null_list; + return 0; +} static virDriver testDriver = { .no = VIR_DRV_TEST, @@ -5872,6 +5887,7 @@ static virDriver testDriver = { .connectIsAlive = testConnectIsAlive, /* 0.9.8 */ .nodeGetCPUMap = testNodeGetCPUMap, /* 1.0.0 */ .domainScreenshot = testDomainScreenshot, /* 1.0.5 */ + .connectGetCPUModelNames = testConnectGetCPUModelNames, /* 1.1.2 */ }; static virNetworkDriver testNetworkDriver = { -- 1.8.3.1
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- python/generator.py | 2 +- python/libvirt-override-api.xml | 7 ++++++ python/libvirt-override.c | 56 +++++++++++++++++++++++++++++++++++++++++ python/libvirt-override.py | 11 ++++++++ 4 files changed, 75 insertions(+), 1 deletion(-) diff --git a/python/generator.py b/python/generator.py index f8ac100..10d4a49 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..8536561 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2276,6 +2276,62 @@ 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, *pyobj_conn; + char **it, **models = NULL; + int flags = 0; + const char *arch = NULL; + unsigned int len; + + 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; + + len = 0; + for (it = models; *it; it++) + len++; + + if ((rv = PyList_New(len)) == NULL) + goto error; + + len = 0; + for (it = models; *it; it++){ + PyObject *str; + if ((str = PyString_FromString(*it)) == NULL) + goto error; + + PyList_SET_ITEM(rv, len++, str); + } + +done: + if (models) { + for (it = models; *it; it++) + VIR_FREE(*it); + VIR_FREE(models); + } + + return rv; + +error: + 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
On Wed, Aug 21, 2013 at 01:19:24PM -0600, Eric Blake wrote:
On 08/21/2013 01:02 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> --- I have collected your comments on my RFC patch into this new version. I've replaced "virConnectGetCPUMapDesc" with "virConnectGetCPUModelNames".
Good that the above paragraph is below the ---...
The new function signature is:
int virConnectGetCPUModelNames(virConnectPtr conn, const char *arch, char **models, unsigned int flags);
It returns (in MODELS) the list of CPU models formatted as an XML document, like:
<models> <arch name='x86'> <model name='486'/> <model name='pentium'/> <model name='pentium2'/> <model name='pentium3'/> <model name='pentiumpro'/> <model name='coreduo'/> ... </arch> </models>
...but this is useful 6 months down the road, and should be in the commit message proper, above the ---.
I'm not sure whether returning XML or a straight-up list makes more sense. If you used char ***models, then the user would get an array of directly-usable strings, "486", "pentium", ..., instead of a document that has to be parsed. On the other hand, your idea of returning XML lets us return information for multiple arches simultaneously. But do we need that flexibility, since arch is also an input parameter? Is the idea that you pass arch=NULL to get the full list, or arch="x86" to get the x86 subset of the xml? Why not just make arch mandatory and return char ***; but then you have the question of which arches are supported.
So, let's get agreement on the best design before worrying about implementation (I'm still 50/50 on whether xml vs. char*** makes more sense, without more discussion to sway me one way or the other).
My intention was that we return a direct list of strings representing model names, *not* any XML. XML only comes into play when they they query the full CPU feature set from virConnectBaselineCPU. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (5)
-
Cole Robinson -
Daniel P. Berrange -
Doug Goldstein -
Eric Blake -
Giuseppe Scrivano