1. Patchew
  2. Libvirt
  3. [PATCH 0/4] Miscellaneous fixes, somehow Windows related
  4. View patch

[PATCH 1/4] util: Parse RSS into ullp

Martin Kletzander posted 4 patches 2 years, 8 months ago
[PATCH 1/4] util: Parse RSS into ullp
Posted by Martin Kletzander 2 years, 8 months ago 
It is used to fill an unsigned long long anyway and if it is negative
than there is really an issue somewhere.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/qemu/qemu_driver.c |  2 +-
 src/util/virprocess.c  | 17 ++++++++++-------
 src/util/virprocess.h  |  2 +-
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 56f4cd619715..857fbfb7992c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9937,7 +9937,7 @@ qemuDomainMemoryStatsInternal(virDomainObj *vm,
 
 {
     int ret = -1;
-    long rss;
+    unsigned long long rss;
 
     if (virDomainObjCheckActive(vm) < 0)
         return -1;
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index d12fe4f7171f..a9f249413ce1 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -23,6 +23,7 @@
 #include <config.h>
 
 #include <fcntl.h>
+#include <limits.h>
 #include <signal.h>
 #ifndef WIN32
 # include <sys/wait.h>
@@ -1739,7 +1740,7 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
                       unsigned long long *userTime,
                       unsigned long long *sysTime,
                       int *lastCpu,
-                      long *vm_rss,
+                      unsigned long long *vm_rss,
                       pid_t pid,
                       pid_t tid)
 {
@@ -1748,14 +1749,16 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
     unsigned long long stime = 0;
     const unsigned long long jiff2nsec = 1000ull * 1000ull * 1000ull /
                                          (unsigned long long) sysconf(_SC_CLK_TCK);
-    long rss = 0;
+    long pagesize = virGetSystemPageSizeKB();
+    unsigned long long rss = 0;
     int cpu = 0;
 
     if (!proc_stat ||
         virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_UTIME], NULL, 10, &utime) < 0 ||
         virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, &stime) < 0 ||
-        virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, &rss) < 0 ||
-        virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, &cpu) < 0) {
+        virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, &rss) < 0 ||
+        virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, &cpu) < 0 ||
+        rss > ULLONG_MAX / pagesize) {
         VIR_WARN("cannot parse process status data");
     }
 
@@ -1771,10 +1774,10 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
         *lastCpu = cpu;
 
     if (vm_rss)
-        *vm_rss = rss * virGetSystemPageSizeKB();
+        *vm_rss = rss * pagesize;
 
 
-    VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%ld",
+    VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%lld",
               (int) pid, tid, utime, stime, cpu, rss);
 
     return 0;
@@ -1851,7 +1854,7 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
                       unsigned long long *userTime,
                       unsigned long long *sysTime,
                       int *lastCpu,
-                      long *vm_rss,
+                      unsigned long long *vm_rss,
                       pid_t pid G_GNUC_UNUSED,
                       pid_t tid G_GNUC_UNUSED)
 {
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 4e216788389f..c18f87e80ec9 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -198,7 +198,7 @@ int virProcessGetStatInfo(unsigned long long *cpuTime,
                           unsigned long long *userTime,
                           unsigned long long *sysTime,
                           int *lastCpu,
-                          long *vm_rss,
+                          unsigned long long *vm_rss,
                           pid_t pid,
                           pid_t tid);
 int virProcessGetSchedInfo(unsigned long long *cpuWait,
-- 
2.41.0
Re: [PATCH 1/4] util: Parse RSS into ullp
Posted by Michal Prívozník 2 years, 8 months ago 
On 6/12/23 09:55, Martin Kletzander wrote:
> It is used to fill an unsigned long long anyway and if it is negative
> than there is really an issue somewhere.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/qemu/qemu_driver.c |  2 +-
>  src/util/virprocess.c  | 17 ++++++++++-------
>  src/util/virprocess.h  |  2 +-
>  3 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 56f4cd619715..857fbfb7992c 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -9937,7 +9937,7 @@ qemuDomainMemoryStatsInternal(virDomainObj *vm,
>  
>  {
>      int ret = -1;
> -    long rss;
> +    unsigned long long rss;
>  
>      if (virDomainObjCheckActive(vm) < 0)
>          return -1;
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index d12fe4f7171f..a9f249413ce1 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -23,6 +23,7 @@
>  #include <config.h>
>  
>  #include <fcntl.h>
> +#include <limits.h>
>  #include <signal.h>
>  #ifndef WIN32
>  # include <sys/wait.h>
> @@ -1739,7 +1740,7 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
>                        unsigned long long *userTime,
>                        unsigned long long *sysTime,
>                        int *lastCpu,
> -                      long *vm_rss,
> +                      unsigned long long *vm_rss,
>                        pid_t pid,
>                        pid_t tid)
>  {
> @@ -1748,14 +1749,16 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
>      unsigned long long stime = 0;
>      const unsigned long long jiff2nsec = 1000ull * 1000ull * 1000ull /
>                                           (unsigned long long) sysconf(_SC_CLK_TCK);
> -    long rss = 0;
> +    long pagesize = virGetSystemPageSizeKB();

const please. We don't really want anybody change this value.

> +    unsigned long long rss = 0;
>      int cpu = 0;
>  
>      if (!proc_stat ||
>          virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_UTIME], NULL, 10, &utime) < 0 ||
>          virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, &stime) < 0 ||
> -        virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, &rss) < 0 ||
> -        virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, &cpu) < 0) {
> +        virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, &rss) < 0 ||
> +        virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, &cpu) < 0 ||
> +        rss > ULLONG_MAX / pagesize) {
>          VIR_WARN("cannot parse process status data");
>      }
>  

