On 16.03.26 16:08, Anthony PERARD wrote:
On Thu, Mar 05, 2026 at 02:52:04PM +0100, Juergen Gross wrote:diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c index 8a06b35808..e283d47184 100644 --- a/tools/xenstored/core.c +++ b/tools/xenstored/core.c @@ -2034,6 +2034,10 @@ static struct { { "GET_FEATURE", do_get_feature, XS_FLAG_PRIV }, [XS_SET_FEATURE] = { "SET_FEATURE", do_set_feature, XS_FLAG_PRIV }, + [XS_GET_QUOTA] = + { "GET_QUOTA", do_get_quota, XS_FLAG_PRIV }, + [XS_SET_QUOTA] = + { "SET_QUOTA", do_set_quota, XS_FLAG_PRIV }, };static const char *sockmsg_string(enum xsd_sockmsg_type type)diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c index 8e52351695..c0bc8a3eb7 100644 --- a/tools/xenstored/domain.c +++ b/tools/xenstored/domain.c @@ -1363,6 +1363,112 @@ static bool parse_quota_name(const char *name, unsigned int *qidx, return true; }+int do_get_quota(const void *ctx, struct connection *conn,+ struct buffered_data *in) +{ + const char *vec[2]; + unsigned int n_pars; + unsigned int domid; + unsigned int q; + unsigned int idx; + char *resp; + const char *name; + const struct quota *quota; + const struct domain *domain; + + n_pars = get_strings(in, vec, ARRAY_SIZE(vec)); + + if (n_pars > 2) + return EINVAL; + + if (n_pars == 0) { + resp = talloc_asprintf(ctx, "%s", "");This could be written with talloc_strdup() instead, since there's no formatting involve.
Right.
+ if (!resp) + return ENOMEM; + for (q = 0; q < ACC_N; q++) { + if (!quota_adm[q].name) + continue; + if (quotas[q].val[Q_IDX_HARD] != Q_VAL_DISABLED) {Having set internally a value of Q_VAL_DISABLED, does it mean the named quota is unsupported?
Yes. Right now all hard quota are supported and only one soft quota is supported.
+ resp = talloc_asprintf_append(resp, "%s%s", + *resp ? " " : "", quota_adm[q].name); + if (!resp) + return ENOMEM; + } + if (quotas[q].val[Q_IDX_SOFT] != Q_VAL_DISABLED) { + resp = talloc_asprintf_append(resp, "%ssoft-%s", + *resp ? " " : "", quota_adm[q].name); + if (!resp) + return ENOMEM; + } + } + } else { + if (n_pars == 1) { + quota = quotas; + name = vec[0]; + } else { + domid = atoi(vec[0]);Shall we check that vec[0] actually contain a plausible domid? (An integer between 0..65535). Right now, this accept everything, and would return 0 if there's not a single digit.
I have followed the pattern used in other places where a domid is expected. In the end nothing will really break. Any integer not being a domid will result in ENOENT, while the case of not a digit is a bug in privileged software (domids can be specified by dom0 only).
+ domain = find_or_alloc_existing_domain(domid); + if (!domain) + return ENOENT; + quota = domain->acc; + name = vec[1]; + } + + if (parse_quota_name(name, &q, &idx)) + return EINVAL; + + resp = talloc_asprintf(ctx, "%u", quota[q].val[idx]);Why do we return 4294967295 for disabled quota check when the spec say to return "0" when a quota check is disabled? That is for quota names that are supposed to be not supported (if we ask "GET_QUOTA" first).
parse_quota_name() should have returned true in this case, so EINVAL should be returned. Will fix that.
+ if (!resp) + return ENOMEM; + } + + send_reply(conn, XS_GET_QUOTA, resp, strlen(resp) + 1); + + return 0; +} + +int do_set_quota(const void *ctx, struct connection *conn, + struct buffered_data *in) +{ + const char *vec[3]; + unsigned int n_pars; + unsigned int domid; + unsigned int q; + unsigned int idx; + const char *name; + unsigned int val; + struct quota *quota; + struct domain *domain; + + n_pars = get_strings(in, vec, ARRAY_SIZE(vec)); + + if (n_pars < 2 || n_pars > 3) + return EINVAL; + + if (n_pars == 2) { + quota = quotas; + name = vec[0]; + val = atoi(vec[1]);We should check that vec[1] is a valid quota value, and also not an internal value. Otherwise, we can just have "-1" on the wire, and have unexpected changes for example. Only "0" is documented as a quota been disabled, "-1" or "4294967295" isn't.
Right, I'll check for val != Q_VAL_DISABLED.
+ } else { + domid = atoi(vec[0]); + domain = find_or_alloc_existing_domain(domid); + if (!domain) + return ENOENT; + quota = domain->acc; + name = vec[1]; + val = atoi(vec[2]); + } + + if (parse_quota_name(name, &q, &idx)) + return EINVAL; + + quota[q].val[idx] = val; + + send_ack(conn, XS_SET_QUOTA); + + return 0; +} + static int close_xgt_handle(void *_handle) { xengnttab_close(*(xengnttab_handle **)_handle); diff --git a/tools/xenstored/domain.h b/tools/xenstored/domain.h index 62ce3b3166..6a06b0d1af 100644 --- a/tools/xenstored/domain.h +++ b/tools/xenstored/domain.h @@ -93,6 +93,14 @@ int do_get_feature(const void *ctx, struct connection *conn, int do_set_feature(const void *ctx, struct connection *conn, struct buffered_data *in);+/* Get quota names or value */This could say "implement GET_QUOTA" or something instead. But a comment here isn't going to give much value for internal functions.
If nobody objects I'll drop the comment.
+int do_get_quota(const void *ctx, struct connection *conn, + struct buffered_data *in); + +/* Set quota value */ +int do_set_quota(const void *ctx, struct connection *conn, + struct buffered_data *in); + void domain_early_init(void); void domain_init(int evtfd); void init_domains(bool live_update);
Thanks, Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
