From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH] xen/pv-on-hvm kexec: add xs_reset_watches to shutdown watches from old kernel Date: Mon, 15 Aug 2011 08:53:06 -0400 Message-ID: <20110815125306.GA11127@dumpdata.com> References: <4b483f4fa715566847b5.1313397256@probook.site> <20110815092530.GA3001@aepfle.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20110815092530.GA3001@aepfle.de> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Olaf Hering Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On Mon, Aug 15, 2011 at 11:25:30AM +0200, Olaf Hering wrote: > Add new xs_reset_watches function to shutdown watches from old kernel after > kexec boot. The old kernel does not unregister all watches in the > shutdown path. They are still active, the double registration can not > be detected by the new kernel. When the watches fire, unexpected events > will arrive and the xenwatch thread will crash (jumps to NULL). An > orderly reboot of a hvm guest will destroy the entire guest with all its > resources (including the watches) before it is rebuilt from scratch, so > the missing unregister is not an issue in that case. So this patch replaces the big patch series you sent some while ago? [I've one of your patches in my tree, but I wasn't sure about the other ones] > > With this change the xenstored is instructed to wipe all active watches > for the guest. However, a patch for xenstored is required so that it > accepts the XS_RESET_WATCHES request from a guest. Ok, make sure to note the c/s when that is accepted. > > v3: > - use XS_RESET_WATCHES instead of XS_INTRODUCE > > v2: > - move all code which deals with XS_INTRODUCE into xs_introduce() > (based on feedback from Ian Campbell) > - remove casts from kvec assignment > > Signed-off-by: Olaf Hering > > --- > drivers/xen/xenbus/xenbus_xs.c | 20 ++++++++++++++++++++ > include/xen/interface/io/xs_wire.h | 6 +++++- > 2 files changed, 25 insertions(+), 1 deletion(-) > > Index: linux-3.0/drivers/xen/xenbus/xenbus_xs.c > =================================================================== > --- linux-3.0.orig/drivers/xen/xenbus/xenbus_xs.c > +++ linux-3.0/drivers/xen/xenbus/xenbus_xs.c > @@ -620,6 +620,22 @@ static struct xenbus_watch *find_watch(c > return NULL; > } > > +static void xs_reset_watches(void) > +{ > + /* dummy, empty iov not handled by xenstored */ > + struct kvec iov[1]; > + int err; > + > + > + iov[0].iov_base = "dummy"; > + iov[0].iov_len = strlen(iov[0].iov_base) + 1; > + > + err = xs_error(xs_talkv(XBT_NIL, XS_RESET_WATCHES, iov, > + ARRAY_SIZE(iov), NULL)); > + if (err) > + printk(KERN_WARNING "xs_reset_watches failed: %d\n", err); If the xenstore does not have the patch for this, what is the error code? Is it ENOSYS? If we get that can we not print this message? Or perhaps print: "Yikes! We can't reset the watches. Potential crash immienient" or something similar. > +} > + > /* Register callback to watch this node. */ > int register_xenbus_watch(struct xenbus_watch *watch) > { > @@ -897,5 +913,9 @@ int xs_init(void) > if (IS_ERR(task)) > return PTR_ERR(task); > > + /* shutdown watches for kexec boot */ > + if (xen_hvm_domain()) > + xs_reset_watches(); > + > return 0; > } > Index: linux-3.0/include/xen/interface/io/xs_wire.h > =================================================================== > --- linux-3.0.orig/include/xen/interface/io/xs_wire.h > +++ linux-3.0/include/xen/interface/io/xs_wire.h > @@ -26,7 +26,11 @@ enum xsd_sockmsg_type > XS_SET_PERMS, > XS_WATCH_EVENT, > XS_ERROR, > - XS_IS_DOMAIN_INTRODUCED > + XS_IS_DOMAIN_INTRODUCED, > + XS_RESUME, > + XS_SET_TARGET, > + XS_RESTRICT, > + XS_RESET_WATCHES > }; > > #define XS_WRITE_NONE "NONE" > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel