Re: [Xen-devel] [PATCH v2] xen: use domid check in is_hardware_domain

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

On 10/07/13 10:38, Jan Beulich wrote:
> > > > On 10.07.13 at 11:18, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
> > On 10/07/13 09:30, Jan Beulich wrote:
> > > > > > On 09.07.13 at 22:28, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
> > > > Instead of checking is_privileged to determine if a domain should
> > > > control the hardware, check that the domain_id is equal to zero (which
> > > > is currently the only domain for which is_privileged is true).  This
> > > > allows other places where domain_id is checked for zero to be replaced
> > > > with is_hardware_domain.
> > > >
> > > > The distinction between is_hardware_domain, is_control_domain, and
> > > > domain 0 is based on the following disaggregation model:
> > > >
> > > > Domain 0 bootstraps the system.  It may remain to perform requested
> > > > builds of domains that need a minimal trust chain (i.e. vTPM domains).
> > > > Other than being built by the hypervisor, nothing is special about this
> > > > domain - although it may be useful to have is_control_domain() return
> > > > true depending on the toolstack it uses to build other domains.
> > > >
> > > > The hardware domain manages devices for PCI pass-through to driver
> > > > domains or can act as a driver domain itself, depending on the desired
> > > > degree of disaggregation.  It is also the domain managing devices that
> > > > do not support pass-through: PCI configuration space access, parsing the
> > > > hardware ACPI tables and system power or machine check events.  This is
> > > > the only domain where is_hardware_domain() is true.  The return of
> > > > is_control_domain() is false for this domain.
> > > >
> > > > The control domain manages other domains, controls guest launch and
> > > > shutdown, and manages resource constraints; is_control_domain() returns
> > > > true.  The functionality guarded by is_control_domain may in the future
> > > > be adapted to use explicit hypercalls, eliminating the special treatment
> > > > of this domain.  It may be reasonable to have multiple control domains
> > > > on a multi-tenant system.
> > > >
> > > > Guest domains and other service or driver domains are all treated
> > > > identically by the hypervisor; the security policy may further constrain
> > > > administrative actions on or communication between these domains.
> > > >
> > > > Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> > > > Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > This isn't correct: I gave my Reviewed-by for the full series; the
> > > Acked-by was given only for the two patches touching only code
> > > I'm maintainer for.
> > >
> > > The distinction we're trying to establish is that an ack implies that
> > > a maintainer is okay with a certain patch (i.e. a non-maintainer
> > > would generally not send ack-s at all), whereas a review means
> > > what it says - the patch was reviewed.
> > The definition you're using for Reviewed-by: is wrong.
> >
> >   From Linux's SubmittingPatches:
> > [...]
> So what was wrong with my description of Reviewed-by?
I think the interpretation of "Ack" is just, "I'm OK with this" / "I 
don't object".  Reviewed-by includes not only, "I think this patch is 
sound", but "I think this patch should be accepted".  As such, it would 
subsume and imply an Ack.
You said, "Reviewed-by means what it says - the patch was reviewed", 
which I understood to mean only "I think this patch is sound", and not 
"I think this patch should be accepted".  Otherwise I don't understand 
the point you are trying to make.
> > So Reviewed-by is much stronger than Acked-by, and one could be forgiven
> > for thinking that it could be "collapsed down" that way.
> What I was trying to point out is my current understanding: No
> matter how Linux understands Acked-by, we aim at it to mean
> that a maintainer is fine with a given patch being committed by
> a committer.
>
> And then again, having offered my Reviewed-by to the whole
> series (and explicitly pointed out that an eventual Acked-by
> would apply only to a subset, in an attempt to make my
> understanding of the tag's meaning explicit),
Yes, and so since Reviewed-by implies everything that Acked-by implies, 
the Acked-by's are redundant.
> I also don't see
> the point in weakening the stronger, wider scope tag.
I'm not what you're talking about here -- which is the stronger scope 
tag, and how do you perceive it being weakened?
 -George

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