[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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





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?

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.

Rather than forcing in this case, how about we quiesce the connection if it starts a transaction when LiveUpdate is pending?

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.