[libvirt] [PATCH 11/17] Introduce remoteDomainPinHypervisorFlags and remoteDomainGetHypervisorPinInfo functions in remote driver.

[Eric Blake]

On 08/03/2012 12:36 AM, Hu Tao wrote:
> From: Tang Chen <tangchen at 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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120807/8175beeb/attachment-0001.sig>