12:14 p.m.
On 08/03/2012 12:36 AM, Hu Tao wrote:
From: Tang Chen <tangchen(a)cn.fujitsu.com>
Again, a long subject line. Given Dan's rename suggestion, I propose:
remote: introduce emulator pinning RPCs
static int
+remoteDispatchDomainPinHypervisorFlags(virNetServerPtr server ATTRIBUTE_UNUSED,
No need for the 'Flags' suffix in the name.
+
+static int
+remoteDispatchDomainGetHypervisorPinInfo(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client ATTRIBUTE_UNUSED,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_domain_get_hypervisor_pin_info_args
*args,
+ remote_domain_get_hypervisor_pin_info_ret
*ret)
+{
+ virDomainPtr dom = NULL;
+ unsigned char *cpumaps = NULL;
+ int num;
+ int rv = -1;
+ struct daemonClientPrivate *priv =
+ virNetServerClientGetPrivateData(client);
+
+ if (!priv->conn) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
+ goto cleanup;
+ }
+
+ if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
+ goto cleanup;
+
+ /* There is only one cpumap struct for all hypervisor threads */
+ if (args->ncpumaps != 1) {
If that's true, then we don't need args->ncpumaps in the first place.
That's a tweak to make in the .x file.
+++ b/src/remote/remote_driver.c
@@ -1841,6 +1841,106 @@ done:
}
static int
+remoteDomainPinHypervisorFlags (virDomainPtr dom,
+ unsigned char *cpumap,
+ int cpumaplen,
+ unsigned int flags)
+{
+ int rv = -1;
+ struct private_data *priv = dom->conn->privateData;
+ remote_domain_pin_hypervisor_flags_args args;
+
+ remoteDriverLock(priv);
+
+ if (cpumaplen > REMOTE_CPUMAP_MAX) {
+ virReportError(VIR_ERR_RPC,
+ _("%s length greater than maximum: %d > %d"),
+ "cpumap", (int)cpumaplen, REMOTE_CPUMAP_MAX);
Why are we casting cpumaplen, which is already an int?
+static int
+remoteDomainGetHypervisorPinInfo (virDomainPtr domain,
+ unsigned char *cpumaps,
+ int maplen,
+ unsigned int flags)
+{
+ int rv = -1;
+ int i;
+ remote_domain_get_hypervisor_pin_info_args args;
+ remote_domain_get_hypervisor_pin_info_ret ret;
+ struct private_data *priv = domain->conn->privateData;
+
+ remoteDriverLock(priv);
+
+ /* There is only one cpumap for all hypervisor threads */
+ if (INT_MULTIPLY_OVERFLOW(1, maplen) ||
This expression is always false ('1 * anything' cannot overflow), and
therefore dead code worth simplifying.
@@ -5254,6 +5354,8 @@ static virDriver remote_driver = {
.domainPinVcpu = remoteDomainPinVcpu, /* 0.3.0 */
.domainPinVcpuFlags = remoteDomainPinVcpuFlags, /* 0.9.3 */
.domainGetVcpuPinInfo = remoteDomainGetVcpuPinInfo, /* 0.9.3 */
+ .domainPinHypervisorFlags = remoteDomainPinHypervisorFlags, /* 0.9.13 */
+ .domainGetHypervisorPinInfo = remoteDomainGetHypervisorPinInfo, /* 0.9.13 */
0.10.0
+++ b/src/remote/remote_protocol.x
@@ -1054,6 +1054,25 @@ struct remote_domain_get_vcpu_pin_info_ret {
int num;
};
+struct remote_domain_pin_hypervisor_flags_args {
Again, no need for '_flags' in the name.
+ remote_nonnull_domain dom;
+ unsigned int vcpu;
No need for a wasted 'vcpu' arg.
+ opaque cpumap<REMOTE_CPUMAP_MAX>; /* (unsigned char *) */
+ unsigned int flags;
+};
+
+struct remote_domain_get_hypervisor_pin_info_args {
+ remote_nonnull_domain dom;
+ int ncpumaps;
No need for an 'ncpumaps' arg.
+ int maplen;
+ unsigned int flags;
+};
+
+struct remote_domain_get_hypervisor_pin_info_ret {
+ opaque cpumaps<REMOTE_CPUMAPS_MAX>;
+ int num;
'num' is a bit misleading for a name, and probably not necessary. The
return value of virDomainGetHypervisorPinInfo is either 0 (no pinning
present) or 1 (pinning present and details returned) (or negative for
error, but that doesn't go through this RPC struct). If RPC allows for
0-length cpumaps<>, then you don't need num; otherwise, I'd rename num
to 'ret', since it is recording the return value of 0 or 1.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org