[libvirt] [ruby PATCH] Fix default values for node_cpu_stats() and node_memory_stats()

Stefano Garzarella posted 1 patch 4 years, 5 months ago
Failed in applying to current master (apply log)
ext/libvirt/connect.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
[libvirt] [ruby PATCH] Fix default values for node_cpu_stats() and node_memory_stats()
Posted by Stefano Garzarella 4 years, 5 months ago
ruby_libvirt_value_to_int() returns 0 if the optional value is
not defined, but in node_cpu_stats() and node_memory_stats()
the default value of cpuNum and cellNum is -1.

Reported-by: Charlie Smurthwaite <charlie@atech.media>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 ext/libvirt/connect.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/ext/libvirt/connect.c b/ext/libvirt/connect.c
index 5932535..b2d041b 100644
--- a/ext/libvirt/connect.c
+++ b/ext/libvirt/connect.c
@@ -2079,7 +2079,12 @@ static VALUE libvirt_connect_node_cpu_stats(int argc, VALUE *argv, VALUE c)
 
     rb_scan_args(argc, argv, "02", &intparam, &flags);
 
-    tmp = ruby_libvirt_value_to_int(intparam);
+    if (NIL_P(intparam)) {
+        tmp = -1;
+    }
+    else {
+        tmp = ruby_libvirt_value_to_int(intparam);
+    }
 
     return ruby_libvirt_get_parameters(c, ruby_libvirt_value_to_uint(flags),
                                        (void *)&tmp, sizeof(virNodeCPUStats),
@@ -2139,7 +2144,12 @@ static VALUE libvirt_connect_node_memory_stats(int argc, VALUE *argv, VALUE c)
 
     rb_scan_args(argc, argv, "02", &intparam, &flags);
 
-    tmp = ruby_libvirt_value_to_int(intparam);
+    if (NIL_P(intparam)) {
+        tmp = -1;
+    }
+    else {
+        tmp = ruby_libvirt_value_to_int(intparam);
+    }
 
     return ruby_libvirt_get_parameters(c, ruby_libvirt_value_to_uint(flags),
                                        (void *)&tmp, sizeof(virNodeMemoryStats),
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [ruby PATCH] Fix default values for node_cpu_stats() and node_memory_stats()
Posted by Cole Robinson 4 years, 5 months ago
On 10/22/19 11:47 AM, Stefano Garzarella wrote:
> ruby_libvirt_value_to_int() returns 0 if the optional value is
> not defined, but in node_cpu_stats() and node_memory_stats()
> the default value of cpuNum and cellNum is -1.
> 

I know nothing about ruby or the libvirt bindings, but what you describe
makes sense and matches my reading of the code. -1 is
VIR_NODE_CPU_STATS_ALL_CPUS and VIR_NODE_MEMORY_STATS_ALL_CELLS so it
makes sense that would be the default.

Reviewed-by: Cole Robinson <crobinso@redhat.com>

CCing clalance who is the primary ruby-libvirt maintainer, but if he
doesn't get to it by next week then I'll figure out how to build test
it, and push

- Cole

> Reported-by: Charlie Smurthwaite <charlie@atech.media>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  ext/libvirt/connect.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/ext/libvirt/connect.c b/ext/libvirt/connect.c
> index 5932535..b2d041b 100644
> --- a/ext/libvirt/connect.c
> +++ b/ext/libvirt/connect.c
> @@ -2079,7 +2079,12 @@ static VALUE libvirt_connect_node_cpu_stats(int argc, VALUE *argv, VALUE c)
>  
>      rb_scan_args(argc, argv, "02", &intparam, &flags);
>  
> -    tmp = ruby_libvirt_value_to_int(intparam);
> +    if (NIL_P(intparam)) {
> +        tmp = -1;
> +    }
> +    else {
> +        tmp = ruby_libvirt_value_to_int(intparam);
> +    }
>  
>      return ruby_libvirt_get_parameters(c, ruby_libvirt_value_to_uint(flags),
>                                         (void *)&tmp, sizeof(virNodeCPUStats),
> @@ -2139,7 +2144,12 @@ static VALUE libvirt_connect_node_memory_stats(int argc, VALUE *argv, VALUE c)
>  
>      rb_scan_args(argc, argv, "02", &intparam, &flags);
>  
> -    tmp = ruby_libvirt_value_to_int(intparam);
> +    if (NIL_P(intparam)) {
> +        tmp = -1;
> +    }
> +    else {
> +        tmp = ruby_libvirt_value_to_int(intparam);
> +    }
>  
>      return ruby_libvirt_get_parameters(c, ruby_libvirt_value_to_uint(flags),
>                                         (void *)&tmp, sizeof(virNodeMemoryStats),
> 


- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [ruby PATCH] Fix default values for node_cpu_stats() and node_memory_stats()
Posted by Cole Robinson 4 years, 4 months ago
On 11/14/19 4:57 PM, Cole Robinson wrote:
> On 10/22/19 11:47 AM, Stefano Garzarella wrote:
>> ruby_libvirt_value_to_int() returns 0 if the optional value is
>> not defined, but in node_cpu_stats() and node_memory_stats()
>> the default value of cpuNum and cellNum is -1.
>>
> 
> I know nothing about ruby or the libvirt bindings, but what you describe
> makes sense and matches my reading of the code. -1 is
> VIR_NODE_CPU_STATS_ALL_CPUS and VIR_NODE_MEMORY_STATS_ALL_CELLS so it
> makes sense that would be the default.
> 
> Reviewed-by: Cole Robinson <crobinso@redhat.com>
> 
> CCing clalance who is the primary ruby-libvirt maintainer, but if he
> doesn't get to it by next week then I'll figure out how to build test
> it, and push
> 

I've pushed this now

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [ruby PATCH] Fix default values for node_cpu_stats() and node_memory_stats()
Posted by Chris Lalancette 4 years, 4 months ago
On Fri, Nov 22, 2019 at 7:53 PM Cole Robinson <crobinso@redhat.com> wrote:

> > CCing clalance who is the primary ruby-libvirt maintainer, but if he
> > doesn't get to it by next week then I'll figure out how to build test
> > it, and push
> >
>
> I've pushed this now
>

Sorry I missed this.  Thanks for the patch, and thanks to Cole for merging
it.

Chris
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ruby PATCH] Fix default values for node_cpu_stats() and node_memory_stats()
Posted by Stefano Garzarella 4 years, 5 months ago
On Thu, Nov 14, 2019 at 04:57:49PM -0500, Cole Robinson wrote:
> On 10/22/19 11:47 AM, Stefano Garzarella wrote:
> > ruby_libvirt_value_to_int() returns 0 if the optional value is
> > not defined, but in node_cpu_stats() and node_memory_stats()
> > the default value of cpuNum and cellNum is -1.
> > 
> 
> I know nothing about ruby or the libvirt bindings, but what you describe
> makes sense and matches my reading of the code. -1 is
> VIR_NODE_CPU_STATS_ALL_CPUS and VIR_NODE_MEMORY_STATS_ALL_CELLS so it
> makes sense that would be the default.
> 
> Reviewed-by: Cole Robinson <crobinso@redhat.com>

Thank you for reviewing it! Charlie found this issue some weeks ago and
I tried to fix it, but I don't have much experience with ruby.

> 
> CCing clalance who is the primary ruby-libvirt maintainer, but if he
> doesn't get to it by next week then I'll figure out how to build test
> it, and push

Thanks,
Stefano


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list