On 13.03.26 15:23, Anthony PERARD wrote:
On Thu, Mar 05, 2026 at 02:51:58PM +0100, Juergen Gross wrote:diff --git a/tools/include/xenstore.h b/tools/include/xenstore.h index 423422dc50..6b661e5895 100644 --- a/tools/include/xenstore.h +++ b/tools/include/xenstore.h @@ -277,6 +277,25 @@ bool xs_get_features_domain(struct xs_handle *h, unsigned int domid, bool xs_set_features_domain(struct xs_handle *h, unsigned int domid, unsigned int features);+/* Get names of supported quota. */+char **xs_get_quota_names(struct xs_handle *h, unsigned int *num); + +/* Get the value of one global quota. */ +bool xs_get_global_quota(struct xs_handle *h, char *quota, + unsigned int *value); + +/* Set the value of one global quota. */ +bool xs_set_global_quota(struct xs_handle *h, char *quota, + unsigned int value); + +/* Get the value of one domain quota. */ +bool xs_get_domain_quota(struct xs_handle *h, unsigned int domid, + char *quota, unsigned int *value); + +/* Set the value of one domain quota. */ +bool xs_set_domain_quota(struct xs_handle *h, unsigned int domid, + char *quota, unsigned int value); +Do you think all those new prototype could get a bit more descriptions? Which parameter are actually output (and not input), what does it mean to return false, do they set errno, is there something to do with the return value of xs_get_quota_names?
Oh yes, of course.
For output arguments, libxl have a convention (well at least a mention in the coding style) to suffix argument names with `_r` or `_out`.
Hmm, I don't think I'd like to change style now. This is not libxl after all.
For the strings, could we use `const char *` instead of non-const one?
Yes.
diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c index 8f4b90a3cf..dda37f7526 100644 --- a/tools/libs/store/xs.c +++ b/tools/libs/store/xs.c @@ -1456,6 +1456,117 @@ bool xs_set_features_domain(struct xs_handle *h, unsigned int domid, return xs_bool(xs_talkv(h, iov, ARRAY_SIZE(iov), NULL)); }+char **xs_get_quota_names(struct xs_handle *h, unsigned int *num)+{ + struct xsd_sockmsg msg = { .type = XS_GET_QUOTA }; + struct iovec iov[1]; + char **quota; + char *reply; + char *c; + unsigned int i; + + iov[0].iov_base = &msg; + iov[0].iov_len = sizeof(msg); + + reply = xs_talkv(h, iov, ARRAY_SIZE(iov), NULL); + if (!reply) + return NULL; + + *num = 1; + for (c = reply; *c; c++) + if (*c == ' ') + (*num)++; + + quota = malloc(*num * sizeof(char *) + strlen(reply) + 1); + c = (char *)(quota + *num); + strcpy(c, reply); + for (i = 0; i < *num; i++) { + quota[i] = c; + c = strchr(c, ' '); + if (c) {If `c` is NULL, it's likely that this is the last iteration of the `for` loop. But just in case, should we prevent the code from doing another round and prevent `strchr(NULL, ' ')`? (Or just check that `c` is !NULL, and let the loop finish set NULL for the remaining slot in `quota`)
Not sure this is really needed. *num is set just a few lines further up using the same way to count the number of strings. Do we really need to do consistency checks of intermediate results in such a short function?
+ *c = 0; + c++; + } + } + + return quota; +} + +bool xs_get_global_quota(struct xs_handle *h, char *quota, + unsigned int *value) +{ + struct xsd_sockmsg msg = { .type = XS_GET_QUOTA }; + struct iovec iov[2]; + + iov[0].iov_base = &msg; + iov[0].iov_len = sizeof(msg); + iov[1].iov_base = quota; + iov[1].iov_len = strlen(quota) + 1; + + return xs_uint(xs_talkv(h, iov, ARRAY_SIZE(iov), NULL), value); +} + +bool xs_set_global_quota(struct xs_handle *h, char *quota, + unsigned int value) +{ + struct xsd_sockmsg msg = { .type = XS_SET_QUOTA }; + char val_str[MAX_STRLEN(value)];MAX_STRLEN doesn't have a great name, I wounder what is was :-). And it's not about a maximum size of payload that could go on xs wire or something, it's actually the maximum string size that can take a numerical value, when converted to charaters.
Unfortunately the MAX_STRLEN() macro is defined in a public header file. I could define another macro with a different name doing the same and use that here, but MAX_STRLEN() would still be there. What is your preference? Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
