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(a)swemel.ru&gt; --- 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;

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).

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(a)swemel.ru&gt; >> --- >>   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 >