On Tue, Dec 06, 2022 at 10:18:15PM -0800, Vikram Garhwal wrote:
> diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
> index 35182ca196..868364c58d 100644
> --- a/tools/xl/xl_cmdtable.c
> +++ b/tools/xl/xl_cmdtable.c
> @@ -630,6 +630,12 @@ const struct cmd_spec cmd_table[] = {
> "Issue a qemu monitor command to the device model of a domain",
> "<Domain> <Command>",
> },
> + { "dt_overlay",
Command with multiple words are using '-' instead of '_', could you
rename the command to "dt-overlay"?
> + &main_dt_overlay, 0, 1,
> + "Add/Remove a device tree overlay",
> + "add/remove <.dtbo>"
> + "-h print this help\n"
> + },
> };
>
> const int cmdtable_len = ARRAY_SIZE(cmd_table);
> diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
> index 5518c78dc6..b5f04e2741 100644
> --- a/tools/xl/xl_vmcontrol.c
> +++ b/tools/xl/xl_vmcontrol.c
> @@ -1265,6 +1265,54 @@ int main_create(int argc, char **argv)
> return 0;
> }
>
> +int main_dt_overlay(int argc, char **argv)
> +{
> + const char *overlay_ops = NULL;
> + const char *overlay_config_file = NULL;
> + void *overlay_dtb = NULL;
> + int rc;
> + uint8_t op;
> + int overlay_dtb_size = 0;
> + const int overlay_add_op = 1;
> + const int overlay_remove_op = 2;
> +
> + if (argc < 2) {
> + help("dt_overlay");
> + return EXIT_FAILURE;
> + }
> +
> + overlay_ops = argv[1];
> + overlay_config_file = argv[2];
> +
> + if (strcmp(overlay_ops, "add") == 0)
> + op = overlay_add_op;
> + else if (strcmp(overlay_ops, "remove") == 0)
> + op = overlay_remove_op;
> + else {
> + fprintf(stderr, "Invalid dt overlay operation\n");
> + return ERROR_FAIL;
ERROR_FAIL isn't really a value to be used when exiting the programme,
it's value is -3. It's from libxl API.
Instead, could you return EXIT_FAILURE?
> + }
> +
> + if (overlay_config_file) {
> + rc = libxl_read_file_contents(ctx, overlay_config_file,
> + &overlay_dtb, &overlay_dtb_size);
> +
> + if (rc) {
> + fprintf(stderr, "failed to read the overlay device tree file
> %s\n",
> + overlay_config_file);
> + free(overlay_dtb);
> + return ERROR_FAIL;
> + }
> + } else {
> + fprintf(stderr, "overlay dtbo file not provided\n");
> + return ERROR_FAIL;
> + }
> +
> + rc = libxl_dt_overlay(ctx, overlay_dtb, overlay_dtb_size, op);
Value returned by libxl_*() are going to be negative when there's an
error, so not something to be return by main(). Could you check if
there's an error and return EXIT_FAILURE instead?
> + free(overlay_dtb);
> + return rc;
> +}
Thanks,
--
Anthony PERARD