Michal
Re: [PATCH 1/4] util: Parse RSS into ullp
Posted by Martin Kletzander 2 years, 7 months ago 
On Mon, Jun 12, 2023 at 10:25:26AM +0200, Michal Prívozník wrote:
>On 6/12/23 09:55, Martin Kletzander wrote:
>> It is used to fill an unsigned long long anyway and if it is negative
>> than there is really an issue somewhere.
>>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>>  src/qemu/qemu_driver.c |  2 +-
>>  src/util/virprocess.c  | 17 ++++++++++-------
>>  src/util/virprocess.h  |  2 +-
>>  3 files changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 56f4cd619715..857fbfb7992c 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -9937,7 +9937,7 @@ qemuDomainMemoryStatsInternal(virDomainObj *vm,
>>
>>  {
>>      int ret = -1;
>> -    long rss;
>> +    unsigned long long rss;
>>
>>      if (virDomainObjCheckActive(vm) < 0)
>>          return -1;
>> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
>> index d12fe4f7171f..a9f249413ce1 100644
>> --- a/src/util/virprocess.c
>> +++ b/src/util/virprocess.c
>> @@ -23,6 +23,7 @@
>>  #include <config.h>
>>
>>  #include <fcntl.h>
>> +#include <limits.h>
>>  #include <signal.h>
>>  #ifndef WIN32
>>  # include <sys/wait.h>
>> @@ -1739,7 +1740,7 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
>>                        unsigned long long *userTime,
>>                        unsigned long long *sysTime,
>>                        int *lastCpu,
>> -                      long *vm_rss,
>> +                      unsigned long long *vm_rss,
>>                        pid_t pid,
>>                        pid_t tid)
>>  {
>> @@ -1748,14 +1749,16 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
>>      unsigned long long stime = 0;
>>      const unsigned long long jiff2nsec = 1000ull * 1000ull * 1000ull /
>>                                           (unsigned long long) sysconf(_SC_CLK_TCK);
>> -    long rss = 0;
>> +    long pagesize = virGetSystemPageSizeKB();
>
>const please. We don't really want anybody change this value.
>

Of course, if someone changed the page size the machine would clearly
crash =D  But in all seriousness, I'll change that, but don't really see
the point, especially in C.

>> +    unsigned long long rss = 0;
>>      int cpu = 0;
>>
>>      if (!proc_stat ||
>>          virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_UTIME], NULL, 10, &utime) < 0 ||
>>          virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, &stime) < 0 ||
>> -        virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, &rss) < 0 ||
>> -        virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, &cpu) < 0) {
>> +        virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, &rss) < 0 ||
>> +        virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, &cpu) < 0 ||
>> +        rss > ULLONG_MAX / pagesize) {
>>          VIR_WARN("cannot parse process status data");
>>      }
>>
>
>Michal
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.