Re: [libvirt] [PATCH] Fix initialization of current vcpus in libxl driver

[Jim Fehlig <jfehlig novell com>]

Eric Blake wrote:
> On 05/24/2011 09:14 AM, Daniel Veillard wrote:
>   
>> On Mon, May 23, 2011 at 04:43:19PM -0600, Eric Blake wrote:
>>     
>>> On 05/23/2011 04:26 PM, Jim Fehlig wrote:
>>>       
>>>>      b_info->hvm = hvm;
>>>>      b_info->max_vcpus = def->maxvcpus;
>>>> -    b_info->cur_vcpus = def->vcpus;
>>>> +    b_info->cur_vcpus = (1 << def->vcpus) - 1;
>>>>         
>>> That's a compilation pitfall if def->vcpus is exactly 32.
>>>
>>> Shoot; xenFormatSxpr in xenxs/xen_sxpr.c has the same bug.
>>>       
>> What would be the best way to fix ? cast to a larger int and
>> recast the result or provide some kind of helper function for
>> bitmaps ?
>>     
>
> Maybe special-case:
>
> if (def->vcpus == 32)
>   b_info->cur_vcpus = (uint32_t) -1;
> else
>   b_info->cur_vcpus = (1 << def->vcpus) - 1;
>   

This works in the libxl driver since cur_vcpus is an int.  But
vcpu_avail is a long in xenxs/xen_sxpr.c and I'm hoping there is
something better than the following approach

diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index 4cebb21..dc000a8 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -2006,9 +2006,14 @@ xenFormatSxpr(virConnectPtr conn,
     virBufferAsprintf(&buf, "(vcpus %u)", def->maxvcpus);
     /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is
        either 32, or 64 on a platform where long is big enough.  */
-    if (def->vcpus < def->maxvcpus)
-        virBufferAsprintf(&buf, "(vcpu_avail %lu)",
-                          def->vcpus == 32 ? 1 << 31 : (1UL <<
def->vcpus) - 1);
+    if (def->vcpus < def->maxvcpus) {
+        if (sizeof(long) == 4 && def->vcpus == 32)
+            virBufferAsprintf(&buf, "(vcpu_avail %lu)", 1UL << 31);
+        else if (sizeof(long) == 8 && def->vcpus == 64)
+            virBufferAsprintf(&buf, "(vcpu_avail %lu)", 1UL << 63);
+        else
+            virBufferAsprintf(&buf, "(vcpu_avail %lu)", (1UL <<
def->vcpus) - 1);
+    }

     if (def->cpumask) {
         char *ranges = virDomainCpuSetFormat(def->cpumask,
def->cpumasklen);

Suggestions?

Thanks,
Jim