[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 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 Acked-by: David Scott <dave.scott@xxxxxxxxxx> Cheers, Dave > + > +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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |