Is: Xen pci backend state transition from Closed -> Initialized broken. Was:Re: 10s shutdown delay for PV guests with PCI passthrough

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Sun, Jul 20, 2014 at 09:01:38PM +0100, Simon Martin wrote:
> Hello Konrad,
> 
> Friday, July 18, 2014, 7:45:06 PM, you wrote:
> 
> >>
> >> 3.- frontend set state XenbusStateInitialising and waits for frontend
> >> to go to a state in the interval [XenbusStateInitWait,
> >> XenbusStateClosed).
> >> 
> >> If I perform step 3 then the frontend never exits as the backend state
> >> stays at XenbusStateClosed
> 
> > It looks like there is no code for XenbusStateInitialising so it
> > just ignores it. There is a state for XenbusStateInitialised.
> 
> OK.  So  do  I set XenbusStateInitialised? If so, what's the handshake
> value from the backend?

Looking at the code (xen_pcibk_frontend_changed) it looks like it
will ignore it - as 'xen_pcibk_attach' does:

149         /* Make sure we only do this setup once */                              
150         if (xenbus_read_driver_state(pdev->xdev->nodename) !=                   
151             XenbusStateInitialised)            

so it will just exit (as the last state is XenbusStateClosed) and not
perform the steps. 

It looks like the steps it expects are that the backend state gets
moved by a toolstack to XenbusStateInitialised.  Comparing that
to the other backend drivers - like Xen Block backend - it looks
like it is missing a step. That is if fe is XenbusStateClosed
the be goes to XenbusStateClosed. If the fe goes to XenbusStateInitialising
the be should be OK continuing if it was in
XenbusStateClosed (reconnect) or in XenbusStateInitialised (first time).

Would this patch help (not compile tested)?

diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
index 4a7e6e0..c518b42 100644
--- a/drivers/xen/xen-pciback/xenbus.c
+++ b/drivers/xen/xen-pciback/xenbus.c
@@ -143,12 +143,20 @@ static int xen_pcibk_attach(struct xen_pcibk_device *pdev)
 	int err = 0;
 	int gnt_ref, remote_evtchn;
 	char *magic = NULL;
-
+	int state;
 
 	mutex_lock(&pdev->dev_lock);
+	state = xenbus_read_driver_state(pdev->xdev->nodename);
+	/* Frontend reconnected after closing. */
+	if (state == XenbusStateClosed) {
+		err = xenbus_switch_state(pdev->xdev, XenbusStateInitialized);
+		if (err)
+			xenbus_dev_fatal(pdev->xdev, err, "Error switching to initialized state!");
+		/* Re-read */
+		state = xenbus_read_driver_state(pdev->xdev->nodename);
+	}
 	/* Make sure we only do this setup once */
-	if (xenbus_read_driver_state(pdev->xdev->nodename) !=
-	    XenbusStateInitialised)
+	if (state != XenbusStateInitialised)
 		goto out;
 
 	/* Wait for frontend to state that it has published the configuration */
> 
> >> 
> >> If I do not perform step 3 then PV shutsdown after a 10s timeout.
> 
> > How does it shutdown? What causes the shutdown?
> 
> PV will exit, however xl does not release the domain for another 10s.

That looks to be another issue. Maybe it is already fixed by some
of the patches I posted (search for " xen/pciback: Don't deadlock when unbinding.").

> 
> -- 
> Best regards,
>  Simon                            mailto:furryfuttock@gmail.com
>