[PATCH 0/2] oxenstored quota handling fixes

[PATCH 0/2] oxenstored quota handling fixes

[Euan Harris]

Two fixes to oxenstored quota handling, on behalf of Jerome Maloberti
and Vincent Bernardoff.

Jerome Maloberti (1):
  oxenstored: perform a 3-way merge of the quota after a transaction

Vincent Bernardoff (1):
  oxenstored: exempt dom0 from domU node quotas

 tools/ocaml/xenstored/quota.ml       |    5 +++++
 tools/ocaml/xenstored/store.ml       |   19 ++++++++-----------
 tools/ocaml/xenstored/transaction.ml |    4 +++-
 3 files changed, 16 insertions(+), 12 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] oxenstored: exempt dom0 from domU node quotas

[Euan Harris]

From: Vincent Bernardoff <vincent.bernardoff@citrix.com>

If a domU has exhausted its quota we still want the toolstack in dom0 to
be able to create new nodes in places like
  /local/domain/%d/control/shutdown

Without this patch, a domU which has exhausted its quota can only be
powered off, which is not as good as being able to request a clean
shutdown.

Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
Signed-off-by: Euan Harris <euan.harris@citrix.com>
---
 tools/ocaml/xenstored/store.ml |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/ocaml/xenstored/store.ml b/tools/ocaml/xenstored/store.ml
index cac0b44..3efe515 100644
--- a/tools/ocaml/xenstored/store.ml
+++ b/tools/ocaml/xenstored/store.ml
@@ -383,7 +383,7 @@ let set_node store path node =
 let write store perm path value =
 	let node, existing = get_deepest_existing_node store path in
 	let owner = Node.get_owner node in
-	if existing then
+	if existing || (Perms.Connection.is_dom0 perm) then
 		(* Only check the string length limit *)
 		Quota.check store.quota (-1) (String.length value)
 	else
