Re: [PATCH v7 5/6] tools: Allow building xen-hptool without CONFIG_MIGRATE

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, Apr 16, 2026 at 08:22:32AM +0000, Mykyta Poturai wrote:
> On 4/16/26 09:49, Jan Beulich wrote:
> > On 15.04.2026 16:51, Mykyta Poturai wrote:
> >> On 3/30/26 15:32, Jan Beulich wrote:
> >>> This looks wrong to me. There are x86-specifics in that file, which 
> >>> shouldn't
> >>> be built on Arm. And the name of the file also doesn't indicate any 
> >>> relation
> >>> to CPU management.
> >>
> >> xen-hptool requires xg_offline_page as it has both CPU and memory
> >> hotplug commands. Without building xg_offline_page it fails with
> >>
> >> xen-hptool: symbol lookup error: xen-hptool: undefined symbol:
> >> xc_mark_page_offline, version libxenguest_4.22.0
> >>
> >> when trying to do memory ops.
> >>
> >> Is it an acceptable behavior?
> > 
> > I don't think so, no. The tool wouldn't, aiui, load at all then if built 
> > with
> > "bindnow" enabled.
> > 
> >> If so I guess we can build xg_offline page only on x86.
> > 
> > We still need to, imo. But the tool still needs to be usable no matter how
> > specifically it is built. It should avoid referencing xg_offline_page.c
> > functions when built for non-x86.
> 
> As I understand, the usage of arch-specific compile time checks is 
> heavily discouraged in tools. So I don’t think it would be approved by 
> tools maintainers. Do we really need to omit this file if memory ops are 
> already getting blocked by Xen on Arm anyway?

So you are trying to modify a library and introduced untested
functionality just to be able to build a different tool? I don't think
that a good idea especially in this case where it's more than just glue
code between a binary and xen.

We could change the library to provide the missing symbols, but it is
probably best to keep it that way for now.

So, how about changing `xen-hptool` to have reduced functionality on
other platform, and keep the 'mem-*' command on x86 only? You could move
the function that implement the 'mem-*' command into a separate file,
that compile only on x86 (or more specifically when CONFIG_MIGRATE is
set) and just have a "#if defined(__i386__) || defined(__x86_64__)" in
the `main_options` array.

They are compile-time arch-specific check everywhere in tools. Arch
specific are often implemented in separated source file, this mean we
can limit the #ifdefs to a minimum and keep the code readable.

Thanks,


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech