+Henry for release ack
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 >
