Add central functions for checking a value (either an absolute one or
the current domain value plus an offset) against a specific quota.
This is in preparation of introducing per-domain quota.
The required changes allow to drop the "update" parameter from
domain_nbentry_fix().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
tools/xenstored/core.c | 4 +-
tools/xenstored/domain.c | 74 +++++++++++++++++------------------
tools/xenstored/domain.h | 7 ++--
tools/xenstored/transaction.c | 2 +-
tools/xenstored/watch.c | 4 +-
5 files changed, 43 insertions(+), 48 deletions(-)
diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
index d509736c32..5a4bf3e302 100644
--- a/tools/xenstored/core.c
+++ b/tools/xenstored/core.c
@@ -1538,7 +1538,7 @@ static struct node *create_node(struct connection *conn, const void *ctx,
for (i = node; i; i = i->parent) {
/* i->parent is set for each new node, so check quota. */
if (i->parent &&
- domain_nbentry(conn) >= hard_quotas[ACC_NODES].val) {
+ domain_check_quota_add(conn->domain, ACC_NODES, 1)) {
ret = ENOSPC;
goto err;
}
@@ -2320,7 +2320,7 @@ void setup_structure(bool live_update)
manual_node("/tool/xenstored", NULL);
manual_node("@releaseDomain", NULL);
manual_node("@introduceDomain", NULL);
- domain_nbentry_fix(priv_domid, 5, true);
+ domain_nbentry_fix(priv_domid, 5);
}
}
diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
index e453b3061f..1df9265ad5 100644
--- a/tools/xenstored/domain.c
+++ b/tools/xenstored/domain.c
@@ -389,6 +389,25 @@ void wrl_apply_debit_trans_commit(struct connection *conn)
wrl_apply_debit_actual(conn->domain);
}
+static bool domain_check_quota_val(struct domain *d, enum accitem what,
+ unsigned int val)
+{
+ unsigned int quota = hard_quotas[what].val;
+
+ if (!quota || !domid_is_unprivileged(d->domid))
+ return false;
+
+ return val >= quota;
+}
+
+bool domain_check_quota_add(struct domain *d, enum accitem what, int add)
+{
+ if (add < 0 || !d)
+ return false;
+
+ return domain_check_quota_val(d, what, d->acc[what].val + add);
+}
+
static bool check_indexes(XENSTORE_RING_IDX cons, XENSTORE_RING_IDX prod)
{
return ((prod - cons) <= XENSTORE_RING_SIZE);
@@ -490,10 +509,9 @@ static bool domain_can_read(struct connection *conn)
if (domain_is_unprivileged(conn)) {
if (domain->wrl_credit < 0)
return false;
- if (domain->acc[ACC_OUTST].val >= hard_quotas[ACC_OUTST].val)
+ if (domain_check_quota_add(domain, ACC_OUTST, 0))
return false;
- if (domain->acc[ACC_MEM].val >= hard_quotas[ACC_MEM].val &&
- hard_quotas[ACC_MEM].val)
+ if (domain_check_quota_add(domain, ACC_MEM, 0))
return false;
}
@@ -904,16 +922,20 @@ do { \
int acc_fix_domains(struct list_head *head, bool chk_quota, bool update)
{
+ struct domain *d;
struct changed_domain *cd;
- int cnt;
list_for_each_entry(cd, head, list) {
- cnt = domain_nbentry_fix(cd->domid, cd->acc[ACC_NODES], update);
- if (!update) {
- if (chk_quota && cnt >= hard_quotas[ACC_NODES].val)
- return ENOSPC;
- if (cnt < 0)
+ if (update) {
+ domain_nbentry_fix(cd->domid, cd->acc[ACC_NODES]);
+ } else if (chk_quota) {
+ d = find_or_alloc_existing_domain(cd->domid);
+
+ if (!d)
return ENOMEM;
+ if (domain_check_quota_add(d, ACC_NODES,
+ cd->acc[ACC_NODES]))
+ return ENOSPC;
}
}
@@ -1732,7 +1754,7 @@ bool domain_max_chk(const struct connection *conn, enum accitem what,
if (!conn || !conn->domain)
return false;
- if (domain_is_unprivileged(conn) && val > hard_quotas[what].val)
+ if (domain_check_quota_val(conn->domain, what, val))
return true;
domain_acc_valid_max(conn->domain, what, val);
@@ -1752,21 +1774,9 @@ int domain_nbentry_dec(struct connection *conn, unsigned int domid)
? errno : 0;
}
-int domain_nbentry_fix(unsigned int domid, int num, bool update)
-{
- int ret;
-
- ret = domain_acc_add(NULL, domid, ACC_NODES, update ? num : 0, update);
- if (ret < 0 || update)
- return ret;
-
- return domid_is_unprivileged(domid) ? ret + num : 0;
-}
-
-unsigned int domain_nbentry(struct connection *conn)
+int domain_nbentry_fix(unsigned int domid, int num)
{
- return domain_is_unprivileged(conn)
- ? domain_acc_add(conn, conn->id, ACC_NODES, 0, true) : 0;
+ return domain_acc_add(NULL, domid, ACC_NODES, num, true);
}
static bool domain_chk_quota(struct connection *conn, unsigned int mem)
@@ -1781,7 +1791,7 @@ static bool domain_chk_quota(struct connection *conn, unsigned int mem)
domain = conn->domain;
now = time(NULL);
- if (mem >= hard_quotas[ACC_MEM].val && hard_quotas[ACC_MEM].val) {
+ if (domain_check_quota_val(domain, ACC_MEM, mem)) {
if (domain->hard_quota_reported)
return true;
syslog(LOG_ERR, "Domain %u exceeds hard memory quota, Xenstore interface to domain stalled\n",
@@ -1857,13 +1867,6 @@ void domain_watch_dec(struct connection *conn)
domain_acc_add(conn, conn->id, ACC_WATCH, -1, true);
}
-int domain_watch(struct connection *conn)
-{
- return (domain_is_unprivileged(conn))
- ? domain_acc_add(conn, conn->id, ACC_WATCH, 0, true)
- : 0;
-}
-
void domain_outstanding_inc(struct connection *conn)
{
domain_acc_add(conn, conn->id, ACC_OUTST, 1, true);
@@ -1884,13 +1887,6 @@ void domain_transaction_dec(struct connection *conn)
domain_acc_add(conn, conn->id, ACC_TRANS, -1, true);
}
-unsigned int domain_transaction_get(struct connection *conn)
-{
- return (domain_is_unprivileged(conn))
- ? domain_acc_add(conn, conn->id, ACC_TRANS, 0, true)
- : 0;
-}
-
const char *dump_state_connections(FILE *fp)
{
const char *ret = NULL;
diff --git a/tools/xenstored/domain.h b/tools/xenstored/domain.h
index 28186ccac0..b229f6f4e0 100644
--- a/tools/xenstored/domain.h
+++ b/tools/xenstored/domain.h
@@ -113,8 +113,7 @@ int domain_alloc_permrefs(struct node_perms *perms);
/* Quota manipulation */
int domain_nbentry_inc(struct connection *conn, unsigned int domid);
int domain_nbentry_dec(struct connection *conn, unsigned int domid);
-int domain_nbentry_fix(unsigned int domid, int num, bool update);
-unsigned int domain_nbentry(struct connection *conn);
+int domain_nbentry_fix(unsigned int domid, int num);
int domain_memory_add(struct connection *conn, unsigned int domid, int mem,
bool no_quota_check);
@@ -141,12 +140,10 @@ static inline void domain_memory_add_nochk(struct connection *conn,
}
void domain_watch_inc(struct connection *conn);
void domain_watch_dec(struct connection *conn);
-int domain_watch(struct connection *conn);
void domain_outstanding_inc(struct connection *conn);
void domain_outstanding_dec(struct connection *conn, unsigned int domid);
void domain_transaction_inc(struct connection *conn);
void domain_transaction_dec(struct connection *conn);
-unsigned int domain_transaction_get(struct connection *conn);
int domain_get_quota(const void *ctx, struct connection *conn,
unsigned int domid);
@@ -161,6 +158,8 @@ int domain_max_global_acc(const void *ctx, struct connection *conn);
void domain_reset_global_acc(void);
bool domain_max_chk(const struct connection *conn, enum accitem what,
unsigned int val);
+bool domain_check_quota_add(struct domain *d, enum accitem what,
+ int add);
extern long wrl_ntransactions;
diff --git a/tools/xenstored/transaction.c b/tools/xenstored/transaction.c
index 167cd597fd..ccf93a1132 100644
--- a/tools/xenstored/transaction.c
+++ b/tools/xenstored/transaction.c
@@ -470,7 +470,7 @@ int do_transaction_start(const void *ctx, struct connection *conn,
if (conn->transaction)
return EBUSY;
- if (domain_transaction_get(conn) > hard_quotas[ACC_TRANS].val)
+ if (domain_check_quota_add(conn->domain, ACC_TRANS, 1))
return ENOSPC;
/* Attach transaction to ctx for autofree until it's complete */
diff --git a/tools/xenstored/watch.c b/tools/xenstored/watch.c
index b66a9f1a39..36e4d33f22 100644
--- a/tools/xenstored/watch.c
+++ b/tools/xenstored/watch.c
@@ -220,8 +220,8 @@ int do_watch(const void *ctx, struct connection *conn, struct buffered_data *in)
return EEXIST;
}
- if (domain_watch(conn) > hard_quotas[ACC_WATCH].val)
- return E2BIG;
+ if (domain_check_quota_add(conn->domain, ACC_WATCH, 1))
+ return ENOSPC;
watch = add_watch(conn, vec[0], vec[1], relative, false);
if (!watch)
--
2.53.0On 2026-03-05 08:51, Juergen Gross wrote:
> Add central functions for checking a value (either an absolute one or
> the current domain value plus an offset) against a specific quota.
>
> This is in preparation of introducing per-domain quota.
>
> The required changes allow to drop the "update" parameter from
> domain_nbentry_fix().
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
> index e453b3061f..1df9265ad5 100644
> --- a/tools/xenstored/domain.c
> +++ b/tools/xenstored/domain.c
> @@ -389,6 +389,25 @@ void wrl_apply_debit_trans_commit(struct connection *conn)
> wrl_apply_debit_actual(conn->domain);
> }
>
> +static bool domain_check_quota_val(struct domain *d, enum accitem what,
> + unsigned int val)
> +{
> + unsigned int quota = hard_quotas[what].val;
> +
> + if (!quota || !domid_is_unprivileged(d->domid))
> + return false;
> +
> + return val >= quota;
Personally, I don't like the naming of *check* where the "good" return
is false. That seems backwards from what I expect. So I'd suggest
either flipping the return value or renaming. domain_quota_fail() or
something?
> +}
> +
> +bool domain_check_quota_add(struct domain *d, enum accitem what, int add)
> +{
> + if (add < 0 || !d)
> + return false;
> +
> + return domain_check_quota_val(d, what, d->acc[what].val + add);
> +}
> +
> static bool check_indexes(XENSTORE_RING_IDX cons, XENSTORE_RING_IDX prod)
> {
> return ((prod - cons) <= XENSTORE_RING_SIZE);
As an example, here "good" is true.
I see Anthony already gave an R-b, so just consider it as a suggestion.
Thanks,
Jason
On 13.03.26 22:22, Jason Andryuk wrote:
> On 2026-03-05 08:51, Juergen Gross wrote:
>> Add central functions for checking a value (either an absolute one or
>> the current domain value plus an offset) against a specific quota.
>>
>> This is in preparation of introducing per-domain quota.
>>
>> The required changes allow to drop the "update" parameter from
>> domain_nbentry_fix().
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>
>> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
>> index e453b3061f..1df9265ad5 100644
>> --- a/tools/xenstored/domain.c
>> +++ b/tools/xenstored/domain.c
>> @@ -389,6 +389,25 @@ void wrl_apply_debit_trans_commit(struct connection *conn)
>> wrl_apply_debit_actual(conn->domain);
>> }
>> +static bool domain_check_quota_val(struct domain *d, enum accitem what,
>> + unsigned int val)
>> +{
>> + unsigned int quota = hard_quotas[what].val;
>> +
>> + if (!quota || !domid_is_unprivileged(d->domid))
>> + return false;
>> +
>> + return val >= quota;
>
> Personally, I don't like the naming of *check* where the "good" return is
> false. That seems backwards from what I expect. So I'd suggest either flipping
> the return value or renaming. domain_quota_fail() or something?
I'll go with domain_quota_exceeded().
Thanks,
Juergen
On Thu, Mar 05, 2026 at 02:51:59PM +0100, Juergen Gross wrote:
> Add central functions for checking a value (either an absolute one or
> the current domain value plus an offset) against a specific quota.
>
> This is in preparation of introducing per-domain quota.
>
> The required changes allow to drop the "update" parameter from
> domain_nbentry_fix().
This patch changes the return value of do_watch(), from E2BIG to ENOSPC,
but that's not mention.
> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
> index e453b3061f..1df9265ad5 100644
> --- a/tools/xenstored/domain.c
> +++ b/tools/xenstored/domain.c
> @@ -490,10 +509,9 @@ static bool domain_can_read(struct connection *conn)
> if (domain_is_unprivileged(conn)) {
> if (domain->wrl_credit < 0)
> return false;
> - if (domain->acc[ACC_OUTST].val >= hard_quotas[ACC_OUTST].val)
> + if (domain_check_quota_add(domain, ACC_OUTST, 0))
Is this change fixing a potential bug? As now we check if there's a
quota for OUTST.
> return false;
> - if (domain->acc[ACC_MEM].val >= hard_quotas[ACC_MEM].val &&
> - hard_quotas[ACC_MEM].val)
> + if (domain_check_quota_add(domain, ACC_MEM, 0))
> return false;
> }
>
> diff --git a/tools/xenstored/watch.c b/tools/xenstored/watch.c
> index b66a9f1a39..36e4d33f22 100644
> --- a/tools/xenstored/watch.c
> +++ b/tools/xenstored/watch.c
> @@ -220,8 +220,8 @@ int do_watch(const void *ctx, struct connection *conn, struct buffered_data *in)
> return EEXIST;
> }
>
> - if (domain_watch(conn) > hard_quotas[ACC_WATCH].val)
> - return E2BIG;
> + if (domain_check_quota_add(conn->domain, ACC_WATCH, 1))
> + return ENOSPC;
Change of return value here not mentionned, even if it now aligned with
the value returned in other places.
>
> watch = add_watch(conn, vec[0], vec[1], relative, false);
> if (!watch)
Patch looks fine to me:
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Thanks,
--
Anthony Perard | Vates XCP-ng Developer
XCP-ng & Xen Orchestra - Vates solutions
web: https://vates.tech
On 13.03.26 16:01, Anthony PERARD wrote:
> On Thu, Mar 05, 2026 at 02:51:59PM +0100, Juergen Gross wrote:
>> Add central functions for checking a value (either an absolute one or
>> the current domain value plus an offset) against a specific quota.
>>
>> This is in preparation of introducing per-domain quota.
>>
>> The required changes allow to drop the "update" parameter from
>> domain_nbentry_fix().
>
> This patch changes the return value of do_watch(), from E2BIG to ENOSPC,
> but that's not mention.
I can add a remark to the commit message.
>
>> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
>> index e453b3061f..1df9265ad5 100644
>> --- a/tools/xenstored/domain.c
>> +++ b/tools/xenstored/domain.c
>> @@ -490,10 +509,9 @@ static bool domain_can_read(struct connection *conn)
>> if (domain_is_unprivileged(conn)) {
>> if (domain->wrl_credit < 0)
>> return false;
>> - if (domain->acc[ACC_OUTST].val >= hard_quotas[ACC_OUTST].val)
>> + if (domain_check_quota_add(domain, ACC_OUTST, 0))
>
> Is this change fixing a potential bug? As now we check if there's a
> quota for OUTST.
This is just changing how the quota is tested.
>
>> return false;
>> - if (domain->acc[ACC_MEM].val >= hard_quotas[ACC_MEM].val &&
>> - hard_quotas[ACC_MEM].val)
>> + if (domain_check_quota_add(domain, ACC_MEM, 0))
>> return false;
>> }
>>
>> diff --git a/tools/xenstored/watch.c b/tools/xenstored/watch.c
>> index b66a9f1a39..36e4d33f22 100644
>> --- a/tools/xenstored/watch.c
>> +++ b/tools/xenstored/watch.c
>> @@ -220,8 +220,8 @@ int do_watch(const void *ctx, struct connection *conn, struct buffered_data *in)
>> return EEXIST;
>> }
>>
>> - if (domain_watch(conn) > hard_quotas[ACC_WATCH].val)
>> - return E2BIG;
>> + if (domain_check_quota_add(conn->domain, ACC_WATCH, 1))
>> + return ENOSPC;
>
> Change of return value here not mentionned, even if it now aligned with
> the value returned in other places.
>
>>
>> watch = add_watch(conn, vec[0], vec[1], relative, false);
>> if (!watch)
>
> Patch looks fine to me:
> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Thanks
Juergen
© 2016 - 2026 Red Hat, Inc.