Re: [PATCH] xen/pv-on-hvm kexec: add xs_reset_watches to shutdown watches from old kernel

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

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 <olaf@aepfle.de>
> 
> ---
>  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