Hi Juergen,
On 13/12/2022 06:53, Juergen Gross wrote:
On 01.12.22 22:58, Julien Grall wrote:
Hi Juergen,
On 01/11/2022 15:28, Juergen Gross wrote:
static bool check_indexes(XENSTORE_RING_IDX cons, XENSTORE_RING_IDX
prod)
@@ -492,8 +504,12 @@ static struct domain
*find_or_alloc_existing_domain(unsigned int domid)
xc_dominfo_t dominfo;
domain = find_domain_struct(domid);
- if (!domain && get_domain_info(domid, &dominfo))
- domain = alloc_domain(NULL, domid);
+ if (!domain) {
+ if (!get_domain_info(domid, &dominfo))
+ errno = ENOENT;
+ else
+ domain = alloc_domain(NULL, domid);
+ }
I don't understand how this change is related to this commit.
It is directly related to the hunk below. Returning errno in
acc_add_dom_nbentry() requires it to be set correctly in
find_or_alloc_existing_domain().
I'll add a remark in the commit message.
[...]
+int acc_add_dom_nbentry(const void *ctx, struct list_head *head, int
val,
+ unsigned int domid)
+{
+ struct changed_domain *cd;
+
+ cd = acc_get_changed_domain(ctx, head, domid);
+ if (!cd)
+ return errno;
+
+ cd->nbentry += val;
As a future improvement, it would be worth considering to check for
underflow/overflow.
Do you really think we need to make sure not to add/remove more than
2 billion nodes owned by a single domain?
No and that's not my point. If you look at domain_entry_fix() we have an
assert() to check if the sum is still over 0.
This assert() was actually triggered a few times while testing the
previous XSAs batch. So I think it would be worth to carry a similar
check (maybe not an assert()) just in case we mess up with accounting in
the future.
Cheers,
--
Julien Grall