Re: [PATCH v2 10/19] tools/xenstore: change per-domain node accounting interface

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 12/01/2023 05:49, Juergen Gross wrote:
> On 11.01.23 18:48, Julien Grall wrote:
> > Hi Juergen,
> >
> > On 11/01/2023 08:59, Juergen Gross wrote:
> > > > ... to make sure domain_nbentry_add() is not returning a negative 
> > > > value. Then it would not work.
> > > > A good example imagine you have a transaction removing nodes from 
> > > > tree but not adding any. Then the "ret" would be negative.
> > > > Meanwhile the nodes are also removed outside of the transaction. So 
> > > > the sum of "d->nbentry + ret" would be negative resulting to a 
> > > > failure here.
> > > Thanks for catching this.
> > >
> > > I think the correct way to handle this is to return max(d->nbentry + 
> > > ret, 0) in
> > > domain_nbentry_add(). The value might be imprecise, but always >= 0 
> > > and never
> > > wrong outside of a transaction collision.
> > I am bit confused with your proposal. If the return value is 
> > imprecise, then what's the point of returning max(...) instead of 
> > simply 0?
> Please have a look at the use case especially in domain_nbentry(). 
> Returning
> always 0 would clearly break quota checks.
I am a bit concerned that we would have a code checking the quota based 
on an imprecise value.
At the moment, I don't have a better suggestion. But we should at least 
document in the code when we think the value is imprecise and explain 
why bypassing the quota check is OK (IOW who will check it?).
Cheers,

--
Julien Grall