Re: [PATCH for-4.15] tools/xenstored: liveupdate: Properly check long transaction

[Jürgen Groß <jgross@xxxxxxxx>]

On 04.03.21 10:39, Julien Grall wrote:
> On 04/03/2021 09:00, Jürgen Groß wrote:
> > On 03.03.21 18:05, Julien Grall wrote:
> > > From: Julien Grall <jgrall@xxxxxxxxxx>
> > >
> > > As XenStored is single-threaded, conn->ta_start_time will always be
> > > smaller than now. As we substract the latter from the former, it means
> > > a transaction will never be considered long running.
> > >
> > > Invert the two operands of the substraction in both lu_reject_reason()
> > > and lu_check_allowed(). In addition to that, the former also needs to
> > > check that conn->ta_start_time is not 0 (i.e the transaction is not
> > > active).
> > >
> > > Take the opportunity to document the return condition of
> > > lu_check_allowed().
> > >
> > > Fixes: e04e53a5be20 ("tools/xenstore: allow live update only with no 
> > > transaction active")
> > > Reported-by: Bjoern Doebel <doebel@xxxxxxxxx>
> > > Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> > Reviewed-by: Juergen Gross <jgross@xxxxxxxx>
> >
> > > ---
> > >
> > > I am a bit puzzled on how -F is implemented. From my understanding we
> > > will force LiveUpdate when one of the following conditions is met:
> > >    1) All the active transactions are long running
> > >    2) If we didn't manage to LiveUpdate after N sec
> > >
> > > It is not quite clear why we need the both as 2) would indirectly cover
> > > 1). However 2) is probably unsafe as we may reset transactions for
> > > "well-behaving" guest.
> > >
> > > So I am thinking to send a patch to drop 2). Any opinions?
> > This will enable two guests working together to block LU by using
> > overlapping transactions:
> >
> > Guest 1: ----- ----- -----  ...
> > Guest 2: -- ----- ----- --- ... >
> > There is always a transaction active, but none is regarded to be
> > long running.
> Right, how do you know whether two guests are working together?
We can't know that. And this is the reason why you have to use the -F
option to force a LU.

> I understand that "-F" means that things could break... However, I am 
> not entirely sure who can realistically use this option without risking 
> breaking other guests. For instance, there might be a 3rd guest that has 
> an active transaction and not cooperating with the first two.
Yes. OTOH the chances are rather low that multiple LU attempts are
failing due to transactions being active all the time.

> Rather than forcing in this case, how about we quiesce the connection if 
> it starts a transaction when LiveUpdate is pending?
Yes, this should work, too.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys

Attachment: OpenPGP_signature
Description: OpenPGP digital signature