Re: [PATCH v2 7/8] tools: Introduce the "xl dt-overlay {attach,detach}" commands

[Jason Andryuk <jason.andryuk@amd.com>]

On 2024-05-16 06:03, Henry Wang wrote:
> With the XEN_DOMCTL_dt_overlay DOMCTL added, users should be able to
> attach/detach devices from the provided DT overlay to domains.
> Support this by introducing a new set of "xl dt-overlay" commands and
> related documentation, i.e. "xl dt-overlay {attach,detach}". Slightly
> rework the command option parsing logic.
> 
> Since the addition of these two commands modifies the existing libxl
> API libxl_dt_overlay(), also provide the backward compatible for it.
> 
> Signed-off-by: Henry Wang <xin.wang2@amd.com>
> ---
> v2:
> - New patch.

Mostly looks good.  One small thing below.

> diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
> index 02575d5d36..53d1fa3655 100644
> --- a/tools/xl/xl_vmcontrol.c
> +++ b/tools/xl/xl_vmcontrol.c
> @@ -1268,32 +1268,43 @@ int main_create(int argc, char **argv)
>   #ifdef LIBXL_HAVE_DT_OVERLAY
>   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;
> +    uint32_t domain_id = 0;
>   
>       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;
> +    if (strcmp(argv[optind], "add") == 0)
> +        op = LIBXL_DT_OVERLAY_ADD;
> +    else if (strcmp(argv[optind], "remove") == 0)
> +        op = LIBXL_DT_OVERLAY_REMOVE;
> +    else if (strcmp(argv[optind], "attach") == 0)
> +        op = LIBXL_DT_OVERLAY_ATTACH;
> +    else if (strcmp(argv[optind], "detach") == 0)
> +        op = LIBXL_DT_OVERLAY_DETACH;
>       else {
>           fprintf(stderr, "Invalid dt overlay operation\n");
>           return EXIT_FAILURE;
>       }
>   
> +    overlay_config_file = argv[optind+1];
> +
> +    if (op == LIBXL_DT_OVERLAY_ATTACH || op == LIBXL_DT_OVERLAY_DETACH) {
> +        if (argc <= optind + 2) {
> +            fprintf(stderr, "Missing domain ID\n");
> +            help("dt-overlay");
> +            return EXIT_FAILURE;
> +        } else {
> +            domain_id = strtol(argv[optind+2], NULL, 10);

domain_id = find_domain(argv[optind+2]);

And you'll get name resolution, too.

Thanks,
Jason