On 25.04.2024 02:54, Henry Wang wrote: > On 4/24/2024 2:16 PM, Jan Beulich wrote: >> On 24.04.2024 05:34, Henry Wang wrote: >>> From: Vikram Garhwal <[email protected]> >>> >>> Introduce a shell script that runs in the background and calls >>> get_overlay to retrive overlays and add them (or remove them) to Linux >>> device tree (running as a domU). >>> >>> Signed-off-by: Vikram Garhwal <[email protected]> >>> Signed-off-by: Stefano Stabellini <[email protected]> >>> Signed-off-by: Henry Wang <[email protected]> >>> --- >>> tools/helpers/Makefile | 2 +- >>> tools/helpers/get_overlay.sh | 81 ++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 82 insertions(+), 1 deletion(-) >>> create mode 100755 tools/helpers/get_overlay.sh >> Besides the same naming issue as in the earlier patch, the script also >> looks very Linux-ish. Yet ... > > I will fix the naming issue in v2. Would you mind elaborating a bit more > about the "Linux-ish" concern? I guess this is because the original use > case is on Linux, should I do anything about this?
Well, the script won't work on other than Linux, will it? Therefore ... >>> --- a/tools/helpers/Makefile >>> +++ b/tools/helpers/Makefile >>> @@ -58,7 +58,6 @@ init-dom0less: $(INIT_DOM0LESS_OBJS) >>> get_overlay: $(SHARE_OVERLAY_OBJS) >>> $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenvchan) $(LDLIBS_libxenstore) >>> $(LDLIBS_libxenctrl) $(LDLIBS_libxengnttab) $(APPEND_LDFLAGS) >>> >>> - >>> .PHONY: install >>> install: all >>> $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN) >>> @@ -67,6 +66,7 @@ install: all >>> .PHONY: uninstall >>> uninstall: >>> for i in $(TARGETS); do rm -f $(DESTDIR)$(LIBEXEC_BIN)/$$i; done >>> + $(RM) $(DESTDIR)$(LIBEXEC_BIN)/get_overlay.sh >>> >>> .PHONY: clean >>> clean: >> ... you touching only the uninstall target, it's not even clear to me >> how (and under what conditions) the script is going to make it into >> $(DESTDIR)$(LIBEXEC_BIN)/. Did you mean to add to $(TARGETS), perhaps, >> alongside the earlier added get-overlay binary? ... it first of needs to become clear under what conditions it is actually going to be installed. > You are right, I think the get-overlay binary and this script should be > installed if DTB overlay is supported. Checking the code, I found > LIBXL_HAVE_DT_OVERLAY which can indicate if we have this feature > supported in libxl. Do you think it is a good idea to use it to install > these two files in Makefile? Thanks. Counter question: If it's not going to be installed, how are people going to make use of it? If the script is intended for manual use only, I think that would want saying in the description. Yet then I couldn't see why the uninstall goal would need modifying. As to LIBXL_HAVE_DT_OVERLAY - that's not accessible from a Makefile, I guess? Jan
