| Message ID | 20221207061815.7404-10-vikram.garhwal@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dynamic node programming using overlay dtbo | expand |
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,
Hi Anthony, On 12/9/22 9:55 AM, Anthony PERARD wrote: > 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"? understood. > >> + &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? Will address this in next version. >> + } >> + >> + 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? Okay. >> + free(overlay_dtb); >> + return rc; >> +} > Thanks, >
diff --git a/tools/xl/xl.h b/tools/xl/xl.h index 7c9aff6ad7..6e95dfe6ea 100644 --- a/tools/xl/xl.h +++ b/tools/xl/xl.h @@ -138,6 +138,7 @@ 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); +int main_dt_overlay(int argc, char **argv); 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 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", + &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; + } + + 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); + + free(overlay_dtb); + return rc; +} /* * Local variables: * mode: C
Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com> --- tools/xl/xl.h | 1 + tools/xl/xl_cmdtable.c | 6 ++++++ tools/xl/xl_vmcontrol.c | 48 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+)