[ruby PATCH] Fix cpumap allocation for virDomainGetVcpus and use return value

Charlie Smurthwaite posted 1 patch 4 years, 2 months ago
Failed in applying to current master (apply log)
ext/libvirt/domain.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
[ruby PATCH] Fix cpumap allocation for virDomainGetVcpus and use return value
Posted by Charlie Smurthwaite 4 years, 2 months ago
This patch fixes a bug in which only enough memory for one cpumap is 
allocated
for virDomainGetVcpus instead of one per virtual CPU. This Fixes an 
overflow.
Additionally, it uses the return value of virDomainGetVcpus to determine how
many cpuinfo structs were actually populated rather than assuming they 
all are.
Finally, it uses the logical CPU number from the cpuinfo struct in the 
retrurn
data instead of assuming CPU numbers are sequential. This should handle 
cases
where arbitrary CPUs are offline.

Signed-off-by: Charlie Smurthwaite <charlie@atechmedia.com>
---
ext/libvirt/domain.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/ext/libvirt/domain.c b/ext/libvirt/domain.c
index d665907..c6de1bf 100644
--- a/ext/libvirt/domain.c
+++ b/ext/libvirt/domain.c
@@ -803,7 +803,7 @@ static VALUE libvirt_domain_vcpus(VALUE d)
cpumaplen = VIR_CPU_MAPLEN(maxcpus);
- cpumap = alloca(sizeof(unsigned char) * cpumaplen);
+ cpumap = alloca(sizeof(unsigned char) * cpumaplen * dominfo.nrVirtCpu);
r = virDomainGetVcpus(ruby_libvirt_domain_get(d), cpuinfo,
dominfo.nrVirtCpu, cpumap, cpumaplen);
@@ -832,15 +832,16 @@ static VALUE libvirt_domain_vcpus(VALUE d)
result = rb_ary_new();
- for (i = 0; i < dominfo.nrVirtCpu; i++) {
+ for (i = 0; i < r; i++) {
vcpuinfo = rb_class_new_instance(0, NULL, c_domain_vcpuinfo);
- rb_iv_set(vcpuinfo, "@number", UINT2NUM(i));
if (cpuinfo != NULL) {
+ rb_iv_set(vcpuinfo, "@number", INT2NUM(cpuinfo[i].number));
rb_iv_set(vcpuinfo, "@state", INT2NUM(cpuinfo[i].state));
rb_iv_set(vcpuinfo, "@cpu_time", ULL2NUM(cpuinfo[i].cpuTime));
rb_iv_set(vcpuinfo, "@cpu", INT2NUM(cpuinfo[i].cpu));
}
else {
+ rb_iv_set(vcpuinfo, "@number", Qnil);
rb_iv_set(vcpuinfo, "@state", Qnil);
rb_iv_set(vcpuinfo, "@cpu_time", Qnil);
rb_iv_set(vcpuinfo, "@cpu", Qnil);

-- 
2.25.0

Re: [ruby PATCH] Fix cpumap allocation for virDomainGetVcpus and use return value
Posted by Ján Tomko 4 years, 1 month ago
On Thu, Feb 20, 2020 at 11:01:32AM +0000, Charlie Smurthwaite wrote:
>This patch fixes a bug in which only enough memory for one cpumap is 
>allocated
>for virDomainGetVcpus instead of one per virtual CPU. This Fixes an 
>overflow.
>Additionally, it uses the return value of virDomainGetVcpus to determine how
>many cpuinfo structs were actually populated rather than assuming they 
>all are.
>Finally, it uses the logical CPU number from the cpuinfo struct in the 
>retrurn
>data instead of assuming CPU numbers are sequential. This should 
>handle cases
>where arbitrary CPUs are offline.
>
>Signed-off-by: Charlie Smurthwaite <charlie@atechmedia.com>
>---
>ext/libvirt/domain.c | 7 ++++---
>1 file changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Ján Tomko <jtomko@redhat.com>

and pushed

Jano