On 08/03/2012 12:36 AM, Hu Tao wrote:
> From: Tang Chen <tangchen@xxxxxxxxxxxxxx>
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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list