@@ -398,7 +398,7 @@ let mkdir store perm path =
 	let node, existing = get_deepest_existing_node store path in
 	let owner = Node.get_owner node in
 	(* It's upt to the mkdir logic to decide what to do with existing path *)
-	if not existing then Quota.check store.quota owner 0;
+	if not (existing || (Perms.Connection.is_dom0 perm)) then Quota.check store.quota owner 0;
 	store.root <- path_mkdir store perm path;
 	Quota.add_entry store.quota owner
 
@@ -416,7 +416,7 @@ let setperms store perm path nperms =
 	| Some node ->
 		let old_owner = Node.get_owner node in
 		let new_owner = Perms.Node.get_owner nperms in
-		if old_owner <> new_owner then Quota.check store.quota new_owner 0;
+		if not ((old_owner = new_owner) || (Perms.Connection.is_dom0 perm)) then Quota.check store.quota new_owner 0;
 		store.root <- path_setperms store perm path nperms;
 		Quota.del_entry store.quota old_owner;
 		Quota.add_entry store.quota new_owner
-- 
1.7.9.5

[PATCH 2/2] oxenstored: perform a 3-way merge of the quota after a transaction

[Euan Harris]

From: Jerome Maloberti <jerome.maloberti@citrix.com>

At a beginning of a transaction, the quotas from the global store
are duplicated and modified by the transaction. If during the
transaction, an action associated to no transaction is concurrently
executed, the quotas of the global store are updated, and then the
updates are lost when the transaction merges.

We fix this problem by keeping another copy of the quota at the
beginning of the transaction, and performing a 3-way merge between
the quotas from the transaction and the "original" copy of the quota
onto the quota of the global store.

Signed-off-by: Jerome Maloberti <jerome.maloberti@citrix.com>
Signed-off-by: Euan Harris <euan.harris@citrix.com>
---
 tools/ocaml/xenstored/quota.ml       |    5 +++++
 tools/ocaml/xenstored/store.ml       |   13 +++++--------
 tools/ocaml/xenstored/transaction.ml |    4 +++-
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/tools/ocaml/xenstored/quota.ml b/tools/ocaml/xenstored/quota.ml
index c668302..e6953c6 100644
--- a/tools/ocaml/xenstored/quota.ml
+++ b/tools/ocaml/xenstored/quota.ml
@@ -81,3 +81,8 @@ let add_entry quota id =
 
 let add quota diff =
 	Hashtbl.iter (fun id nb -> set_entry quota id (get_entry quota id + nb)) diff.cur
+
+let merge orig_quota mod_quota dest_quota =
+	  Hashtbl.iter (fun id nb -> let diff = nb - (get_entry orig_quota id) in
+				if diff <> 0 then
+					set_entry dest_quota id ((get_entry dest_quota id) + diff)) mod_quota.cur
diff --git a/tools/ocaml/xenstored/store.ml b/tools/ocaml/xenstored/store.ml
index 3efe515..223ee21 100644
--- a/tools/ocaml/xenstored/store.ml
+++ b/tools/ocaml/xenstored/store.ml
@@ -188,20 +188,17 @@ let rec get_deepest_existing_node node = function
 		with Not_found -> node, false
 
 let set_node rnode path nnode =
-	let quota = Quota.create () in
-	if !Quota.activate then Node.recurse (fun node -> Quota.add_entry quota (Node.get_owner node)) nnode;
 	if path = [] then
-		nnode, quota
+		nnode
 	else
 		let set_node node name =
 			try
 				let ent = Node.find node name in
-				if !Quota.activate then Node.recurse (fun node -> Quota.del_entry quota (Node.get_owner node)) ent;
 				Node.replace_child node ent nnode
 			with Not_found ->
 				Node.add_child node nnode
 			in
-		apply_modify rnode path set_node, quota
+		apply_modify rnode path set_node
 
 (* read | ls | getperms use this *)
 let rec lookup node path fct =
@@ -375,10 +372,10 @@ let dump_buffer store = dump_store_buf store.root
 
 
 (* modifying functions with quota udpate *)
-let set_node store path node =
-	let root, quota_diff = Path.set_node store.root path node in
+let set_node store path node orig_quota mod_quota =
+	let root = Path.set_node store.root path node in
 	store.root <- root;
-	Quota.add store.quota quota_diff
+	Quota.merge orig_quota mod_quota store.quota
 
 let write store perm path value =
 	let node, existing = get_deepest_existing_node store path in
diff --git a/tools/ocaml/xenstored/transaction.ml b/tools/ocaml/xenstored/transaction.ml
index e59d681..77de4e8 100644
--- a/tools/ocaml/xenstored/transaction.ml
+++ b/tools/ocaml/xenstored/transaction.ml
@@ -74,6 +74,7 @@ type ty = No | Full of (int * Store.Node.t * Store.t)
 type t = {
 	ty: ty;
 	store: Store.t;
+	quota: Quota.t;
 	mutable ops: (Xenbus.Xb.Op.operation * Store.Path.t) list;
 	mutable read_lowpath: Store.Path.t option;
 	mutable write_lowpath: Store.Path.t option;
@@ -84,6 +85,7 @@ let make id store =
 	{
 		ty = ty;
 		store = if id = none then store else Store.copy store;
+		quota = Quota.copy store.Store.quota;
 		ops = [];
 		read_lowpath = None;
 		write_lowpath = None;
@@ -155,7 +157,7 @@ let commit ~con t =
 
 					(* it has to be in the store, otherwise it means bugs
 					   in the lowpath registration. we don't need to handle none. *)
-					maybe (fun n -> Store.set_node cstore p n) n;
+					maybe (fun n -> Store.set_node cstore p n t.quota store.Store.quota) n;
 					Logging.write_coalesce ~tid:(get_id t) ~con (Store.Path.to_string p);
 				) t.write_lowpath;
 				maybe (fun p ->
-- 
1.7.9.5

Re: [PATCH 0/2] oxenstored quota handling fixes

[Dave Scott]

Hi Euan,

On 3 Jul 2014, at 15:02, Euan Harris <euan.harris@citrix.com> wrote:

> Two fixes to oxenstored quota handling, on behalf of Jerome Maloberti
> and Vincent Bernardoff.
> 
> Jerome Maloberti (1):
>  oxenstored: perform a 3-way merge of the quota after a transaction
> 
> Vincent Bernardoff (1):
>  oxenstored: exempt dom0 from domU node quotas

These both look fine to me. It’s not great to have to exempt dom0 from the system (it seems a bit arbitrary) but it’s probably the best we can do without a bigger redesign.

Acked-by: David Scott <dave.scott@citrix.com>

Cheers,
Dave

> 
> tools/ocaml/xenstored/quota.ml       |    5 +++++
> tools/ocaml/xenstored/store.ml       |   19 ++++++++-----------
> tools/ocaml/xenstored/transaction.ml |    4 +++-
> 3 files changed, 16 insertions(+), 12 deletions(-)
> 
> -- 
> 1.7.9.5
> 

Re: [PATCH 0/2] oxenstored quota handling fixes

[Ian Campbell]

On Thu, 2014-07-03 at 15:11 +0000, Dave Scott wrote:
> Hi Euan,
> 
> On 3 Jul 2014, at 15:02, Euan Harris <euan.harris@citrix.com> wrote:
> 
> > Two fixes to oxenstored quota handling, on behalf of Jerome Maloberti
> > and Vincent Bernardoff.
> > 
> > Jerome Maloberti (1):
> >  oxenstored: perform a 3-way merge of the quota after a transaction
> > 
> > Vincent Bernardoff (1):
> >  oxenstored: exempt dom0 from domU node quotas
> 
> These both look fine to me. It’s not great to have to exempt dom0 from
> the system (it seems a bit arbitrary) but it’s probably the best we
> can do without a bigger redesign.

Speaking of bigger redesigns -- how's that new version of oxenstored
looking?

> Acked-by: David Scott <dave.scott@citrix.com>

Applied, thanks.

> 
> Cheers,
> Dave
> 
> > 
> > tools/ocaml/xenstored/quota.ml       |    5 +++++
> > tools/ocaml/xenstored/store.ml       |   19 ++++++++-----------
> > tools/ocaml/xenstored/transaction.ml |    4 +++-
> > 3 files changed, 16 insertions(+), 12 deletions(-)
> > 
> > -- 
> > 1.7.9.5
> > 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel



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