Xen devel - Feb 2008 - [1/3] Domgrps/SchedGrps Merge RFC: domgrps-vmm

diffstat domgrps-vmm.patch
  arch/ia64/xen/xensetup.c                 |    7
  arch/powerpc/powerpc64/hypercall_table.S |    1
  arch/powerpc/setup.c                     |    7
  arch/x86/setup.c                         |    8
  arch/x86/x86_32/entry.S                  |    2
  arch/x86/x86_64/entry.S                  |    3
  common/Makefile                          |    2
  common/domain.c                          |    8
  common/domctl.c                          |   13 +
  common/domgrp.c                          |  320 ++++++++++++++++++++ 
+++++++++++
  common/domgrpctl.c                       |  134 ++++++++++++
  common/sysctl.c                          |    1
  include/public/domctl.h                  |    2
  include/public/domgrpctl.h               |   86 ++++++++
  include/public/xen.h                     |    5
  include/xen/compile.h                    |   39 +++
  include/xen/domgrp.h                     |   36 +++
  include/xen/hypercall.h                  |    5
  include/xen/sched.h                      |   21 ++
  19 files changed, 697 insertions(+), 3 deletions(-)



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

Hi, Chris

Please write summaries of each patches.
And can you wirte usage of domgrps/schedgrps?
My comments are below.

