Re: [Xen-devel] [PATCH 11/11] xen/hvm kdump: reset PV devices in crash kernel

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2011-08-01 at 13:58 +0100, Olaf Hering wrote:
> On Fri, Jul 29, Stefano Stabellini wrote:
> 
> > On Thu, 28 Jul 2011, Olaf Hering wrote:
> > > (this is actually a forward port of rev 1079 from 2.6.18.hg, untested 
> > > because
> > > kdump just hangs before (or while) entering the crash kernel in 3.0)
> > > 
> > > After triggering a crash dump in a HVM guest, the PV backend drivers
> > > will remain in connected state. When the kdump kernel starts the PV
> > > drivers will skip such devices. As a result, no root device is found and
> > > the vmcore cant be saved.
> > > 
> > > With this change all frontend devices with state XenbusStateConnected
> > > will be reset by changing the state file to Closing/Closed/Initializing.
> > > This will trigger a disconnect in the backend drivers. Now the frontend
> > > drivers will find the backend drivers in state Initwait and can connect.
> > > 
> > > Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
> > > 
> > > ---
> > >  drivers/xen/xenbus/xenbus_comms.c          |    4 -
> > >  drivers/xen/xenbus/xenbus_probe_frontend.c |   97 
> > > +++++++++++++++++++++++++++++
> > >  2 files changed, 100 insertions(+), 1 deletion(-)
> > > 
> > > Index: linux-3.0/drivers/xen/xenbus/xenbus_comms.c
> > > ===================================================================
> > > --- linux-3.0.orig/drivers/xen/xenbus/xenbus_comms.c
> > > +++ linux-3.0/drivers/xen/xenbus/xenbus_comms.c
> > > @@ -212,7 +212,9 @@ int xb_init_comms(void)
> > >           printk(KERN_WARNING "XENBUS response ring is not quiescent "
> > >                  "(%08x:%08x): fixing up\n",
> > >                  intf->rsp_cons, intf->rsp_prod);
> > > -         intf->rsp_cons = intf->rsp_prod;
> > > +         /* breaks kdump */
> > > +         if (!reset_devices)
> > > +                 intf->rsp_cons = intf->rsp_prod;
> > >   }
> > 
> > Where is reset_devices coming from?
> > I hope is set only on a kexec reboot.
> 
> This is for a kdump boot, its added as additional kernel cmdline option
> for the kdump kernel.

It is commonly used with kdump but is it kdump specific?

I suppose this usage is not that far from the stated semantics of the
option ("Force drivers to reset the underlying device during
initialization" from Documentation/kernel-parameters.txt). But is there
any reason not to just always reset the rings on initialisation? There
can't be any real outstanding requests at this point and I think we
determined that xenstored should (and does) cope gracefully with this
(since hvmloader also does it).

> > Considering that all the other patches in this series follow the
> > opposite strategy, that is closing stuff on shutdown, why are you trying
> > to close xenbus connections on boot here?
> > At this point I would rather be coherent and simply switch to
> > XenbusStateClosing or XenbusStateClosed in the shutdown path if
> > kexec_is_loaded.

I'd prefer to be coherent too but in the other direction -- i.e. as much
of the work as possible should be pushed into the target kernel not done
in the source kernel. By definition you can't really rely on the source
kernel to be a) working at all (in the crash dump case) and b) not have
bugs on the shutdown path (at least the target kernel can have the fixes
applied before attempting to kexec etc).

> 
> To allow the kdump kernel to store the /proc/vmcore file somewhere, it
> has to get either storage or network access.  But the kdump kernel finds
> all devices in the Connected state, because the to-be-debugged kernel
> just crashed. So it has to reset the connection to the backend.
> 
> Olaf
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel