Re: [Xen-devel] [PATCH RFC 0/4] 'reset everything' approach to PVHVM guest kexec

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Jun 08, 2015 at 04:53:46PM +0100, Wei Liu wrote:
> On Wed, Jun 03, 2015 at 03:35:18PM +0200, Vitaly Kuznetsov wrote:
> > Jan and Tim,
> > 
> > last week you expressed some concerns about if the toolstack-based
> > approach to PVHVM guest kexec is the best. Here you can see the 'reset
> > everything' approach to the same problem. It is the bare minimum of what
> > should be done to make it possible for the new kernel to boot. Despite
> > the fact that this implementation is much smaller in size and that it
> > doesn't require any toolstack changes I'm personally in favor of the
> > previous toolstack-based approach as it looks more general, less fragile
> > and easier to support to me. Please let me know your thoughts.
> > 
> > I used SCHEDOP_ interface here as DOMCTL_* is not currently supported in
> > Linux kernel and I seriously doubt we need to support something different
> > from DOMID_SELF for soft reset.
> > 
> > Current Linux kernel requires two changes to make use of these hypervisor
> > changes:
> > 1) As XS_RESET_WATCHES is not supported by oxenstored we need to try 
> > removing
> > the watch in case add operation failed, e.g.:
> 
> The changeset to implement XS_RESET_WATCHES in cxenstored is
> 1f9d04fb021cbf35cc420d401a88c696d6524c14
> 
> It doesn't look too complicated to do that in oxenstored. Dave
> (oxenstored maintainer, CC'ed) might have insight.
> 
> Wei.
> 

And I took a stab at it. Here is my oxenstored patch. May contain
bugs. :-)

---8<---
From 8dd35370442340493f856b0be8d99c87243e79f6 Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@xxxxxxxxxx>
Date: Mon, 8 Jun 2015 18:39:45 +0100
Subject: [PATCH] oxenstored: implement XS_RESET_WATCHES

Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
---
 tools/ocaml/libs/xb/op.ml           | 6 ++++--
 tools/ocaml/libs/xb/xb.mli          | 1 +
 tools/ocaml/xenstored/connection.ml | 8 ++++++++
 tools/ocaml/xenstored/process.ml    | 6 ++++++
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/tools/ocaml/libs/xb/op.ml b/tools/ocaml/libs/xb/op.ml
index 0ee8666..69346d8 100644
--- a/tools/ocaml/libs/xb/op.ml
+++ b/tools/ocaml/libs/xb/op.ml
@@ -19,7 +19,8 @@ type operation = Debug | Directory | Read | Getperms |
                  Transaction_end | Introduce | Release |
                  Getdomainpath | Write | Mkdir | Rm |
                  Setperms | Watchevent | Error | Isintroduced |
-                 Resume | Set_target | Restrict | Invalid
+                 Resume | Set_target | Restrict | Reset_watches |
+                 Invalid
 
 let operation_c_mapping =
        [| Debug; Directory; Read; Getperms;
@@ -27,7 +28,7 @@ let operation_c_mapping =
            Transaction_end; Introduce; Release;
            Getdomainpath; Write; Mkdir; Rm;
            Setperms; Watchevent; Error; Isintroduced;
-           Resume; Set_target; Restrict |]
+           Resume; Set_target; Restrict; Reset_watches |]
 let size = Array.length operation_c_mapping
 
 let array_search el a =
@@ -68,4 +69,5 @@ let to_string ty =
        | Resume                -> "RESUME"
        | Set_target            -> "SET_TARGET"
        | Restrict              -> "RESTRICT"
+       | Reset_watches         -> "RESET_WATCHES"
        | Invalid               -> "INVALID"
diff --git a/tools/ocaml/libs/xb/xb.mli b/tools/ocaml/libs/xb/xb.mli
index 4e1f833..6c242da 100644
--- a/tools/ocaml/libs/xb/xb.mli
+++ b/tools/ocaml/libs/xb/xb.mli
@@ -23,6 +23,7 @@ module Op :
       | Resume
       | Set_target
       | Restrict
+      | Reset_watches
       | Invalid
     val operation_c_mapping : operation array
     val size : int
diff --git a/tools/ocaml/xenstored/connection.ml 
b/tools/ocaml/xenstored/connection.ml
index b4dc9cb..5e31588 100644
--- a/tools/ocaml/xenstored/connection.ml
+++ b/tools/ocaml/xenstored/connection.ml
@@ -186,6 +186,14 @@ let del_watch con path token =
        con.nb_watches <- con.nb_watches - 1;
        apath, w
 
+let del_watches con =
+  Hashtbl.clear con.watches;
+  con.next_tid <- initial_next_tid
+
+let del_transactions con =
+  Hashtbl.clear con.transactions;
+  con.nb_watches <- 0
+
 let list_watches con =
        let ll = Hashtbl.fold 
                (fun _ watches acc -> List.map (fun watch -> watch.path, 
watch.token) watches :: acc)
diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml
index 0620585..e827678 100644
--- a/tools/ocaml/xenstored/process.ml
+++ b/tools/ocaml/xenstored/process.ml
@@ -272,6 +272,11 @@ let do_restrict con t domains cons data =
        in
        Connection.restrict con domid
 
+(* only in xen >= 4.2 *)
+let do_reset_watches con t domains cons data =
+  Connection.del_watches con;
+  Connection.del_transactions con
+
 (* only in >= xen3.3                                                           
                         *)
 (* we ensure backward compatibility with restrict by counting the number of 
argument of set_target ...  *)
 (* This is not very elegant, but it is safe as 'restrict' only restricts 
permission of dom0 connections *)
@@ -324,6 +329,7 @@ let function_of_type ty =
        | Xenbus.Xb.Op.Resume            -> reply_ack do_resume
        | Xenbus.Xb.Op.Set_target        -> reply_ack do_set_target
        | Xenbus.Xb.Op.Restrict          -> reply_ack do_restrict
+       | Xenbus.Xb.Op.Reset_watches     -> reply_ack do_reset_watches
        | Xenbus.Xb.Op.Invalid           -> reply_ack do_error
        | _                              -> reply_ack do_error
 
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel