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

On 05/24/2011 11:22 AM, Jim Fehlig wrote: > The cur_vcpus member of struct libxl_domain_build_info was incorrectly > initialized to the number of vcpus, when it should have been interpreted > as a bitmap, where bit X corresponds to online/offline status of vcpuX. > > To complicate matters, cur_vcpus is an int, so only 32 vcpus can be > set online. Add a check to ensure vcpus does not exceed this limit. > > V2: Eric Blake noted a compilation pitfal when '1 << 32' on an int. > If you keep the V2 message in your commit log, then s/pitfal/pitfall/ (personally, I tend to drop V2 messages when pushing - they are handy during review to point out what to look for, but once it is part of history, I no longer care how many versions it took to get to the final pushed version). > Account for vcpus == 32. > > + * as an int, but its' semantic is a bitmap of online vcpus, so > s/its'/its/ (That is, drop the spurious apostrophe). > b_info->hvm = hvm; > b_info->max_vcpus = def->maxvcpus; > - b_info->cur_vcpus = def->vcpus; > + if (def->vcpus == 32) > + b_info->cur_vcpus = 1 << 31; > Still wrong. You want 32 bits, not one bit: b_info->cur_vcpus = (uint32_t) -1;

> + else > + b_info->cur_vcpus = (1 << def->vcpus) - 1; > if (def->clock.ntimers > 0 && > def->clock.timers[0]->name == VIR_DOMAIN_TIMER_NAME_TSC) { > switch (def->clock.timers[0]->mode) { > ACK with those changes.