+long do_domgrpctl(XEN_GUEST_HANDLE(xen_domgrpctl_t) u_domgrpctl)
+{
+	long ret = 0;
+	struct xen_domgrpctl curop, *op = &curop;
+	static DEFINE_SPINLOCK(domgrpctl_lock);
+	struct domain_group *grp;
+	dgid_t dgid;
+
+	if (!IS_PRIV(current->domain)) {
+		ret = -EPERM;
+		goto out;
+	}
+
+	if (copy_from_guest(op, u_domgrpctl, 1)) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	if (op->interface_version != XEN_DOMGRPCTL_INTERFACE_VERSION) {
+		ret = -EINVAL;
I think "ret = -EACESS" is right.

diff -urN xen-unstable/xen/arch/x86/setup.c
xen-unstable-domgrps/xen/arch/x86/setup.c
--- xen-unstable/xen/arch/x86/setup.c	2008-01-29 12:44:24.000000000 -0500
+++ xen-unstable-domgrps/xen/arch/x86/setup.c	2008-01-29 12:53:23.000000000
-0500
@@ -3,6 +3,7 @@
 #include <xen/lib.h>
 #include <xen/sched.h>
 #include <xen/domain.h>
+#include <xen/domgrp.h>
 #include <xen/serial.h>
 #include <xen/softirq.h>
 #include <xen/acpi.h>
@@ -969,6 +970,10 @@
     if ( opt_watchdog ) 
         watchdog_enable();
 
+    /* initialize domain groups */
+    if (init_domain_groups())
+        panic("Error creating default groups\n");
+
     /* Create initial domain 0. */
     dom0 = domain_create(0, 0, DOM0_SSIDREF);
     if ( (dom0 == NULL) || (alloc_vcpu(dom0, 0, 0) == NULL) )
@@ -977,6 +982,9 @@
     dom0->is_privileged = 1;
     dom0->target = NULL;
 
+    if (add_dom_to_grp(dom0, 0))
+        panic("Error adding dom0 to grp0\n");
+
     /* Grab the DOM0 command line. */
     cmdline = (char *)(mod[0].string ? __va(mod[0].string) : NULL);
     if ( (cmdline != NULL) || (kextra != NULL) )
You change this part in the second patch.

diff -urN xen-unstable/xen/arch/x86/x86_64/entry.S
xen-unstable-domgrps/xen/arch/x86/x86_64/entry.S
--- xen-unstable/xen/arch/x86/x86_64/entry.S	2007-11-19 10:38:08.000000000 -0500
+++ xen-unstable-domgrps/xen/arch/x86/x86_64/entry.S	2007-11-19
18:43:06.000000000 -0500
@@ -672,6 +672,7 @@
         .quad do_sysctl             /* 35 */
         .quad do_domctl
         .quad do_kexec_op
+        .quad do_domgrpctl
         .rept NR_hypercalls-((.-hypercall_table)/8)
         .quad do_ni_hypercall
         .endr
@@ -715,7 +716,7 @@
         .byte 1 /* do_sysctl            */  /* 35 */
         .byte 1 /* do_domctl            */
         .byte 2 /* do_kexec             */
-        .byte 1 /* do_xsm_op            */
+        .byte 1 /* do_domgrpctl         */
         .rept NR_hypercalls-(.-hypercall_args_table)
         .byte 0 /* do_ni_hypercall      */
         .endr
Why do you remove do_xsm_op?

diff -urN xen-unstable/xen/include/xen/compile.h
xen-unstable-domgrps/xen/include/xen/compile.h
--- xen-unstable/xen/include/xen/compile.h	1969-12-31 19:00:00.000000000 -0500
+++ xen-unstable-domgrps/xen/include/xen/compile.h	2008-01-23 14:58:34.000000000
-0500
Please don''t inlcude this file.

Best Regards,

Akio Takebe
>diffstat domgrps-vmm.patch
>  arch/ia64/xen/xensetup.c                 |    7
>  arch/powerpc/powerpc64/hypercall_table.S |    1
>  arch/powerpc/setup.c                     |    7
>  arch/x86/setup.c                         |    8
>  arch/x86/x86_32/entry.S                  |    2
>  arch/x86/x86_64/entry.S                  |    3
>  common/Makefile                          |    2
>  common/domain.c                          |    8
>  common/domctl.c                          |   13 +
>  common/domgrp.c                          |  320 ++++++++++++++++++++++++++
>+++++
>  common/domgrpctl.c                       |  134 ++++++++++++
>  common/sysctl.c                          |    1
>  include/public/domctl.h                  |    2
>  include/public/domgrpctl.h               |   86 ++++++++
>  include/public/xen.h                     |    5
>  include/xen/compile.h                    |   39 +++
>  include/xen/domgrp.h                     |   36 +++
>  include/xen/hypercall.h                  |    5
>  include/xen/sched.h                      |   21 ++
>  19 files changed, 697 insertions(+), 3 deletions(-)
>
>
>-------------------------------text/plain-------------------------------
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel

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

On Feb 6, 2008, at 7:00 AM, Akio Takebe wrote:
> +long do_domgrpctl(XEN_GUEST_HANDLE(xen_domgrpctl_t) u_domgrpctl)
> +{
> +	long ret = 0;
> +	struct xen_domgrpctl curop, *op = &curop;
> +	static DEFINE_SPINLOCK(domgrpctl_lock);
> +	struct domain_group *grp;
> +	dgid_t dgid;
> +
> +	if (!IS_PRIV(current->domain)) {
> +		ret = -EPERM;
> +		goto out;
> +	}
> +
> +	if (copy_from_guest(op, u_domgrpctl, 1)) {
> +		ret = -EFAULT;
> +		goto out;
> +	}
> +
> +	if (op->interface_version != XEN_DOMGRPCTL_INTERFACE_VERSION) {
> +		ret = -EINVAL;
> I think "ret = -EACESS" is right.
Ack
> diff -urN xen-unstable/xen/arch/x86/setup.c xen-unstable-domgrps/ 
> xen/arch/x86/setup.c
> --- xen-unstable/xen/arch/x86/setup.c	2008-01-29 12:44:24.000000000  
> -0500
> +++ xen-unstable-domgrps/xen/arch/x86/setup.c	2008-01-29  
> 12:53:23.000000000 -0500
> @@ -3,6 +3,7 @@
>  #include <xen/lib.h>
>  #include <xen/sched.h>
>  #include <xen/domain.h>
> +#include <xen/domgrp.h>
>  #include <xen/serial.h>
>  #include <xen/softirq.h>
>  #include <xen/acpi.h>
> @@ -969,6 +970,10 @@
>      if ( opt_watchdog )
>          watchdog_enable();
>
> +    /* initialize domain groups */
> +    if (init_domain_groups())
> +        panic("Error creating default groups\n");
> +
>      /* Create initial domain 0. */
>      dom0 = domain_create(0, 0, DOM0_SSIDREF);
>      if ( (dom0 == NULL) || (alloc_vcpu(dom0, 0, 0) == NULL) )
> @@ -977,6 +982,9 @@
>      dom0->is_privileged = 1;
>      dom0->target = NULL;
>
> +    if (add_dom_to_grp(dom0, 0))
> +        panic("Error adding dom0 to grp0\n");
> +
>      /* Grab the DOM0 command line. */
>      cmdline = (char *)(mod[0].string ? __va(mod[0].string) : NULL);
>      if ( (cmdline != NULL) || (kextra != NULL) )
> You change this part in the second patch
You''re right.  There are in fact several places where the second  
(schedgrps) patch goes back and changes the first (domgrps), which is  
confusing.  It got that way b/c merging with schedgrps identified  
areas where changes to domgrps where needed.  I''ll try to separate  
patches more cleanly in the future.
> diff -urN xen-unstable/xen/arch/x86/x86_64/entry.S xen-unstable- 
> domgrps/xen/arch/x86/x86_64/entry.S
> --- xen-unstable/xen/arch/x86/x86_64/entry.S	2007-11-19  
> 10:38:08.000000000 -0500
> +++ xen-unstable-domgrps/xen/arch/x86/x86_64/entry.S	2007-11-19  
> 18:43:06.000000000 -0500
> @@ -672,6 +672,7 @@
>          .quad do_sysctl             /* 35 */
>          .quad do_domctl
>          .quad do_kexec_op
> +        .quad do_domgrpctl
>          .rept NR_hypercalls-((.-hypercall_table)/8)
>          .quad do_ni_hypercall
>          .endr
> @@ -715,7 +716,7 @@
>          .byte 1 /* do_sysctl            */  /* 35 */
>          .byte 1 /* do_domctl            */
>          .byte 2 /* do_kexec             */
> -        .byte 1 /* do_xsm_op            */
> +        .byte 1 /* do_domgrpctl         */
>          .rept NR_hypercalls-(.-hypercall_args_table)
>          .byte 0 /* do_ni_hypercall      */
>          .endr
> Why do you remove do_xsm_op?
It''s actually not removing xsm_op.  There''s a redundant op  
declaration in the x86_64 hyercall table.  A bit higher in the file  
(it''s op 27 for x86_64), xsm_op is declared where it replaced  
acm_op.  The latter declaration was unused, so I swapped it out for  
domgrpctl.  This fix probably should''ve been submitted as a separate  
patch to avoid confusion.
> diff -urN xen-unstable/xen/include/xen/compile.h xen-unstable- 
> domgrps/xen/include/xen/compile.h
> --- xen-unstable/xen/include/xen/compile.h	1969-12-31  
> 19:00:00.000000000 -0500
> +++ xen-unstable-domgrps/xen/include/xen/compile.h	2008-01-23  
> 14:58:34.000000000 -0500
> Please don''t inlcude this file.
Thanks for catching my mistake, that file shouldn''t be included. 
  

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

Sorry for the lateness of this review. My opinion is that sched groups
should (as implemented for credit sharing to support stub domains)
should go upstream for 3.3 without being merged with domain groups. 

I think domain groups are more powerful and will eventually assimilate
the focused scheduling group patchset. At the same time, more thought
needs to go into domain groups. To that end, here are a few of my
belated thoughts and questions.



On 04/02/08 14:14 -0500, Chris wrote:
>diff -urN xen-unstable/xen/arch/powerpc/powerpc64/hypercall_table.S
xen-unstable-domgrps/xen/arch/powerpc/powerpc64/hypercall_table.S
>--- xen-unstable/xen/arch/powerpc/powerpc64/hypercall_table.S	2007-08-06
17:59:54.000000000 -0400
>+++ xen-unstable-domgrps/xen/arch/powerpc/powerpc64/hypercall_table.S
2007-11-19 18:42:00.000000000 -0500
>@@ -41,6 +41,7 @@
>         .quad 0 /* do_hvm_op */
>         .quad do_sysctl             /* 35 */
>         .quad do_domctl
>+        .quad do_domgrpctl
>         .rept NR_hypercalls-((.-__hypercall_table)/8)
>         .quad do_ni_hypercall
>         .endr
Rather than creating a new hypercall, it may be best to implement all
the grouping and grouping control as part of a sub-command of the
domctrl hypercall. Then we don''t need to add this extra hypercall into
the table of every platform.


>+
>+    if (d->group)
>+        grp = d->group;
>+    else
>+        grp = find_grp_by_id(NULL_GROUP_ID);
>+
Wondering why we have to search for the null group. Why can''t the null
group be statically initialized and then always referred to by a
pointer? In fact, what is the role of the null group? I see that it is
a real domain group and it is created automatically. Also it can''t be
created by a hypercall.

All domains that do not belong to a group actually belong to the null
group. Could it also be just as easy for each domain to have an
identity group (itself), to which it belongs in the absence of any
other group membership? Or is there a specific purpose to the null
group?

>+#define SERIALIZE_LINKED_LIST(op_name, list_name)		\
>+    rcu_read_lock(&grp->op_name##_read_lock);			\
>+    memset(&info->op_name, 0, sizeof(domid_t)*MAX_GROUP_SIZE);	\
>+    if (!list_empty(&grp->op_name)) {				\
>+        i = 0;							\
>+        list_for_each_entry(dom, &grp->op_name, list_name) {	\
>+            info->op_name[i] = dom->domain_id;			\
>+       	    i++;						\
>+	}							\
>+    } else 							\
>+	info->op_name[i] = NULL_GROUP_ID;			\
>+    rcu_read_unlock(&grp->op_name##_read_lock);

This should really be cleaned up and made into an inline function. It
also refers to a variable declared outside of the macro and parameters
(info). It''s not clear what''s happening inside the macro.

>+#define RM_DOM_FROM_LIST(list_name, entry)	\
>+    spin_lock(&grp->list_name##_update_lock);	\
>+    if (!list_empty(&grp->list_name)) 		\
>+        list_del(&dom->entry);		    	\
>+    spin_unlock(&grp->list_name##_update_lock);
>+
Another good candidate for inlining.
>+int add_dom_to_grp(struct domain *dom, dgid_t dgid)
>+{
...
>+
>+	/* skip it if dom is already a member */
>+	if (dom_in_member_list(dom->domain_id, grp))
>+		goto out;
>+	
>+	/* remove dom from old group */
>+	if (dom->group != NULL)
>+		del_dom_from_grp(dom);

So a domain can only belong to one group at a time. Would it ever be
useful for a domain to belong to more than one group? This type of
restriction seems to work against the idea of a general grouping
concept. For example, all domains that belong to a scheduling group
(assuming we eventually merge domgrp and schedgrp) would automatically
be removed from a scheduling group if added to any other type of
group.

This argues for keeping different types of groups under different
grouping infrastructures unless we can figure out a way for a domain
to have multiple group memberships. It may be too complicated to do so.

>+int pause_grp(dgid_t dgid)
>+{
>+	int ret = 0;
>+	struct domain *member;
>+	struct domain_group *grp;
>+
>+	if (dgid == 0) {
>+		ret = -EPERM;
>+		goto out;
>+	}
>+
>+	grp = find_grp_by_id(dgid);
>+
>+	if (grp == NULL) {
>+		ret = -ESRCH;
>+		goto out;
>+	}
>+
>+	spin_lock_recursive(&grp->grp_big_lock);
>+	rcu_read_lock(&grp->member_list_read_lock);
>+	/* could ignore interupts during this loop to increase atomicity */
>+	list_for_each_entry(member, &grp->member_list, member) {
>+		if (member != current->domain && member->domain_id != 0)
>+			domain_pause_by_systemcontroller(member);
>+	}
>+	rcu_read_unlock(&grp->member_list_read_lock);
>+	spin_unlock_recursive(&grp->grp_big_lock);
>+      out:
>+	return ret;
>+}
>+

Groups are paused in the order in which they are added to the group,
right? (FIFO). What do we do when we have groups which are dependant
upon each other in certain ways. 

For example, the stub domain and HVM domain. At first glance, it seems
that you would want to pause the hvm domain before pausing its stub
domain. And then the reverse on resume: unpause the stub domain, then
unpause the hvm domain. (I could have it backward). But the point is,
group operations may have inter-domain dependencies that are not
accounted for simply by the order in which the domains are added to a
group.  This is true for pause, unpause, start, migrate, etc., on down
the line of possible group operations.

Mike

-- 
Mike D. Day
IBM LTC
Cell: 919 412-3900
Sametime: ncmike@us.ibm.com AIM: ncmikeday  Yahoo: ultra.runner
PGP key: http://www.ncultra.org/ncmike/pubkey.asc

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

Mike D. Day, le Mon 11 Feb 2008 17:07:38 -0500, a écrit
:
> So a domain can only belong to one group at a time. Would it ever be
> useful for a domain to belong to more than one group? This type of
> restriction seems to work against the idea of a general grouping
> concept.
Right.
> For example, all domains that belong to a scheduling group
> (assuming we eventually merge domgrp and schedgrp) would automatically
> be removed from a scheduling group if added to any other type of
> group.
Which makes sense, doesn''t it?
> This argues for keeping different types of groups under different
> grouping infrastructures unless we can figure out a way for a domain
> to have multiple group memberships. It may be too complicated to do so.
Yes, usually group mecanisms are just hierarchical rather than being so
generic as to permit several membership.  I guess the common pitfall is
when enumerating groups, you may see a domain several times, and such
strange effects.

Samuel

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

On Feb 11, 2008, at 5:07 PM, Mike D. Day wrote:
>> diff -urN xen-unstable/xen/arch/powerpc/powerpc64/ 
>> hypercall_table.S xen-unstable-domgrps/xen/arch/powerpc/powerpc64/ 
>> hypercall_table.S
>> --- xen-unstable/xen/arch/powerpc/powerpc64/hypercall_table.S	 
>> 2007-08-06 17:59:54.000000000 -0400
>> +++ xen-unstable-domgrps/xen/arch/powerpc/powerpc64/ 
>> hypercall_table.S	2007-11-19 18:42:00.000000000 -0500
>> @@ -41,6 +41,7 @@
>>         .quad 0 /* do_hvm_op */
>>         .quad do_sysctl             /* 35 */
>>         .quad do_domctl
>> +        .quad do_domgrpctl
>>         .rept NR_hypercalls-((.-__hypercall_table)/8)
>>         .quad do_ni_hypercall
>>         .endr
>
> Rather than creating a new hypercall, it may be best to implement all
> the grouping and grouping control as part of a sub-command of the
> domctrl hypercall. Then we don''t need to add this extra hypercall
into
> the table of every platform.
Ack.  There would be no functionality or flexibility lost by making  
group operations part of the domctl hypercall and doing so would also  
shave off a few LOC.  The reasons I had for make domgrpctl operations  
a separate hypercall are now moot.
>> +
>> +    if (d->group)
>> +        grp = d->group;
>> +    else
>> +        grp = find_grp_by_id(NULL_GROUP_ID);
>> +
>
> Wondering why we have to search for the null group. Why can''t the
null
> group be statically initialized and then always referred to by a
> pointer?
Ack.  The searching for the null group is an artifact of an older  
design.
> In fact, what is the role of the null group? I see that it is
> a real domain group and it is created automatically. Also it can''t
be
> created by a hypercall.
The purpose of the null group is to provide a means of efficiently  
addressing all domains that aren''t explicitly in a group.  Although  
it''s a real group, it isn''t intended to be used as one from a
user
perspective.  Perhaps it would be useful for xm to display domains in  
the null group as "ungrouped."
> All domains that do not belong to a group actually belong to the null
> group. Could it also be just as easy for each domain to have an
> identity group (itself), to which it belongs in the absence of any
> other group membership? Or is there a specific purpose to the null
> group?
The current null group implementation isn''t the only way to provide  
an efficient means to identify ungrouped domains, so I''m open to  
alternatives.  That said, is there a win to automatically creating a  
group for each domain?  When needed, a group can be created for an  
existing domain.

The null group should not provide any privileges or otherwise induce  
relationships between members.  The null group was intended primarily  
as a convenient tool for security policy analysis where it''s useful  
to ensure the same property(ies) apply to all ungrouped domains.
>> +#define SERIALIZE_LINKED_LIST(op_name, list_name)		\
>> +    rcu_read_lock(&grp->op_name##_read_lock);			\
>> +    memset(&info->op_name, 0, sizeof(domid_t)*MAX_GROUP_SIZE);
\
>> +    if (!list_empty(&grp->op_name)) {				\
>> +        i = 0;							\
>> +        list_for_each_entry(dom, &grp->op_name, list_name) {	\
>> +            info->op_name[i] = dom->domain_id;			\
>> +       	    i++;						\
>> +	}							\
>> +    } else 							\
>> +	info->op_name[i] = NULL_GROUP_ID;			\
>> +    rcu_read_unlock(&grp->op_name##_read_lock);
>
> This should really be cleaned up and made into an inline function. It
> also refers to a variable declared outside of the macro and parameters
> (info). It''s not clear what''s happening inside the macro.
Ack.
>> +#define RM_DOM_FROM_LIST(list_name, entry)	\
>> +    spin_lock(&grp->list_name##_update_lock);	\
>> +    if (!list_empty(&grp->list_name)) 		\
>> +        list_del(&dom->entry);		    	\
>> +    spin_unlock(&grp->list_name##_update_lock);
>> +
>
> Another good candidate for inlining.
Ack.
>> +
>> +	/* skip it if dom is already a member */
>> +	if (dom_in_member_list(dom->domain_id, grp))
>> +		goto out;
>> +	
>> +	/* remove dom from old group */
>> +	if (dom->group != NULL)
>> +		del_dom_from_grp(dom);
>
> So a domain can only belong to one group at a time. Would it ever be
> useful for a domain to belong to more than one group? This type of
> restriction seems to work against the idea of a general grouping
> concept. For example, all domains that belong to a scheduling group
> (assuming we eventually merge domgrp and schedgrp) would automatically
> be removed from a scheduling group if added to any other type of
> group.
I agree with Samuel; it makes sense that a domain would be removed  
from its schedgrp when changing its domgrp.

In addition to the aesthetic issues Samuel mentioned (domains  
appearing in multiple listings), domgrps intentionally disallows  
membership in multiple groups because of the complications it would  
create.  Consider the following example with 3 domains and 2 groups.   
DomA and domB are in grpX; domB and domC are in grpY.  In this  
scenario XSM security policy expresses two properties: (1) group  
members can communicate with each other and (2) there is no cross- 
group communication.  However, from an information flow perspective  
grpX and grpY are effectively a single group because domB is a member  
of both and can be used to pass information between groups.  While it  
is possible to detect this policy conflict, doing so loses the  
simplicity gains of integrating groups with security policy.

In short, allowing domains to join multiple groups creates many more  
problems than it solves and there are extremely few good use cases  
(if any) where it''s a win.  Contrary to popular belief, my goal is to  
add as little code to Xen as possible.  :)
> This argues for keeping different types of groups under different
> grouping infrastructures unless we can figure out a way for a domain
> to have multiple group memberships. It may be too complicated to do  
> so.
To increase flexibility, I propose allowing domains to opt in to/out  
of schedgrps (or any other feature that is integrated with the domgrp  
mechanism).  Can you describe a likely scenario where this extra  
ability doesn''t meet your needs?
>> +int pause_grp(dgid_t dgid)
>> +{
>> +	int ret = 0;
>> +	struct domain *member;
>> +	struct domain_group *grp;
>> +
>> +	if (dgid == 0) {
>> +		ret = -EPERM;
>> +		goto out;
>> +	}
>> +
>> +	grp = find_grp_by_id(dgid);
>> +
>> +	if (grp == NULL) {
>> +		ret = -ESRCH;
>> +		goto out;
>> +	}
>> +
>> +	spin_lock_recursive(&grp->grp_big_lock);
>> +	rcu_read_lock(&grp->member_list_read_lock);
>> +	/* could ignore interupts during this loop to increase atomicity */
>> +	list_for_each_entry(member, &grp->member_list, member) {
>> +		if (member != current->domain && member->domain_id !=
0)
>> +			domain_pause_by_systemcontroller(member);
>> +	}
>> +	rcu_read_unlock(&grp->member_list_read_lock);
>> +	spin_unlock_recursive(&grp->grp_big_lock);
>> +      out:
>> +	return ret;
>> +}
>> +
>
> Groups are paused in the order in which they are added to the group,
> right? (FIFO). What do we do when we have groups which are dependant
> upon each other in certain ways.
In an earlier domgrps prototype I had a means for expressing  
dependencies that was read in from a group configuration file and was  
propagated into the VMM.  This allowed fine-grained dependencies to  
be expressed and handled at the VMM level.  However, I decided to  
remove it from the VMM because of the complexity it added.  Although  
there are certainly advantages to having that information in the VMM,  
my opinion today is that that functionality belongs in the control  
plane.
> For example, the stub domain and HVM domain. At first glance, it seems
> that you would want to pause the hvm domain before pausing its stub
> domain. And then the reverse on resume: unpause the stub domain, then
> unpause the hvm domain. (I could have it backward). But the point is,
> group operations may have inter-domain dependencies that are not
> accounted for simply by the order in which the domains are added to a
> group.  This is true for pause, unpause, start, migrate, etc., on down
> the line of possible group operations.
I agree.  The configuration-file based approach I mentioned earlier  
allowed users to specify reactions to various events and error  
conditions to include the events you listed above.  That  
functionality is not submitted as part of this patch, but it  
certainly has a place in future work.

I greatly appreciate the feedback.

-Chris

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