Hi Stefano, > On Oct 17, 2023, at 07:14, Stefano Stabellini <[email protected]> wrote: > > +Henry for release ack
Thanks, I think we did have Juergen for reviewing this patch so: Release-acked-by: Henry Wang <[email protected]> Kind regards, Henry > > > On Fri, 13 Oct 2023, Stefano Stabellini wrote: >> From: George Dunlap <[email protected]> >> >> Commit fc2b57c9a ("xenstored: send an evtchn notification on >> introduce_domain") introduced the sending of an event channel to the >> guest when first introduced, so that dom0less domains waiting for the >> connection would know that xenstore was ready to use. >> >> Unfortunately, it was introduced in introduce_domain(), which 1) is >> called by other functions, where such functionality is unneeded, and >> 2) after the main XS_INTRODUCE call, calls domain_conn_reset(). This >> introduces a race condition, whereby if xenstored is delayed, a domain >> can wake up, send messages to the buffer, only to have them deleted by >> xenstore before finishing its processing of the XS_INTRODUCE message. >> >> Move the connect-and-notfy call into do_introduce() instead, after the >> domain_conn_rest(); predicated on the state being in the >> XENSTORE_RECONNECT state. >> >> (We don't need to check for "restoring", since that value is always >> passed as "false" from do_domain_introduce()). >> >> Also take the opportunity to add a missing wmb barrier after resetting >> the indexes of the ring in domain_conn_reset. >> >> This change will also remove an extra event channel notification for >> dom0 (because the notification is now done by do_introduce which is not >> called for dom0.) The extra dom0 event channel notification was only >> introduced by fc2b57c9a and was never present before. It is not needed >> because dom0 is the one to tell xenstored the connection parameters, so >> dom0 has to know that the ring page is setup correctly by the time >> xenstored starts looking at it. It is dom0 that performs the ring page >> init. >> >> Signed-off-by: George Dunlap <[email protected]> >> Signed-off-by: Stefano Stabellini <[email protected]> >> CC: [email protected] >> CC: [email protected] >> CC: [email protected] >> --- >> tools/xenstored/domain.c | 14 ++++++++------ >> 1 file changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c >> index a6cd199fdc..f6ef37856c 100644 >> --- a/tools/xenstored/domain.c >> +++ b/tools/xenstored/domain.c >> @@ -923,6 +923,7 @@ static void domain_conn_reset(struct domain *domain) >> >> domain->interface->req_cons = domain->interface->req_prod = 0; >> domain->interface->rsp_cons = domain->interface->rsp_prod = 0; >> + xen_wmb(); >> } >> >> /* >> @@ -988,12 +989,6 @@ static struct domain *introduce_domain(const void *ctx, >> /* Now domain belongs to its connection. */ >> talloc_steal(domain->conn, domain); >> >> - if (!restore) { >> - /* Notify the domain that xenstore is available */ >> - interface->connection = XENSTORE_CONNECTED; >> - xenevtchn_notify(xce_handle, domain->port); >> - } >> - >> if (!is_master_domain && !restore) >> fire_special_watches("@introduceDomain"); >> } else { >> @@ -1033,6 +1028,13 @@ int do_introduce(const void *ctx, struct connection >> *conn, >> >> domain_conn_reset(domain); >> >> + if (domain->interface != NULL && >> + domain->interface->connection == XENSTORE_RECONNECT) { >> + /* Notify the domain that xenstore is available */ >> + domain->interface->connection = XENSTORE_CONNECTED; >> + xenevtchn_notify(xce_handle, domain->port); >> + } >> + >> send_ack(conn, XS_INTRODUCE); >> >> return 0; >> -- >> 2.25.1 >>
