1. Patchew
  2. Xen
  3. View series

[PATCH] tools/xl: Guard main_dt_overlay() with LIBXL_HAVE_DT_OVERLAY

Michal Orzel posted 1 patch 7 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230906083614.25315-1-michal.orzel@amd.com
tools/xl/xl.h           | 2 ++
tools/xl/xl_cmdtable.c  | 2 ++
tools/xl/xl_vmcontrol.c | 3 +++
3 files changed, 7 insertions(+)
[PATCH] tools/xl: Guard main_dt_overlay() with LIBXL_HAVE_DT_OVERLAY
Posted by Michal Orzel 7 months, 3 weeks ago 
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(+)

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
-- 
2.25.1
Re: [PATCH] tools/xl: Guard main_dt_overlay() with LIBXL_HAVE_DT_OVERLAY
Posted by Anthony PERARD 7 months, 3 weeks ago 
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,

-- 
Anthony PERARD
Re: [PATCH] tools/xl: Guard main_dt_overlay() with LIBXL_HAVE_DT_OVERLAY
Posted by Jan Beulich 7 months, 3 weeks ago 
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
Re: [PATCH] tools/xl: Guard main_dt_overlay() with LIBXL_HAVE_DT_OVERLAY
Posted by Michal Orzel 7 months, 3 weeks ago 

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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.