Re: [PATCH v2 01/13] tools/xenstore: don't allow creating too many nodes in a transaction

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 21/02/2023 08:10, Juergen Gross wrote:
> On 20.02.23 19:01, Julien Grall wrote:
> > So I have recreated an XTF test which I think match what you wrote [1].
> >
> > It is indeed failing without your patch. But then there are still some 
> > weird behavior here.
> > I would expect the creation of the node would also fail if instead of 
> > removing the node if removed outside of the transaction.
> > This is not the case because we are looking at the current quota. So 
> > shouldn't we snapshot the global count?
> As we don't do a global snapshot of the data base for a transaction 
> (this was
> changed due to huge memory needs, bad performance, and a higher transaction
> failure rate), 
I am a bit surprised that the only way to do proper transaction is to 
have a global snapshot. Instead, you could have an overlay.
> I don't think we should snapshot the count either.
But that would mean that the quota will change depending on modification 
of the global database while the transaction is inflight.
I guess this is not better nor worse that the current situation. But it 
is still really confusing for a client because:
  1) The error could happen at random point
  2) You may see an inconsistent database as nodes are only cached when 
they are first accessed
> A transaction is performed atomically at the time it is finished. Therefore
> seeing the current global state inside the transaction (with the 
> transaction
> private state on top like an overlay) is absolutely fine IMO.
To me it is just showing that our concept of transaction is very broken 
in C Xenstored. I am curious to know whether OXenstored is behaving the 
same way.
Anyway, I agree this is not better nor worse than the current situation. 
So I will acked this patch:
Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall