[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC 0/4] 'reset everything' approach to PVHVM guest kexec
On Tue, Jun 09, 2015 at 09:19:19AM +0100, Dave Scott wrote: > > > On 8 Jun 2015, at 18:43, Wei Liu <wei.liu2@xxxxxxxxxx> wrote: > > > > 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. :-) > > That was quick :-) > > I have only one question, see below: > > > > > ---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 > > I couldnât spot the part in the C version where the next transaction id is > reset â is this a (minor) difference between the two implementations, or have > I misread the code? > > If you could clarify that for me, and assuming this all compiled ok, then > feel free to add > Yes, it compiles OK. I think I will just delete that line. I couldn't recollect why I added that. Maybe it's copy-n-paste error. Can I still have your ack if I drop that line? > Acked-by: David Scott <dave.scott@xxxxxxxxxx> > > Cheers, > Dave _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |