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

[Juergen Gross <jgross@xxxxxxxx>]

On 13.01.23 10:53, Julien Grall wrote:
> 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?).
The imprecise value will never be too low, it can only be too high (i.e. 0
instead of negative), and that will only happen in a transaction which can't
succeed.

Adding a comment is good idea, though.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature