[libvirt] [RFC] Expose cpu_map.xml via API
Doug Goldstein
cardoe at gentoo.org
Mon Aug 19 19:22:59 UTC 2013
On Mon, Aug 19, 2013 at 1:19 PM, Giuseppe Scrivano <gscrivan at 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 at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130819/0404af9c/attachment-0001.htm>
More information about the libvir-list
mailing list