Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read

[Blue Swirl]

On Sat, Apr 28, 2012 at 11:42, Andreas Färber <address@hidden> wrote:
> Am 28.04.2012 10:14, schrieb Blue Swirl:
>> On Thu, Apr 26, 2012 at 13:43, Andreas Färber <address@hidden> wrote:
>>> Am 17.04.2012 22:45, schrieb Blue Swirl:
>>>> On Mon, Apr 16, 2012 at 21:47, Anthony Liguori <address@hidden> wrote:
>>>>> On 04/16/2012 04:24 PM, Peter Maydell wrote:
>>>>>> On 16 April 2012 18:42, Anthony Liguori<address@hidden>  wrote:
>>>>>>> * For something to be marked Maintained, there must be a person on M: 
>>>>>>> and
>>>>>>> there must be a git tree for the subsystem.
>>>>>>
>>>>>> Do you mean "there must be a git tree" or "there must be a git tree
>>>>>> listed under T: for this area" ? We have I think several subsystems
>>>>>> where things do come in via pullreq for a submaintainer tree but that
>>>>>> tree isn't officially public except in as much as the branch name
>>>>>> for the pullreq is always the same...
>>>>>
>>>>> I'd like to record T: as part of a way to validate pull requests.  I get
>>>>> slightly nervous about pull requests because it's an easy way to sneak 
>>>>> code
>>>>> into the tree if you're malicious.
>>>>
>>>> Wouldn't signed PULL requests help? They need a very recent git though.
>>>
>>> Signed PULL requests can authenticate the person sending the PULL but
>>> not authorize what areas the contents of the PULL is allowed to touch.
>>
>> Yes, but was that the problem Anthony had with PULLs? For a person
>> with malicious intentions, a pull would not necessarily be the tool of
>> choice, since it could lead to banning and investigations after
>> discovery. I thought Anthony was talking about MITM (or kernel.org
>> style breach) scenarios,
>
> Didn't read that into his words. To me it sounded like he wanted for his
> mailbox scripts to be able to automatically decide whether to accept a
> PULL (which he receives by unsigned email) or not, based on the tree
> documented in MAINTAINERS.
>
> On IRC however Anthony turned towards trusting persons not trees. That
> would easily be supportable by signed PULLs I guess, by whitelisting
> public keys.
>
>>> Any definition of key -> files (just like email -> files) is going to be
>>> surrounded by grey zones and exceptions to the rule, so I guess
>>> verifying each PULL's diff stat and good judgment are the only weapons
>>> against malicious PULLs, given that PULLs have become quite popular
>>> these days and are no longer limited to recurring block, pci, ppc, etc.
>>
>> How is a PULL any different from email, can you do something in a PULL
>> which is not possible with email? I think signed PULLs and commits
>> would give higher level of integrity and non-repudiation than unsigned
>> email.
>
> A PULL is a standardized form of unsigned email. It contains a From:, an
> upstream commit hash, a branch/tag name and usually a diffstat and list
> of commits. So it's more than just a personal mail naming a branch.
>
> The diffstat can easily be verified by fetching from the branch and
> recalculating it from the commit hash in the message. This could then be
> checked against the file patterns in MAINTAINERS.
>
> But like I said, there's exceptions - my upcoming Cocoa PULL will
> include changes to configure and block/*, both not file patterns of the
> Cocoa subsystem, but semantically related and coordinated by mail/IRC.
> Therefore my saying, validating PULLs against MAINTAINERS sections is
> not going to work; validating PULLs for interim projects (like
> QOM'ifications) not documented in MAINTAINERS are not going to work, nor
> is "if no one replies to it after X days send me a PULL". Thus we can
> try to whitelist where to PULL from, but we cannot blacklist PULLs not
> whitelisted, they'll still need to be reviewed individually with some
> common sense.

There's also plenty of code which is not covered by MAINTAINERS, also
qemu-trivial could touch anything.

>
> What would be neat btw is if incoming PULLs could automatically be
> checked by our build bots (we can dream).

We could also dream of having bots to run checkpatch.pl on incoming
patches, or buildbots that use clang analyzer or cppcheck instead of
GCC, or run tests and check test coverage with gcov. We could also use
Gerrit for patch review.

>
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg