Re: [PATCH] xenbus: Fix loopback event channel assuming domain 0

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

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