| Message ID | 20230508114754.31514-13-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools/xenstore: rework internal accounting | expand |
Hi Juergen, On 08/05/2023 12:47, Juergen Gross wrote: > The maxrequests, node size, number of node permissions, and path length > quota are a little bit special, as they are either active in > transactions only (maxrequests), or they are just per item instead of > count values. Nevertheless being able to know the maximum number of > those quota related values per domain would be beneficial, so add them > to the generic accounting. > > The per domain value will never show current numbers other than zero, > but the maximum number seen can be gathered the same way as the number > of nodes during a transaction. > > To be able to use the const qualifier for a new function switch > domain_is_unprivileged() to take a const pointer, too. > > For printing the quota/max values, adapt the print format string to > the longest quota name (now 17 characters long). > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > V5: > - add comment (Julien Grall) > - add missing quota printing (Julien Grall) > --- > tools/xenstore/xenstored_core.c | 15 +++++---- > tools/xenstore/xenstored_core.h | 2 +- > tools/xenstore/xenstored_domain.c | 45 +++++++++++++++++++++----- > tools/xenstore/xenstored_domain.h | 6 ++++ > tools/xenstore/xenstored_transaction.c | 4 +-- > tools/xenstore/xenstored_watch.c | 2 +- > 6 files changed, 55 insertions(+), 19 deletions(-) > > diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c > index c98d30561f..fce73b883e 100644 > --- a/tools/xenstore/xenstored_core.c > +++ b/tools/xenstore/xenstored_core.c > @@ -799,8 +799,9 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node, > + node->perms.num * sizeof(node->perms.p[0]) > + node->datalen + node->childlen; > > - if (!no_quota_check && domain_is_unprivileged(conn) && > - data.dsize >= quota_max_entry_size) { > + /* Call domain_max_chk() in any case in order to record max values. */ > + if (domain_max_chk(conn, ACC_NODESZ, data.dsize, quota_max_entry_size) > + && !no_quota_check) { > errno = ENOSPC; > return errno; > } > @@ -1170,7 +1171,7 @@ static bool valid_chars(const char *node) > "0123456789-/_@") == strlen(node)); > } > > -bool is_valid_nodename(const char *node) > +bool is_valid_nodename(const struct connection *conn, const char *node) > { > int local_off = 0; > unsigned int domid; > @@ -1190,7 +1191,8 @@ bool is_valid_nodename(const char *node) > if (sscanf(node, "/local/domain/%5u/%n", &domid, &local_off) != 1) > local_off = 0; > > - if (strlen(node) > local_off + quota_max_path_len) > + if (domain_max_chk(conn, ACC_PATHLEN, strlen(node) - local_off, > + quota_max_path_len)) > return false; > > return valid_chars(node); > @@ -1252,7 +1254,7 @@ static struct node *get_node_canonicalized(struct connection *conn, > *canonical_name = canonicalize(conn, ctx, name); > if (!*canonical_name) > return NULL; > - if (!is_valid_nodename(*canonical_name)) { > + if (!is_valid_nodename(conn, *canonical_name)) { > errno = EINVAL; > return NULL; > } > @@ -1778,8 +1780,7 @@ static int do_set_perms(const void *ctx, struct connection *conn, > return EINVAL; > > perms.num--; > - if (domain_is_unprivileged(conn) && > - perms.num > quota_nb_perms_per_node) > + if (domain_max_chk(conn, ACC_NPERM, perms.num, quota_nb_perms_per_node)) > return ENOSPC; > > permstr = in->buffer + strlen(in->buffer) + 1; > diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h > index 3564d85d7d..9339820156 100644 > --- a/tools/xenstore/xenstored_core.h > +++ b/tools/xenstore/xenstored_core.h > @@ -258,7 +258,7 @@ void check_store(void); > void corrupt(struct connection *conn, const char *fmt, ...); > > /* Is this a valid node name? */ > -bool is_valid_nodename(const char *node); > +bool is_valid_nodename(const struct connection *conn, const char *node); > > /* Get name of parent node. */ > char *get_parent(const void *ctx, const char *node); > diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c > index 6f3a27765a..b3a719569e 100644 > --- a/tools/xenstore/xenstored_domain.c > +++ b/tools/xenstore/xenstored_domain.c > @@ -431,7 +431,7 @@ int domain_get_quota(const void *ctx, struct connection *conn, > return ENOMEM; > > #define ent(t, e) \ > - resp = talloc_asprintf_append(resp, "%-16s: %8u (max: %8u\n", #t, \ > + resp = talloc_asprintf_append(resp, "%-17s: %8u (max: %8u\n", #t, \ > d->acc[e].val, d->acc[e].max); \ > if (!resp) return ENOMEM > > @@ -440,6 +440,10 @@ int domain_get_quota(const void *ctx, struct connection *conn, > ent(transactions, ACC_TRANS); > ent(outstanding, ACC_OUTST); > ent(memory, ACC_MEM); > + ent(transaction-nodes, ACC_TRANSNODES); > + ent(node-permissions, ACC_NPERM); > + ent(path-length, ACC_PATHLEN); > + ent(node-size, ACC_NODESZ); > > #undef ent > > @@ -457,7 +461,7 @@ int domain_max_global_acc(const void *ctx, struct connection *conn) > return ENOMEM; > > #define ent(t, e) \ > - resp = talloc_asprintf_append(resp, "%-16s: %8u\n", #t, \ > + resp = talloc_asprintf_append(resp, "%-17s: %8u\n", #t, \ > acc_global_max[e]); \ > if (!resp) return ENOMEM > > @@ -466,6 +470,10 @@ int domain_max_global_acc(const void *ctx, struct connection *conn) > ent(transactions, ACC_TRANS); > ent(outstanding, ACC_OUTST); > ent(memory, ACC_MEM); > + ent(transaction-nodes, ACC_TRANSNODES); > + ent(node-permissions, ACC_NPERM); > + ent(path-length, ACC_PATHLEN); > + ent(node-size, ACC_NODESZ); > > #undef ent > > @@ -1079,12 +1087,22 @@ int domain_adjust_node_perms(struct node *node) > return 0; > } > > +static void domain_acc_valid_max(struct domain *d, enum accitem what, > + unsigned int val) > +{ > + assert(what < ARRAY_SIZE(d->acc)); > + assert(what < ARRAY_SIZE(acc_global_max)); > + > + if (val > d->acc[what].max) > + d->acc[what].max = val; > + if (val > acc_global_max[what] && domid_is_unprivileged(d->domid)) > + acc_global_max[what] = val; > +} > + > static int domain_acc_add_valid(struct domain *d, enum accitem what, int add) > { > unsigned int val; > > - assert(what < ARRAY_SIZE(d->acc)); > - I didn't have a chance to reply on your comment on the previous version. So doing it here: > Following this reasoning I'd need to put it into even more functions. Possibly. But for now, the discussion is about not removing the existing one (see more below). > And an assert() triggering a little bit late is no real problem, as it will abort xenstored anyway. Not really. Xenstored would only be aborted if the condition is false. If it is not, we would return an error. That said, the condition that a change to be true in some condition. But now you are relying on the compiler to never optimize out the check. We know that compilers can remove NULL check if a pointer was dereferenced beforehand. I wouldn't be surprised if they can do the same trick with accessing an array first and then checking the bound. So the abort() may actually never happen. > Additionally with the global and the per-domain arrays now covering all possible quotas, it would even be reasonable to drop the assert()s in domain_acc_valid_max() completely. I have two concerns: * This is the state after this series. But I don't see what would prevent any change in the future. * If I am not mistaken none of the compilers properly enforce the enum in C. So you could in theory pass an outside value without the compiler shouting at you. So to me, this is not warrant to completely drop the assert(). If we discard the latter point, the ideal would be a BUILD_BUG_ON() to tie the enum with the array but IIRC it is not possible to use BUILD_BUG_ON() with an enum. Therefore, the assert() should the best at the moment. Ideally, we should add the assert() in other places. But, this is not something I think should be requested here. My only request is to not removing the existing one. Cheers,
On 09.05.23 21:21, Julien Grall wrote: > Hi Juergen, > > On 08/05/2023 12:47, Juergen Gross wrote: >> The maxrequests, node size, number of node permissions, and path length >> quota are a little bit special, as they are either active in >> transactions only (maxrequests), or they are just per item instead of >> count values. Nevertheless being able to know the maximum number of >> those quota related values per domain would be beneficial, so add them >> to the generic accounting. >> >> The per domain value will never show current numbers other than zero, >> but the maximum number seen can be gathered the same way as the number >> of nodes during a transaction. >> >> To be able to use the const qualifier for a new function switch >> domain_is_unprivileged() to take a const pointer, too. >> >> For printing the quota/max values, adapt the print format string to >> the longest quota name (now 17 characters long). >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> V5: >> - add comment (Julien Grall) >> - add missing quota printing (Julien Grall) >> --- >> tools/xenstore/xenstored_core.c | 15 +++++---- >> tools/xenstore/xenstored_core.h | 2 +- >> tools/xenstore/xenstored_domain.c | 45 +++++++++++++++++++++----- >> tools/xenstore/xenstored_domain.h | 6 ++++ >> tools/xenstore/xenstored_transaction.c | 4 +-- >> tools/xenstore/xenstored_watch.c | 2 +- >> 6 files changed, 55 insertions(+), 19 deletions(-) >> >> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c >> index c98d30561f..fce73b883e 100644 >> --- a/tools/xenstore/xenstored_core.c >> +++ b/tools/xenstore/xenstored_core.c >> @@ -799,8 +799,9 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, >> struct node *node, >> + node->perms.num * sizeof(node->perms.p[0]) >> + node->datalen + node->childlen; >> - if (!no_quota_check && domain_is_unprivileged(conn) && >> - data.dsize >= quota_max_entry_size) { >> + /* Call domain_max_chk() in any case in order to record max values. */ >> + if (domain_max_chk(conn, ACC_NODESZ, data.dsize, quota_max_entry_size) >> + && !no_quota_check) { >> errno = ENOSPC; >> return errno; >> } >> @@ -1170,7 +1171,7 @@ static bool valid_chars(const char *node) >> "0123456789-/_@") == strlen(node)); >> } >> -bool is_valid_nodename(const char *node) >> +bool is_valid_nodename(const struct connection *conn, const char *node) >> { >> int local_off = 0; >> unsigned int domid; >> @@ -1190,7 +1191,8 @@ bool is_valid_nodename(const char *node) >> if (sscanf(node, "/local/domain/%5u/%n", &domid, &local_off) != 1) >> local_off = 0; >> - if (strlen(node) > local_off + quota_max_path_len) >> + if (domain_max_chk(conn, ACC_PATHLEN, strlen(node) - local_off, >> + quota_max_path_len)) >> return false; >> return valid_chars(node); >> @@ -1252,7 +1254,7 @@ static struct node *get_node_canonicalized(struct >> connection *conn, >> *canonical_name = canonicalize(conn, ctx, name); >> if (!*canonical_name) >> return NULL; >> - if (!is_valid_nodename(*canonical_name)) { >> + if (!is_valid_nodename(conn, *canonical_name)) { >> errno = EINVAL; >> return NULL; >> } >> @@ -1778,8 +1780,7 @@ static int do_set_perms(const void *ctx, struct >> connection *conn, >> return EINVAL; >> perms.num--; >> - if (domain_is_unprivileged(conn) && >> - perms.num > quota_nb_perms_per_node) >> + if (domain_max_chk(conn, ACC_NPERM, perms.num, quota_nb_perms_per_node)) >> return ENOSPC; >> permstr = in->buffer + strlen(in->buffer) + 1; >> diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h >> index 3564d85d7d..9339820156 100644 >> --- a/tools/xenstore/xenstored_core.h >> +++ b/tools/xenstore/xenstored_core.h >> @@ -258,7 +258,7 @@ void check_store(void); >> void corrupt(struct connection *conn, const char *fmt, ...); >> /* Is this a valid node name? */ >> -bool is_valid_nodename(const char *node); >> +bool is_valid_nodename(const struct connection *conn, const char *node); >> /* Get name of parent node. */ >> char *get_parent(const void *ctx, const char *node); >> diff --git a/tools/xenstore/xenstored_domain.c >> b/tools/xenstore/xenstored_domain.c >> index 6f3a27765a..b3a719569e 100644 >> --- a/tools/xenstore/xenstored_domain.c >> +++ b/tools/xenstore/xenstored_domain.c >> @@ -431,7 +431,7 @@ int domain_get_quota(const void *ctx, struct connection >> *conn, >> return ENOMEM; >> #define ent(t, e) \ >> - resp = talloc_asprintf_append(resp, "%-16s: %8u (max: %8u\n", #t, \ >> + resp = talloc_asprintf_append(resp, "%-17s: %8u (max: %8u\n", #t, \ >> d->acc[e].val, d->acc[e].max); \ >> if (!resp) return ENOMEM >> @@ -440,6 +440,10 @@ int domain_get_quota(const void *ctx, struct connection >> *conn, >> ent(transactions, ACC_TRANS); >> ent(outstanding, ACC_OUTST); >> ent(memory, ACC_MEM); >> + ent(transaction-nodes, ACC_TRANSNODES); >> + ent(node-permissions, ACC_NPERM); >> + ent(path-length, ACC_PATHLEN); >> + ent(node-size, ACC_NODESZ); >> #undef ent >> @@ -457,7 +461,7 @@ int domain_max_global_acc(const void *ctx, struct >> connection *conn) >> return ENOMEM; >> #define ent(t, e) \ >> - resp = talloc_asprintf_append(resp, "%-16s: %8u\n", #t, \ >> + resp = talloc_asprintf_append(resp, "%-17s: %8u\n", #t, \ >> acc_global_max[e]); \ >> if (!resp) return ENOMEM >> @@ -466,6 +470,10 @@ int domain_max_global_acc(const void *ctx, struct >> connection *conn) >> ent(transactions, ACC_TRANS); >> ent(outstanding, ACC_OUTST); >> ent(memory, ACC_MEM); >> + ent(transaction-nodes, ACC_TRANSNODES); >> + ent(node-permissions, ACC_NPERM); >> + ent(path-length, ACC_PATHLEN); >> + ent(node-size, ACC_NODESZ); >> #undef ent >> @@ -1079,12 +1087,22 @@ int domain_adjust_node_perms(struct node *node) >> return 0; >> } >> +static void domain_acc_valid_max(struct domain *d, enum accitem what, >> + unsigned int val) >> +{ >> + assert(what < ARRAY_SIZE(d->acc)); >> + assert(what < ARRAY_SIZE(acc_global_max)); >> + >> + if (val > d->acc[what].max) >> + d->acc[what].max = val; >> + if (val > acc_global_max[what] && domid_is_unprivileged(d->domid)) >> + acc_global_max[what] = val; >> +} >> + >> static int domain_acc_add_valid(struct domain *d, enum accitem what, int add) >> { >> unsigned int val; >> - assert(what < ARRAY_SIZE(d->acc)); >> - > > I didn't have a chance to reply on your comment on the previous version. So > doing it here: > > > Following this reasoning I'd need to put it into even more functions. > > Possibly. But for now, the discussion is about not removing the existing one > (see more below). > > > And an > assert() triggering a little bit late is no real problem, as it will abort > xenstored anyway. > > Not really. Xenstored would only be aborted if the condition is false. If it is > not, we would return an error. That said, the condition that a change to be true > in some condition. > > But now you are relying on the compiler to never optimize out the check. We know > that compilers can remove NULL check if a pointer was dereferenced beforehand. I > wouldn't be surprised if they can do the same trick with accessing an array > first and then checking the bound. So the abort() may actually never happen. > > > Additionally with the global and the per-domain arrays now covering all > possible quotas, it would even be reasonable to drop the assert()s in > domain_acc_valid_max() completely. > > I have two concerns: > * This is the state after this series. But I don't see what would prevent any > change in the future. > * If I am not mistaken none of the compilers properly enforce the enum in C. > So you could in theory pass an outside value without the compiler shouting at you. > > So to me, this is not warrant to completely drop the assert(). If we discard the > latter point, the ideal would be a BUILD_BUG_ON() to tie the enum with the array > but IIRC it is not possible to use BUILD_BUG_ON() with an enum. Therefore, the > assert() should the best at the moment. > > Ideally, we should add the assert() in other places. But, this is not something > I think should be requested here. My only request is to not removing the > existing one. Okay, I'll keep it. Juergen
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c index c98d30561f..fce73b883e 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c @@ -799,8 +799,9 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node, + node->perms.num * sizeof(node->perms.p[0]) + node->datalen + node->childlen; - if (!no_quota_check && domain_is_unprivileged(conn) && - data.dsize >= quota_max_entry_size) { + /* Call domain_max_chk() in any case in order to record max values. */ + if (domain_max_chk(conn, ACC_NODESZ, data.dsize, quota_max_entry_size) + && !no_quota_check) { errno = ENOSPC; return errno; } @@ -1170,7 +1171,7 @@ static bool valid_chars(const char *node) "0123456789-/_@") == strlen(node)); } -bool is_valid_nodename(const char *node) +bool is_valid_nodename(const struct connection *conn, const char *node) { int local_off = 0; unsigned int domid; @@ -1190,7 +1191,8 @@ bool is_valid_nodename(const char *node) if (sscanf(node, "/local/domain/%5u/%n", &domid, &local_off) != 1) local_off = 0; - if (strlen(node) > local_off + quota_max_path_len) + if (domain_max_chk(conn, ACC_PATHLEN, strlen(node) - local_off, + quota_max_path_len)) return false; return valid_chars(node); @@ -1252,7 +1254,7 @@ static struct node *get_node_canonicalized(struct connection *conn, *canonical_name = canonicalize(conn, ctx, name); if (!*canonical_name) return NULL; - if (!is_valid_nodename(*canonical_name)) { + if (!is_valid_nodename(conn, *canonical_name)) { errno = EINVAL; return NULL; } @@ -1778,8 +1780,7 @@ static int do_set_perms(const void *ctx, struct connection *conn, return EINVAL; perms.num--; - if (domain_is_unprivileged(conn) && - perms.num > quota_nb_perms_per_node) + if (domain_max_chk(conn, ACC_NPERM, perms.num, quota_nb_perms_per_node)) return ENOSPC; permstr = in->buffer + strlen(in->buffer) + 1; diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h index 3564d85d7d..9339820156 100644 --- a/tools/xenstore/xenstored_core.h +++ b/tools/xenstore/xenstored_core.h @@ -258,7 +258,7 @@ void check_store(void); void corrupt(struct connection *conn, const char *fmt, ...); /* Is this a valid node name? */ -bool is_valid_nodename(const char *node); +bool is_valid_nodename(const struct connection *conn, const char *node); /* Get name of parent node. */ char *get_parent(const void *ctx, const char *node); diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c index 6f3a27765a..b3a719569e 100644 --- a/tools/xenstore/xenstored_domain.c +++ b/tools/xenstore/xenstored_domain.c @@ -431,7 +431,7 @@ int domain_get_quota(const void *ctx, struct connection *conn, return ENOMEM; #define ent(t, e) \ - resp = talloc_asprintf_append(resp, "%-16s: %8u (max: %8u\n", #t, \ + resp = talloc_asprintf_append(resp, "%-17s: %8u (max: %8u\n", #t, \ d->acc[e].val, d->acc[e].max); \ if (!resp) return ENOMEM @@ -440,6 +440,10 @@ int domain_get_quota(const void *ctx, struct connection *conn, ent(transactions, ACC_TRANS); ent(outstanding, ACC_OUTST); ent(memory, ACC_MEM); + ent(transaction-nodes, ACC_TRANSNODES); + ent(node-permissions, ACC_NPERM); + ent(path-length, ACC_PATHLEN); + ent(node-size, ACC_NODESZ); #undef ent @@ -457,7 +461,7 @@ int domain_max_global_acc(const void *ctx, struct connection *conn) return ENOMEM; #define ent(t, e) \ - resp = talloc_asprintf_append(resp, "%-16s: %8u\n", #t, \ + resp = talloc_asprintf_append(resp, "%-17s: %8u\n", #t, \ acc_global_max[e]); \ if (!resp) return ENOMEM @@ -466,6 +470,10 @@ int domain_max_global_acc(const void *ctx, struct connection *conn) ent(transactions, ACC_TRANS); ent(outstanding, ACC_OUTST); ent(memory, ACC_MEM); + ent(transaction-nodes, ACC_TRANSNODES); + ent(node-permissions, ACC_NPERM); + ent(path-length, ACC_PATHLEN); + ent(node-size, ACC_NODESZ); #undef ent @@ -1079,12 +1087,22 @@ int domain_adjust_node_perms(struct node *node) return 0; } +static void domain_acc_valid_max(struct domain *d, enum accitem what, + unsigned int val) +{ + assert(what < ARRAY_SIZE(d->acc)); + assert(what < ARRAY_SIZE(acc_global_max)); + + if (val > d->acc[what].max) + d->acc[what].max = val; + if (val > acc_global_max[what] && domid_is_unprivileged(d->domid)) + acc_global_max[what] = val; +} + static int domain_acc_add_valid(struct domain *d, enum accitem what, int add) { unsigned int val; - assert(what < ARRAY_SIZE(d->acc)); - if ((add < 0 && -add > d->acc[what].val) || (add > 0 && (INT_MAX - d->acc[what].val) < add)) { /* @@ -1098,10 +1116,7 @@ static int domain_acc_add_valid(struct domain *d, enum accitem what, int add) } val = d->acc[what].val + add; - if (val > d->acc[what].max) - d->acc[what].max = val; - if (val > acc_global_max[what] && domid_is_unprivileged(d->domid)) - acc_global_max[what] = val; + domain_acc_valid_max(d, what, val); return val; } @@ -1222,6 +1237,20 @@ void domain_reset_global_acc(void) hashtable_iterate(domhash, domain_reset_global_acc_sub, NULL); } +bool domain_max_chk(const struct connection *conn, enum accitem what, + unsigned int val, unsigned int quota) +{ + if (!conn || !conn->domain) + return false; + + if (domain_is_unprivileged(conn) && val > quota) + return true; + + domain_acc_valid_max(conn->domain, what, val); + + return false; +} + int domain_nbentry_inc(struct connection *conn, unsigned int domid) { return (domain_acc_add(conn, domid, ACC_NODES, 1, false) < 0) diff --git a/tools/xenstore/xenstored_domain.h b/tools/xenstore/xenstored_domain.h index 416df25cb2..78ca434531 100644 --- a/tools/xenstore/xenstored_domain.h +++ b/tools/xenstore/xenstored_domain.h @@ -33,6 +33,10 @@ enum accitem { ACC_OUTST, ACC_MEM, ACC_TRANS, + ACC_TRANSNODES, + ACC_NPERM, + ACC_PATHLEN, + ACC_NODESZ, ACC_N, /* Number of elements per domain. */ }; @@ -129,6 +133,8 @@ void acc_drop(struct connection *conn); void acc_commit(struct connection *conn); 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, unsigned int what, + unsigned int val, unsigned int quota); /* Write rate limiting */ diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c index b9e9d76a1f..cecaf27018 100644 --- a/tools/xenstore/xenstored_transaction.c +++ b/tools/xenstore/xenstored_transaction.c @@ -252,8 +252,8 @@ int access_node(struct connection *conn, struct node *node, i = find_accessed_node(trans, node->name); if (!i) { - if (trans->nodes >= quota_trans_nodes && - domain_is_unprivileged(conn)) { + if (domain_max_chk(conn, ACC_TRANSNODES, trans->nodes + 1, + quota_trans_nodes)) { ret = ENOSPC; goto err; } diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c index e30cd89be3..61b1e3421e 100644 --- a/tools/xenstore/xenstored_watch.c +++ b/tools/xenstore/xenstored_watch.c @@ -176,7 +176,7 @@ static int check_watch_path(struct connection *conn, const void *ctx, *path = canonicalize(conn, ctx, *path); if (!*path) return errno; - if (!is_valid_nodename(*path)) + if (!is_valid_nodename(conn, *path)) goto inval; }
The maxrequests, node size, number of node permissions, and path length quota are a little bit special, as they are either active in transactions only (maxrequests), or they are just per item instead of count values. Nevertheless being able to know the maximum number of those quota related values per domain would be beneficial, so add them to the generic accounting. The per domain value will never show current numbers other than zero, but the maximum number seen can be gathered the same way as the number of nodes during a transaction. To be able to use the const qualifier for a new function switch domain_is_unprivileged() to take a const pointer, too. For printing the quota/max values, adapt the print format string to the longest quota name (now 17 characters long). Signed-off-by: Juergen Gross <jgross@suse.com> --- V5: - add comment (Julien Grall) - add missing quota printing (Julien Grall) --- tools/xenstore/xenstored_core.c | 15 +++++---- tools/xenstore/xenstored_core.h | 2 +- tools/xenstore/xenstored_domain.c | 45 +++++++++++++++++++++----- tools/xenstore/xenstored_domain.h | 6 ++++ tools/xenstore/xenstored_transaction.c | 4 +-- tools/xenstore/xenstored_watch.c | 2 +- 6 files changed, 55 insertions(+), 19 deletions(-)