From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel De Graaf Subject: Re: [PATCH] xenbus: Fix loopback event channel assuming domain 0 Date: Thu, 06 Oct 2011 14:32:05 -0400 Message-ID: <4E8DF425.9000807@tycho.nsa.gov> References: <1317824920-639-1-git-send-email-dgdegra@tycho.nsa.gov> <20109.60188.905267.410813@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20109.60188.905267.410813@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Ian Jackson Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On 10/06/2011 01:53 PM, Ian Jackson wrote: > Daniel De Graaf writes ("[Xen-devel] [PATCH] xenbus: Fix loopback event channel assuming domain 0"): >> The xenbus event channel established in xenbus_init is intended to be a >> loopback channel, but the remote domain was hardcoded to 0; this will >> cause the channel to be unusable when xenstore is not being run in >> domain 0. > > I'm not sure I understand this. > > ... >> /* Next allocate a local port which xenstored can bind to */ >> alloc_unbound.dom = DOMID_SELF; >> - alloc_unbound.remote_dom = 0; >> + alloc_unbound.remote_dom = DOMID_SELF; > > The comment doesn't suggest that this is supposedly a loopback channel > (ie one for use by the xenbus client for signalling to itself, > somehow). The event channel being changed here is a loopback event channel exposed in /proc/xen/xsd_port, which xenstored binds to. This code is only used for the initial domain; otherwise, the shared info page is used. > Rather it's supposed to be a channel to xenstore. So the remote > domain should be the xenstore domain, which should come from the > shared info page. > > Have you actually tested this with a separate xenstored domain ? > > Ian. > The test setup that exposed this issue is having a non-dom0 Linux domain running xenstored (in addition to other services); this domain is started with the SIF_INITDOMAIN flag set. It has been tested and can start other domains with references back to the xenstored running there; the local kernel is able to communicate with the locally running xenstore to provide backend services. The test for xen_initial_domain() here might better be replaced with a check on xen_start_info->store_evtchn which should be a valid event channel on all domains except the domain running xenstored. This seems like a more elegant solution than relying on the SIF_INITDOMAIN flag to determine the location of xenstore. -- Daniel De Graaf National Security Agency