Re: [PATCH] fix uint64 underflow

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/18/23 02:22, Дмитрий Фролов wrote:
Hi, Michal

Of course, this changes the semantics. To my opinion, it is obvious from
the source code, that we need some additional memory. I really doubt, that
we intended to allocate some Exabytes additionally (using dirty underflow
hack by the way).

libxlSetMemoryTargetWrapper would have at least prevented allocating > LLONG_MAX :-).

I took a stab at fixing it, along with general brokeness of the dom0 autoballoon logic

https://listman.redhat.com/archives/libvir-list/2023-September/242160.html

It's compile-tested only ATM. I'll check the functionality later today after repurposing a test system.

Regards,
Jim


Dmitry

18.09.2023 10:59, Michal Prívozník пишет:
On 9/12/23 16:50, Dmitry Frolov wrote:
According to previous statement,
'free_mem' is less than 'needed_mem'.
So, the subtraction 'free_mem - needed_mem' is negative,
and will raise uint64 underflow.

Signed-off-by: Dmitry Frolov <frolov@xxxxxxxxx>
---
  src/libxl/libxl_domain.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 6c167df63e..36be042971 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -940,7 +940,7 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config *d_config)
          if (free_mem >= needed_mem)
              return 0;
-        target_mem = free_mem - needed_mem;
+        target_mem = needed_mem - free_mem;
          if (libxlSetMemoryTargetWrapper(ctx, 0, target_mem,
                                          /* relative */ 1, 0) < 0)
              goto error;
Yeah, this fixes the underflow, but I worry about the while semantics a
bit. What the code effectively does:

   libxl_domain_need_memory(&needed_mem);

   do {
     libxl_get_free_memory(&free_mem);

     if (free_mem >= needed_mem)
       return 0;

     target_mem = needed_mem - free_mem;
     libxl_set_memory_target(target_mem);
   } while(...)

Now, if libxl_domain_need_memory() returns how much memory a domain
really needs, then why undergo trouble of getting its free memory? Why
not pass it to set_memory_target() right away?

Or am I misunderstanding something?

Michal







[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux