Am 06.01.2012 16:19, schrieb Anthony Liguori: > I'd like to start a more formal and transparent security audit of QEMU. > The way I'd imagine it working is something like this:
I'd like to propose something else: We should define a more formal process for reviewing and applying patches in the first place. The better upfront review we have, the less issues to track down later. For example: i) Unless it's a build fix, I propose defining a minimum review time before a patch is applied to a (sub)maintainer's queue. Avi's I/O dispatch series was pulled into the trees two days after posting it, which was definitely not enough time to review and test it. For qemu-trivial by comparison we have a rather predictable weekly or bi-weekly rhythm. Otherwise we'll have to 'fearfully' spam the list with Reviewed-bys or possible objections cluttering the list instead of reviewing the whole series first and adding a summarized reply. ii) We regularly point newbies to SubmitAPatch and MAINTAINERS. Core developers should respect those as well to give submaintainers a chance to review and test before the merge: "CC the relevant maintainer -- look in the MAINTAINERS file to find out who that is" git-send-email offers the Cc: line to help make people aware of individual patches touching their subsystem within a large series. If we don't have a maintainer on file for something we need to fix that. iii) The Git mailing list used to have regular "What's cooking" mails listing patches that had been reviewed but not yet applied to master. Sort of like Anthony's recent speak-now reminder or the former aliguori-queue.git. Maybe pull into a next branch and only merge from there into master after a timeout? Not sure if it's worth the effort. iv) Given that i) and ii) are respected, a PULL request should be applied within a reasonable time frame without resparking the basic is-this-patch-doing-the-right-thing discussion since that should've happened on the PATCHes earlier on. A PULL breaking the build is another matter of course, but individual patches can still be reverted or reworked afterwards. Or should a PULL generally be re-reviewed within a fixed timeframe, questionmark? If so, by whom? It would be nice to have a more explicit process of who pulls from whom and how this is handled during maintainers' absences - especially when approaching a release. If queues do not get pulled into master in time, then an orchestrated Test Day or Code Audit is not worth that much. v) Posting static analysis reports is a good thing, but a Launchpad attachment doesn't give them a lot of exposure. Would it be possible to have a regular, differential textual report from some tools, similar to how the Intel guys are summarizing test results for KVM? Maybe even integrated with one of the buildbots? As a simple example, false spelling positives in rarely changed softfloat code might be filtered out by diff'ing against last week's report. Or just a summary "For week w, $TOOL reported n new potential issues". Whatever we decide on, we should document it in the Wiki so that patch contributors know ahead of time how patient they should be, for reviewers to plan or to signal the maintainer an objection or wait-condition in time, and for submaintainers to know how much time to give other reviewers for comments or tags. Comments? Further or contrary suggestions? Andreas