[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] docs: remove wrong statement about bug in xenstore
On 24/10/16 14:06, Andrew Cooper wrote: > On 24/10/16 12:49, Juergen Gross wrote: >> On 24/10/16 13:41, Wei Liu wrote: >>> On Mon, Oct 24, 2016 at 01:27:17PM +0200, Juergen Gross wrote: >>>> docs/misc/xenstore.txt states that xenstored will use "0" as a valid >>>> transaction id after 2^32 transactions. This is not true. Remove that >>>> statement. >>>> >>>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx> >>> Can you point me to the relevant code snippet? Better still I would like >>> to see why it is the case in commit message. >> Sure: tools/xenstore/xenstored_transaction.c >> >> do_transaction_start(): >> ... >> /* Pick an unused transaction identifier. */ >> do { >> trans->id = conn->next_transaction_id; >> exists = transaction_lookup(conn, >> conn->next_transaction_id++); >> } while (!IS_ERR(exists)); >> >> It should be noted here that conn->next_transaction_id is initialized >> to be 0. So the error would occur for the first transaction, too. > > Cxenstored isn't the only xenstored implementation, and I can't see > anything in the Ocaml version which mitigates this issue. Furthermore, > because Ocaml's int is 31 bits or 63 bits, I suspect a 64bit oxenstored > will become unusable when the transaction id hits 4 billion and an a > truncation occurs when writing the id into the ring. A 32bit oxenstored > only uses half the available transaction id space, and does wrap around > to 0. Okay, so either oxenstored should be corrected by some ocaml capable developer, or I can send a patch which will limit the bug statement to oxenstored. Such a simple to fix problem should not be just mentioned in some text file, but it should be fixed! Leaving the text unmodified is no option IMHO. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |