[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/5] vcpupin: implement the remote protocol to address the new API



On 06/24/2011 03:00 AM, Taku Izumi wrote:
> 
> This patch implements the remote protocol to address the new API
> (virDomainGetVcpupinInfo).
> 
> Signed-off-by: Taku Izumi <izumi taku jp fujitsu com>
> ---
>  daemon/remote.c              |   68 ++++++++++++++++++++++++++++++++++++++++
>  src/remote/remote_driver.c   |   73 +++++++++++++++++++++++++++++++++++++++++++
>  src/remote/remote_protocol.x |   15 ++++++++
>  src/remote_protocol-structs  |   13 +++++++
>  4 files changed, 168 insertions(+), 1 deletion(-)
> 
> Index: libvirt/src/remote/remote_protocol.x
> ===================================================================
> --- libvirt.orig/src/remote/remote_protocol.x
> +++ libvirt/src/remote/remote_protocol.x
> @@ -903,6 +903,18 @@ struct remote_domain_pin_vcpu_flags_args
>      unsigned int flags;
>  };
>  
> +struct remote_domain_get_vcpupin_info_args {
> +    remote_nonnull_domain dom;
> +    int maxinfo;

Same renaming.

> @@ -2425,7 +2437,8 @@ enum remote_procedure {
>      REMOTE_PROC_DOMAIN_BLOCK_PULL_ABORT = 231, /* autogen autogen */
>      REMOTE_PROC_DOMAIN_GET_BLOCK_PULL_INFO = 232, /* autogen autogen */
>      REMOTE_PROC_DOMAIN_EVENT_BLOCK_PULL = 233, /* skipgen skipgen */
> -    REMOTE_PROC_DOMAIN_GET_CONTROL_INFO = 234 /* autogen autogen */
> +    REMOTE_PROC_DOMAIN_GET_CONTROL_INFO = 234, /* autogen autogen */
> +    REMOTE_PROC_DOMAIN_GET_VCPUPIN_INFO = 235  /* skipgen skipgen */

Minor merge conflict; easy enough to fix.

>  
>      /*
>       * Notice how the entries are grouped in sets of 10 ?
> Index: libvirt/src/remote/remote_driver.c
> ===================================================================
> --- libvirt.orig/src/remote/remote_driver.c
> +++ libvirt/src/remote/remote_driver.c
> @@ -2141,6 +2141,78 @@ done:
>  }
>  
>  static int
> +remoteDomainGetVcpupinInfo (virDomainPtr domain,
> +                            int maxinfo,

More renaming.

> +                            unsigned char *cpumaps,
> +                            int maplen,
> +                            unsigned int flags)
> +{
> +    int rv = -1;
> +    int i;
> +    remote_domain_get_vcpupin_info_args args;
> +    remote_domain_get_vcpupin_info_ret ret;
> +    struct private_data *priv = domain->conn->privateData;
> +
> +    remoteDriverLock(priv);
> +
> +    if (maxinfo > REMOTE_VCPUINFO_MAX) {
> +        remoteError(VIR_ERR_RPC,
> +                    _("vCPU count exceeds maximum: %d > %d"),
> +                    maxinfo, REMOTE_VCPUINFO_MAX);
> +        goto done;
> +    }
> +
> +    if (maxinfo * maplen > REMOTE_CPUMAPS_MAX) {

Ouch.  Potential for integer overflow causing wraparound that we don't
detect.  But no different than what you copied from, so I'll propose a
separate patch.

> +
> +    ret->num = num;
> +    /* Don't need to allocate/copy the cpumaps if we make the reasonabl

typo

> +     * assumption that unsigned char and char are the same size.

And in response to the comment, C99 requires that 'char' and 'unsigned
char' both be the same size - exactly 1.

ACK; here's what I'm squashing in:

diff --git i/daemon/remote.c w/daemon/remote.c
index 2f3a3e7..27d5dea 100644
--- i/daemon/remote.c
+++ w/daemon/remote.c
@@ -1068,34 +1068,34 @@ remoteDispatchDomainGetVcpupinInfo(struct
qemud_server *server ATTRIBUTE_UNUSED,
     if (!(dom = get_nonnull_domain(conn, args->dom)))
         goto cleanup;

-    if (args->maxinfo > REMOTE_VCPUINFO_MAX) {
-        virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo >
REMOTE_VCPUINFO_MAX"));
+    if (args->ncpumaps > REMOTE_VCPUINFO_MAX) {
+        virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("ncpumaps >
REMOTE_VCPUINFO_MAX"));
         goto cleanup;
     }

-    if (args->maxinfo * args->maplen > REMOTE_CPUMAPS_MAX) {
+    if (args->ncpumaps * args->maplen > REMOTE_CPUMAPS_MAX) {
         virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo * maplen >
REMOTE_CPUMAPS_MAX"));
         goto cleanup;
     }

     /* Allocate buffers to take the results. */
     if (args->maplen > 0 &&
-        VIR_ALLOC_N(cpumaps, args->maxinfo * args->maplen) < 0)
+        VIR_ALLOC_N(cpumaps, args->ncpumaps * args->maplen) < 0)
         goto no_memory;

     if ((num = virDomainGetVcpupinInfo(dom,
-                                       args->maxinfo,
+                                       args->ncpumaps,
                                        cpumaps,
                                        args->maplen,
                                        args->flags)) < 0)
         goto cleanup;

     ret->num = num;
-    /* Don't need to allocate/copy the cpumaps if we make the reasonabl
+    /* Don't need to allocate/copy the cpumaps if we make the reasonable
      * assumption that unsigned char and char are the same size.
      * Note that remoteDispatchClientRequest will free.
      */
-    ret->cpumaps.cpumaps_len = args->maxinfo * args->maplen;
+    ret->cpumaps.cpumaps_len = args->ncpumaps * args->maplen;
     ret->cpumaps.cpumaps_val = (char *) cpumaps;
     cpumaps = NULL;

diff --git i/src/remote/remote_driver.c w/src/remote/remote_driver.c
index d6b5c5d..8ddb2c9 100644
--- i/src/remote/remote_driver.c
+++ w/src/remote/remote_driver.c
@@ -2142,7 +2142,7 @@ done:

 static int
 remoteDomainGetVcpupinInfo (virDomainPtr domain,
-                            int maxinfo,
+                            int ncpumaps,
                             unsigned char *cpumaps,
                             int maplen,
                             unsigned int flags)
@@ -2155,22 +2155,22 @@ remoteDomainGetVcpupinInfo (virDomainPtr domain,

     remoteDriverLock(priv);

-    if (maxinfo > REMOTE_VCPUINFO_MAX) {
+    if (ncpumaps > REMOTE_VCPUINFO_MAX) {
         remoteError(VIR_ERR_RPC,
                     _("vCPU count exceeds maximum: %d > %d"),
-                    maxinfo, REMOTE_VCPUINFO_MAX);
+                    ncpumaps, REMOTE_VCPUINFO_MAX);
         goto done;
     }

-    if (maxinfo * maplen > REMOTE_CPUMAPS_MAX) {
+    if (ncpumaps * maplen > REMOTE_CPUMAPS_MAX) {
         remoteError(VIR_ERR_RPC,
                     _("vCPU map buffer length exceeds maximum: %d > %d"),
-                    maxinfo * maplen, REMOTE_CPUMAPS_MAX);
+                    ncpumaps * maplen, REMOTE_CPUMAPS_MAX);
         goto done;
     }

     make_nonnull_domain (&args.dom, domain);
-    args.maxinfo = maxinfo;
+    args.ncpumaps = ncpumaps;
     args.maplen = maplen;
     args.flags = flags;

@@ -2183,21 +2183,21 @@ remoteDomainGetVcpupinInfo (virDomainPtr domain,
               (char *) &ret) == -1)
         goto done;

-    if (ret.num > maxinfo) {
+    if (ret.num > ncpumaps) {
         remoteError(VIR_ERR_RPC,
                     _("host reports too many vCPUs: %d > %d"),
-                    ret.num, maxinfo);
+                    ret.num, ncpumaps);
         goto cleanup;
     }

-    if (ret.cpumaps.cpumaps_len > maxinfo * maplen) {
+    if (ret.cpumaps.cpumaps_len > ncpumaps * maplen) {
         remoteError(VIR_ERR_RPC,
                     _("host reports map buffer length exceeds maximum:
%d > %d"),
-                    ret.cpumaps.cpumaps_len, maxinfo * maplen);
+                    ret.cpumaps.cpumaps_len, ncpumaps * maplen);
         goto cleanup;
     }

-    memset (cpumaps, 0, maxinfo * maplen);
+    memset (cpumaps, 0, ncpumaps * maplen);

     for (i = 0; i < ret.cpumaps.cpumaps_len; ++i)
         cpumaps[i] = ret.cpumaps.cpumaps_val[i];
diff --git i/src/remote/remote_protocol.x w/src/remote/remote_protocol.x
index bb1dbe1..ee08b82 100644
--- i/src/remote/remote_protocol.x
+++ w/src/remote/remote_protocol.x
@@ -905,7 +905,7 @@ struct remote_domain_pin_vcpu_flags_args {

 struct remote_domain_get_vcpupin_info_args {
     remote_nonnull_domain dom;
-    int maxinfo;
+    int ncpumaps;
     int maplen;
     unsigned int flags;
 };
diff --git i/src/remote_protocol-structs w/src/remote_protocol-structs
index aed5c32..be98135 100644
--- i/src/remote_protocol-structs
+++ w/src/remote_protocol-structs
@@ -582,7 +582,7 @@ struct remote_domain_pin_vcpu_flags_args {
 };
 struct remote_domain_get_vcpupin_info_args {
         remote_nonnull_domain      dom;
-        int                        maxinfo;
+        int                        ncpumaps;
         int                        maplen;
         u_int                      flags;
 };


-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]