Re: [PATCH 10/11] tools/xl: add xl commands for xenstore quota operations

["Anthony PERARD" <anthony.perard@xxxxxxxxxx>]

On Thu, Mar 05, 2026 at 02:52:07PM +0100, Juergen Gross wrote:
> Add "xl xenstore-quota-get" and "xl xenstore-quota-set" commands for
> retrieving and setting global and per-domain Xenstore quota.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>  tools/xl/Makefile      |  1 +
>  tools/xl/xl.h          |  2 +
>  tools/xl/xl_cmdtable.c | 10 +++++
>  tools/xl/xl_parse.c    | 25 ++++++++++++
>  tools/xl/xl_parse.h    |  1 +
>  tools/xl/xl_xsquota.c  | 88 ++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 127 insertions(+)
>  create mode 100644 tools/xl/xl_xsquota.c
> 
> diff --git a/tools/xl/Makefile b/tools/xl/Makefile
> index 973ff0e1a2..e4eed8be13 100644
> --- a/tools/xl/Makefile
> +++ b/tools/xl/Makefile
> @@ -24,6 +24,7 @@ XL_OBJS += xl_sched.o xl_pci.o xl_vcpu.o xl_cdrom.o xl_mem.o
>  XL_OBJS += xl_info.o xl_console.o xl_misc.o
>  XL_OBJS += xl_vmcontrol.o xl_saverestore.o xl_migrate.o
>  XL_OBJS += xl_vdispl.o xl_vsnd.o xl_vkb.o
> +XL_OBJS += xl_xsquota.o
>  
>  $(XL_OBJS): CFLAGS += $(CFLAGS_libxentoollog)
>  $(XL_OBJS): CFLAGS += $(CFLAGS_XL)
> diff --git a/tools/xl/xl.h b/tools/xl/xl.h
> index 9000df00de..0efc07a6ba 100644
> --- a/tools/xl/xl.h
> +++ b/tools/xl/xl.h
> @@ -217,6 +217,8 @@ int main_psr_mba_set(int argc, char **argv);
>  int main_psr_mba_show(int argc, char **argv);
>  #endif
>  int main_qemu_monitor_command(int argc, char **argv);
> +int main_xsquota_get(int argc, char **argv);
> +int main_xsquota_set(int argc, char **argv);
>  
>  void help(const char *command);
>  
> diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
> index 06a0039718..3de12b12ae 100644
> --- a/tools/xl/xl_cmdtable.c
> +++ b/tools/xl/xl_cmdtable.c
> @@ -649,6 +649,16 @@ const struct cmd_spec cmd_table[] = {
>        "-h print this help\n"
>      },
>  #endif
> +    { "xenstore-quota-get",
> +      &main_xsquota_get, 0, 0,
> +      "List global or domain specific Xenstore quota data",

Maybe saying "quota values" instead of "quota data" would be slightly
better. Or maybe even "quotas" would be enough.

> +      "<Domain>|-g",
> +    },
> +    { "xenstore-quota-set",
> +      &main_xsquota_set, 0, 1,
> +      "Set global or domain specific Xenstore quota data",
> +      "<Domain>|-g <quota>=<val>...",
> +    },
>  };
>  
>  const int cmdtable_len = ARRAY_SIZE(cmd_table);
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 1a2ea8b5d5..934ad4eeef 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -1314,6 +1314,31 @@ out:
>      return ret;
>  }
>  
> +int parse_xsquota_item(const char *buf, struct libxl_xs_quota_item *item)
> +{
> +    const char *eq;
> +    char *endptr;
> +
> +    eq = strchr(buf, '=');
> +    if (!eq) {
> +        fprintf(stderr, "Quota specification \"%s\" lacks \"=\".\n", buf);
> +        return ERROR_INVAL;
> +    }
> +    errno = 0;
> +    item->name = strndup(buf, eq - buf);
> +    if (!item->name)
> +        return ERROR_NOMEM;
> +    item->val = strtoul(eq + 1, &endptr, 0);
> +    if (errno || !eq[1] || *endptr) {

I think we also need to check that the value returned by strtoul() can
actually be stored in `item->val`. It would be misleading to accept a
quota value and store a different one.

> +        fprintf(stderr,
> +                "Quota specification \"%s\" uses illegal value \"%s\".\n",
> +                buf, eq);
> +        return ERROR_INVAL;
> +    }
> +
> +    return 0;
> +}
> +
>  void parse_config_data(const char *config_source,
>                         const char *config_data,
>                         int config_len,
> diff --git a/tools/xl/xl_parse.h b/tools/xl/xl_parse.h
> index fe0d586cdd..57bb43a067 100644
> --- a/tools/xl/xl_parse.h
> +++ b/tools/xl/xl_parse.h
> @@ -36,6 +36,7 @@ int parse_nic_config(libxl_device_nic *nic, XLU_Config 
> **config, char *token);
>  int parse_vdispl_config(libxl_device_vdispl *vdispl, char *token);
>  int parse_vsnd_item(libxl_device_vsnd *vsnd, const char *spec);
>  int parse_vkb_config(libxl_device_vkb *vkb, char *token);
> +int parse_xsquota_item(const char *buf, struct libxl_xs_quota_item *item);
>  
>  int match_option_size(const char *prefix, size_t len,
>                        char *arg, char **argopt);
> diff --git a/tools/xl/xl_xsquota.c b/tools/xl/xl_xsquota.c
> new file mode 100644
> index 0000000000..eaf19feac8
> --- /dev/null
> +++ b/tools/xl/xl_xsquota.c
> @@ -0,0 +1,88 @@
> +/* SPDX-License-Identifier: LGPL-2.1-only */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <libxl.h>
> +#include <libxlutil.h>
> +
> +#include "xl.h"
> +#include "xl_utils.h"
> +#include "xl_parse.h"
> +
> +int main_xsquota_get(int argc, char **argv)
> +{
> +    libxl_xs_quota_set q;
> +    unsigned int i;
> +    int rc;

You should call libxl_xs_quota_set_init(&q). (That would avoid a segv
later, when _disposed() is called.)

> +
> +    if (argc != 2) {
> +        fprintf(stderr, "Domain or \"-g\" must be specified.\n");
> +        return EXIT_FAILURE;
> +    }
> +
> +    if (!strcmp(argv[1], "-g")) {
> +        rc = libxl_xsquota_global_get(ctx, &q);
> +    } else {
> +        uint32_t domid = find_domain(argv[1]);
> +
> +        rc = libxl_xsquota_domain_get(ctx, domid, &q);
> +    }
> +
> +    if (rc) {
> +        fprintf(stderr, "Quota could not be obtained.\n");
> +        return EXIT_FAILURE;
> +    }
> +
> +    printf("Quota name           Quota value\n");
> +    printf("--------------------------------\n");
> +    for (i = 0; i < q.num_quota; i++)
> +        printf("%-20s %8u\n", q.quota[i].name, q.quota[i].val);
> +
> +    libxl_xs_quota_set_dispose(&q);
> +
> +    return EXIT_SUCCESS;
> +}
> +
> +int main_xsquota_set(int argc, char **argv)
> +{
> +    unsigned int i;
> +    libxl_xs_quota_set q;
> +    int rc = EXIT_FAILURE;
> +
> +    if (argc < 3) {
> +        fprintf(stderr, "Not enough parameters.\n");

I think you can call help("xenstore-quota-set") to provide the needed
info about what the parameters are.

> +        return EXIT_FAILURE;
> +    }
> +
> +    q.num_quota = argc - 2;
> +    q.quota = calloc(q.num_quota, sizeof(*q.quota));
> +    if (!q.quota) {
> +        fprintf(stderr, "Memory allocation failure!\n");
> +        goto err;
> +    }

There's `xcalloc() that can be use instead. It does check for memory
allocation failure.

(And that would avoid a segv in libxl_xs_quota_set_dispose() as
num_quota is set before quota is allocated.)


Thanks,


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech