On 11/01/2012 09:57 PM, Eric Blake wrote:

It turns out that calling virNodeGetCPUMap(conn, NULL, NULL, 0) is both useful, and with Viktor's patches, common enough to optimize. Since this interface hasn't been released yet, we can change the RPC call.

* src/nodeinfo.c (nodeGetCPUMap): Avoid bitmap when not needed. * src/remote/remote_protocol.x (remote_node_get_cpu_map_args): Supply two separate flags for needed arguments. * src/remote/remote_driver.c (remoteNodeGetCPUMap): Update caller. * daemon/remote.c (remoteDispatchNodeGetCPUMap): Likewise. * src/remote_protocol-structs: Regenerate. ---

This has to be applied before 1.0.0 if we want it; otherwise the change to RPC wire protocol will render this patch an ABI break.

I thought of one alternative, that wouldn't change the RPC size, but still would be a difference. That would be using the old need_results int as a bitmap of which of the two arguments are needed, instead of just a 1 or 0. But even that is risky enough that it should get in before we commit to this interface.

daemon/remote.c | 8 ++++---- src/nodeinfo.c | 3 +++ src/remote/remote_driver.c | 3 ++- src/remote/remote_protocol.x | 3 ++- src/remote_protocol-structs | 3 ++- 5 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c index 7a9df60..340d07d 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4578,7 +4578,7 @@ remoteDispatchNodeGetCPUMap(virNetServerPtr server ATTRIBUTE_UNUSED, remote_node_get_cpu_map_ret *ret) { unsigned char *cpumap = NULL; - unsigned int online; + unsigned int online = 0; unsigned int flags; int cpunum; int rv = -1; @@ -4592,13 +4592,13 @@ remoteDispatchNodeGetCPUMap(virNetServerPtr server ATTRIBUTE_UNUSED,

flags = args->flags;

- cpunum = virNodeGetCPUMap(priv->conn, args->need_results ? &cpumap : NULL, - &online, flags); + cpunum = virNodeGetCPUMap(priv->conn, args->need_map ? &cpumap : NULL, + args->need_online ? &online : NULL, flags); if (cpunum < 0) goto cleanup;

/* 'serialize' return cpumap */ - if (args->need_results) { + if (args->need_map) { ret->cpumap.cpumap_len = VIR_CPU_MAPLEN(cpunum); ret->cpumap.cpumap_val = (char *) cpumap; cpumap = NULL; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 35c5f96..3348ae7 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1277,6 +1277,9 @@ nodeGetCPUMap(virConnectPtr conn ATTRIBUTE_UNUSED,

virCheckFlags(0, -1);

+ if (!cpumap && !online) + return nodeGetCPUCount(); + if (!(cpus = nodeGetCPUBitmap(&maxpresent))) goto cleanup;

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 71218f0..5eca0fa 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5793,7 +5793,8 @@ remoteNodeGetCPUMap(virConnectPtr conn,

remoteDriverLock(priv);

- args.need_results = !!cpumap; + args.need_map = !!cpumap; + args.need_online = !!online; args.flags = flags;

memset (&ret, 0, sizeof(ret)); diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 765ffcd..d6ac3c1 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2671,7 +2671,8 @@ struct remote_node_get_memory_parameters_ret { };

struct remote_node_get_cpu_map_args { - int need_results; + int need_map; + int need_online; unsigned int flags; };

diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 567864a..6fe7213 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2127,7 +2127,8 @@ struct remote_node_get_memory_parameters_ret { int nparams; }; struct remote_node_get_cpu_map_args { - int need_results; + int need_map; + int need_online; u_int flags; }; struct remote_node_get_cpu_map_ret {

After a small bit of explaining the context surrounding the change on IRC, I understand what's going on and the benefit, and I agree that it's either now or never. ACK.