Without this, Dom0 will have have a curiously off-by-1 target_memkb
value displayed by "xl list -l".

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
In retrieve_domain_configuration_end(), isn't it the wrong way round to
set both ->b_info.target_memkb and ->b_info.max_memkb only after calling
libxl__get_targetmem_fudge(), when that uses the two fields? This way we
could as well use ->b_info->video_memkb directly there.

Of course this may point at a bigger problem, as other fields may
similarly never be set for Dom0.

--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -4685,7 +4685,10 @@ uint64_t libxl__get_targetmem_fudge(libx
                                 info->max_memkb > info->target_memkb)
                                 ? LIBXL_MAXMEM_CONSTANT : 0;
 
-    return info->video_memkb + mem_target_fudge;
+    if (info->video_memkb != LIBXL_MEMKB_DEFAULT)
+        mem_target_fudge += info->video_memkb;
+
+    return mem_target_fudge;
 }
 
 int libxl__get_memory_target(libxl__gc *gc, uint32_t domid,

On 2025-08-27 01:56, Jan Beulich wrote:
> Without this, Dom0 will have have a curiously off-by-1 target_memkb
> value displayed by "xl list -l".
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

In theory, b_info->video_memkb should have been defaulted to 0.  In 
practice you found it isn't.  xen-init-dom0 could probably use some fixing.

> ---
> In retrieve_domain_configuration_end(), isn't it the wrong way round to
> set both ->b_info.target_memkb and ->b_info.max_memkb only after calling
> libxl__get_targetmem_fudge(), when that uses the two fields? This way we
> could as well use ->b_info->video_memkb directly there.

I think it's attempting to read the current values from xenstore, in 
case they changed via xl mem-set, and then putting them into the d_config.

Regards,
Jason

> Of course this may point at a bigger problem, as other fields may
> similarly never be set for Dom0.

On Wed, Aug 27, 2025 at 09:12:50PM -0400, Jason Andryuk wrote:
> On 2025-08-27 01:56, Jan Beulich wrote:
> > Without this, Dom0 will have have a curiously off-by-1 target_memkb
> > value displayed by "xl list -l".
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Acked-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

-- 
Anthony PERARD

On 28.08.2025 03:12, Jason Andryuk wrote:
> On 2025-08-27 01:56, Jan Beulich wrote:
>> Without this, Dom0 will have have a curiously off-by-1 target_memkb
>> value displayed by "xl list -l".
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Thanks.

> In theory, b_info->video_memkb should have been defaulted to 0.

I don't think 0 would be a legitimate default; it might be for Dom0, but
not generally.

>  In 
> practice you found it isn't.  xen-init-dom0 could probably use some fixing.

Right, kind of as per the latter of the two post-commit-message remarks.

>> ---
>> In retrieve_domain_configuration_end(), isn't it the wrong way round to
>> set both ->b_info.target_memkb and ->b_info.max_memkb only after calling
>> libxl__get_targetmem_fudge(), when that uses the two fields? This way we
>> could as well use ->b_info->video_memkb directly there.
> 
> I think it's attempting to read the current values from xenstore, in 
> case they changed via xl mem-set, and then putting them into the d_config.

That's my understanding too, yet it still makes little sense to me to
invoke a function when data it consumes isn't set (yet).

Jan

>> Of course this may point at a bigger problem, as other fields may
>> similarly never be set for Dom0.