| Message ID | 20230906083614.25315-1-michal.orzel@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools/xl: Guard main_dt_overlay() with LIBXL_HAVE_DT_OVERLAY | expand |
On 06.09.2023 10:36, Michal Orzel wrote: > main_dt_overlay() makes a call to libxl_dt_overlay() which is for now > only compiled for Arm. This causes the build failure as reported by > gitlab CI and OSSTEST. Fix it by guarding the function, prototype and > entry in cmd_table[] using LIBXL_HAVE_DT_OVERLAY. This has an advantage > over regular Arm guard so that the code will not need to be modified again > if other architecture gain support for this feature. > > Fixes: 61765a07e3d8 ("tools/xl: Add new xl command overlay for device tree overlay support") > Reported-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Michal Orzel <michal.orzel@amd.com> > --- > There are still other parts of dt overlay support in toolstack that would > want to be revisited in order to use guards suitable to be used by other > arches. Since from all I can tell this will do Reviewed-by: Jan Beulich <jbeulich@suse.com> But I still wonder: We agreed to leave libxc alone for now, but was it really intentional that you didn't adjust libxl.h right here, but instead ... > --- a/tools/xl/xl.h > +++ b/tools/xl/xl.h > @@ -138,7 +138,9 @@ int main_shutdown(int argc, char **argv); > int main_reboot(int argc, char **argv); > int main_list(int argc, char **argv); > int main_vm_list(int argc, char **argv); > +#ifdef LIBXL_HAVE_DT_OVERLAY > int main_dt_overlay(int argc, char **argv); > +#endif > int main_create(int argc, char **argv); > int main_config_update(int argc, char **argv); > int main_button_press(int argc, char **argv); ... made this adjustment, which imo isn't strictly necessary. Jan
On 06/09/2023 10:42, Jan Beulich wrote: > > > On 06.09.2023 10:36, Michal Orzel wrote: >> main_dt_overlay() makes a call to libxl_dt_overlay() which is for now >> only compiled for Arm. This causes the build failure as reported by >> gitlab CI and OSSTEST. Fix it by guarding the function, prototype and >> entry in cmd_table[] using LIBXL_HAVE_DT_OVERLAY. This has an advantage >> over regular Arm guard so that the code will not need to be modified again >> if other architecture gain support for this feature. >> >> Fixes: 61765a07e3d8 ("tools/xl: Add new xl command overlay for device tree overlay support") >> Reported-by: Jan Beulich <jbeulich@suse.com> >> Signed-off-by: Michal Orzel <michal.orzel@amd.com> >> --- >> There are still other parts of dt overlay support in toolstack that would >> want to be revisited in order to use guards suitable to be used by other >> arches. > > Since from all I can tell this will do > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > But I still wonder: We agreed to leave libxc alone for now, but was it > really intentional that you didn't adjust libxl.h right here, but instead > ... > >> --- a/tools/xl/xl.h >> +++ b/tools/xl/xl.h >> @@ -138,7 +138,9 @@ int main_shutdown(int argc, char **argv); >> int main_reboot(int argc, char **argv); >> int main_list(int argc, char **argv); >> int main_vm_list(int argc, char **argv); >> +#ifdef LIBXL_HAVE_DT_OVERLAY >> int main_dt_overlay(int argc, char **argv); >> +#endif >> int main_create(int argc, char **argv); >> int main_config_update(int argc, char **argv); >> int main_button_press(int argc, char **argv); > > ... made this adjustment, which imo isn't strictly necessary. I'm not a toolstack expert but I decided to guard the prototype as well given that quite a few of them are guarded either using LIBXL_HAVE or arch guard. ~Michal
On Wed, Sep 06, 2023 at 10:36:14AM +0200, Michal Orzel wrote: > main_dt_overlay() makes a call to libxl_dt_overlay() which is for now > only compiled for Arm. This causes the build failure as reported by > gitlab CI and OSSTEST. Fix it by guarding the function, prototype and > entry in cmd_table[] using LIBXL_HAVE_DT_OVERLAY. This has an advantage > over regular Arm guard so that the code will not need to be modified again > if other architecture gain support for this feature. > > Fixes: 61765a07e3d8 ("tools/xl: Add new xl command overlay for device tree overlay support") > Reported-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> Thanks,
diff --git a/tools/xl/xl.h b/tools/xl/xl.h index a923daccd340..3045b5a8e3f0 100644 --- a/tools/xl/xl.h +++ b/tools/xl/xl.h @@ -138,7 +138,9 @@ int main_shutdown(int argc, char **argv); int main_reboot(int argc, char **argv); int main_list(int argc, char **argv); int main_vm_list(int argc, char **argv); +#ifdef LIBXL_HAVE_DT_OVERLAY int main_dt_overlay(int argc, char **argv); +#endif int main_create(int argc, char **argv); int main_config_update(int argc, char **argv); int main_button_press(int argc, char **argv); diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c index 2463521156a1..62bdb2aeaa35 100644 --- a/tools/xl/xl_cmdtable.c +++ b/tools/xl/xl_cmdtable.c @@ -631,12 +631,14 @@ const struct cmd_spec cmd_table[] = { "Issue a qemu monitor command to the device model of a domain", "<Domain> <Command>", }, +#ifdef LIBXL_HAVE_DT_OVERLAY { "dt-overlay", &main_dt_overlay, 0, 1, "Add/Remove a device tree overlay", "add/remove <.dtbo>" "-h print this help\n" }, +#endif }; const int cmdtable_len = ARRAY_SIZE(cmd_table); diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c index cea5b4a88e81..98f6bd2e7607 100644 --- a/tools/xl/xl_vmcontrol.c +++ b/tools/xl/xl_vmcontrol.c @@ -1265,6 +1265,7 @@ int main_create(int argc, char **argv) return 0; } +#ifdef LIBXL_HAVE_DT_OVERLAY int main_dt_overlay(int argc, char **argv) { const char *overlay_ops = NULL; @@ -1317,6 +1318,8 @@ int main_dt_overlay(int argc, char **argv) return rc; } +#endif + /* * Local variables: * mode: C
main_dt_overlay() makes a call to libxl_dt_overlay() which is for now only compiled for Arm. This causes the build failure as reported by gitlab CI and OSSTEST. Fix it by guarding the function, prototype and entry in cmd_table[] using LIBXL_HAVE_DT_OVERLAY. This has an advantage over regular Arm guard so that the code will not need to be modified again if other architecture gain support for this feature. Fixes: 61765a07e3d8 ("tools/xl: Add new xl command overlay for device tree overlay support") Reported-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Michal Orzel <michal.orzel@amd.com> --- There are still other parts of dt overlay support in toolstack that would want to be revisited in order to use guards suitable to be used by other arches. --- tools/xl/xl.h | 2 ++ tools/xl/xl_cmdtable.c | 2 ++ tools/xl/xl_vmcontrol.c | 3 +++ 3 files changed, 7 insertions(+)