Re: [PATCH 11/20] tools/xenstore: move changed domain handling

[Juergen Gross <jgross@xxxxxxxx>]

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?


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature