Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
On Thu, Apr 26, 2012 at 13:43, Andreas Färber afaer...@suse.de wrote: Am 17.04.2012 22:45, schrieb Blue Swirl: On Mon, Apr 16, 2012 at 21:47, Anthony Liguori anth...@codemonkey.ws wrote: On 04/16/2012 04:24 PM, Peter Maydell wrote: On 16 April 2012 18:42, Anthony Liguorianth...@codemonkey.ws wrote: On 04/16/2012 12:17 PM, Peter Maydell wrote: Here's my stab at it: Maintained: Someone actually looks after it. The maintainer will have a git subtree for this area and patches are expected to go through it. Bug reports will generally be investigated. * 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, there signed PULLs and/or signed commits could help. 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. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
Am 28.04.2012 10:14, schrieb Blue Swirl: On Thu, Apr 26, 2012 at 13:43, Andreas Färber afaer...@suse.de wrote: Am 17.04.2012 22:45, schrieb Blue Swirl: On Mon, Apr 16, 2012 at 21:47, Anthony Liguori anth...@codemonkey.ws wrote: On 04/16/2012 04:24 PM, Peter Maydell wrote: On 16 April 2012 18:42, Anthony Liguorianth...@codemonkey.ws 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. What would be neat btw is if incoming PULLs could automatically be checked by our build bots (we can dream). Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
On Sat, Apr 28, 2012 at 11:42, Andreas Färber afaer...@suse.de wrote: Am 28.04.2012 10:14, schrieb Blue Swirl: On Thu, Apr 26, 2012 at 13:43, Andreas Färber afaer...@suse.de wrote: Am 17.04.2012 22:45, schrieb Blue Swirl: On Mon, Apr 16, 2012 at 21:47, Anthony Liguori anth...@codemonkey.ws wrote: On 04/16/2012 04:24 PM, Peter Maydell wrote: On 16 April 2012 18:42, Anthony Liguorianth...@codemonkey.ws 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
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
Am 17.04.2012 22:45, schrieb Blue Swirl: On Mon, Apr 16, 2012 at 21:47, Anthony Liguori anth...@codemonkey.ws wrote: On 04/16/2012 04:24 PM, Peter Maydell wrote: On 16 April 2012 18:42, Anthony Liguorianth...@codemonkey.ws wrote: On 04/16/2012 12:17 PM, Peter Maydell wrote: Here's my stab at it: Maintained: Someone actually looks after it. The maintainer will have a git subtree for this area and patches are expected to go through it. Bug reports will generally be investigated. * 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. 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. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
Am 16.04.2012 23:47, schrieb Anthony Liguori: On 04/16/2012 04:24 PM, Peter Maydell wrote: I don't particularly object to providing a T: line for target-arm.next/arm-devs.next, but I'm not sure it's particularly useful, since we don't have the same tendency the kernel does to having subtrees which can diverge significantly because of large amounts of change waiting for a merge window. I wouldn't expect people to base arm patches against arm-devs.next rather than master, for instance. (Maybe I should??) Anyway, I think if we have T: lines in MAINTAINERS it should be because (and we should clearly say that) that is the tree that we expect patches in that area to apply to. I think we should (and do already?) say that all patches on qemu-devel should be against qemu.git unless otherwise indicated in the patch subject. In very busy phases I have asked people to base their patches on the block branch in order to avoid conflicts and save unnecessary work for both the author and the maintainer. A patch based on master that conflicts with the subtree is useless, it must be rebased anyway. So the theory is that you base patches sent to me on the block branch rather than master. In practice, the difference between qemu subtrees is so small that in most cases basing directly on master works just fine, but it's not the safe way and I may ask you to rebase. Kevin
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
Am 16.04.2012 23:24, schrieb Peter Maydell: On 16 April 2012 18:42, Anthony Liguori anth...@codemonkey.ws wrote: On 04/16/2012 12:17 PM, Peter Maydell wrote: Here's my stab at it: Maintained: Someone actually looks after it. The maintainer will have a git subtree for this area and patches are expected to go through it. Bug reports will generally be investigated. * 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 don't particularly object to providing a T: line for target-arm.next/arm-devs.next, but I'm not sure it's particularly useful, since we don't have the same tendency the kernel does to having subtrees which can diverge significantly because of large amounts of change waiting for a merge window. I wouldn't expect people to base arm patches against arm-devs.next rather than master, for instance. (Maybe I should??) I'm kind of expecting you to, during the Hard Freeze and in case of acceptable patches not for 1.1 during the Soft Freeze. Which is also my problems towards T:, most maintainers don't have trees dedicated to the subsystem but their personal tree with multiple branches. For example, the branch you'd whitelist for pulling ppc from is ppc-for-upstream, whereas contributors should base patches on either ppc-next or (guessing) ppc-1.1. In Peter's case there's separate branches for arm-devs and targer-arm. Officially the branch name is not part of the documented syntax but IMO it's useful for users reading MAINTAINERS. Whereas Anthony seems to be suggesting using MAINTAINERS as a tool for committers, of which currently only two actively handle PULLs. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
On 17 April 2012 09:59, Andreas Färber afaer...@suse.de wrote: Am 16.04.2012 23:24, schrieb Peter Maydell: I wouldn't expect people to base arm patches against arm-devs.next rather than master, for instance. (Maybe I should??) I'm kind of expecting you to, during the Hard Freeze and in case of acceptable patches not for 1.1 during the Soft Freeze. Mm. Depends how much not-for-1.1 stuff arrives during soft/hardfreeze. Which is also my problems towards T:, most maintainers don't have trees dedicated to the subsystem but their personal tree with multiple branches. For example, the branch you'd whitelist for pulling ppc from is ppc-for-upstream, whereas contributors should base patches on either ppc-next or (guessing) ppc-1.1. In Peter's case there's separate branches for arm-devs and targer-arm. I use .next vs .for-upstream as well. Officially the branch name is not part of the documented syntax but IMO it's useful for users reading MAINTAINERS. Whereas Anthony seems to be suggesting using MAINTAINERS as a tool for committers, of which currently only two actively handle PULLs. I don't particularly mind which we do, but I do think that we should be clear about what the semantics are and how we expect people to use this information. -- PMM
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
On 04/17/2012 02:53 AM, Kevin Wolf wrote: Am 16.04.2012 23:47, schrieb Anthony Liguori: On 04/16/2012 04:24 PM, Peter Maydell wrote: I don't particularly object to providing a T: line for target-arm.next/arm-devs.next, but I'm not sure it's particularly useful, since we don't have the same tendency the kernel does to having subtrees which can diverge significantly because of large amounts of change waiting for a merge window. I wouldn't expect people to base arm patches against arm-devs.next rather than master, for instance. (Maybe I should??) Anyway, I think if we have T: lines in MAINTAINERS it should be because (and we should clearly say that) that is the tree that we expect patches in that area to apply to. I think we should (and do already?) say that all patches on qemu-devel should be against qemu.git unless otherwise indicated in the patch subject. In very busy phases I have asked people to base their patches on the block branch in order to avoid conflicts and save unnecessary work for both the author and the maintainer. Yeah, I think this is really the exception that proves the rule. A patch based on master that conflicts with the subtree is useless, it must be rebased anyway. So the theory is that you base patches sent to me on the block branch rather than master. In practice, the difference between qemu subtrees is so small that in most cases basing directly on master works just fine, but it's not the safe way and I may ask you to rebase. I think the block subtree is really an exception simply because it's the oldest and best established subtree. I think requesting patches be against it is reasonable. It's up to you as to what you want the standard policy to be. If you want patches against the block tree normally, I'd just request having folks add a tag like they do for the qemu-kvm tree ([uq/master]). Regards, Anthony Liguori Kevin
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
On Mon, Apr 16, 2012 at 21:47, Anthony Liguori anth...@codemonkey.ws wrote: On 04/16/2012 04:24 PM, Peter Maydell wrote: On 16 April 2012 18:42, Anthony Liguorianth...@codemonkey.ws wrote: On 04/16/2012 12:17 PM, Peter Maydell wrote: Here's my stab at it: Maintained: Someone actually looks after it. The maintainer will have a git subtree for this area and patches are expected to go through it. Bug reports will generally be investigated. * 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. I'd prefer if we kept an official whitelist of trees in MAINTAINERS verses relying on my local .git/config. I don't particularly object to providing a T: line for target-arm.next/arm-devs.next, but I'm not sure it's particularly useful, since we don't have the same tendency the kernel does to having subtrees which can diverge significantly because of large amounts of change waiting for a merge window. I wouldn't expect people to base arm patches against arm-devs.next rather than master, for instance. (Maybe I should??) Anyway, I think if we have T: lines in MAINTAINERS it should be because (and we should clearly say that) that is the tree that we expect patches in that area to apply to. I think we should (and do already?) say that all patches on qemu-devel should be against qemu.git unless otherwise indicated in the patch subject. Regards, Anthony Liguori -- PMM
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
On 04/16/2012 09:31 AM, Andreas Färber wrote: Hello, Sparked by conversations with Anthony and the discussion on a recent KVM call, I've started overhauling our MAINTAINERS file. Patches 1-5 fix syntax issues. Patch 6 documents our orphaned stable trees, as requested by Anthony. Patch 7 drops the orphaned and by now completely busted darwin-user emulation. Apparently we have unwritten eligibility criteria for new maintainers in terms of qemu-devel participation and patch handling quality, but no formal mechanism to handle replacing lost maintainers. The current practice has become for Anthony to expect people listed in a MAINTAINERS section with S: Maintained to handle patches in that area themselves and to supply a [PULL] request to get those changes into qemu.git. This has the downside that patches falling into an area, where a maintainer is listed but not responding, simply bitrot on the list. I think we ought to aggressively downgrade subsystems if this is really a problem. Unfortunately, it's hard to judge whether this is a problem until someone complains about a specific subsystem. Patches 8-11 therefore propose to upgrade some actively maintained sections to Maintained to formalize the Maintained vs. Odd Fixes semantics: Maintained means PULLs from maintainer expected. Yes. More specifically, if something is Maintained, I would expect the patch to always come in through that specific tree. Odd Fixes means Reviewed-by/Acked-by or committer's gut feeling is sufficient. Yes. Odd Fixes and below means the patch is fair game but that the listed M: probably ought to at least be consulted. Regards, Anthony Liguori Patches 12-13 propose to downgrade the TCG targets in question to Odd Fixes, to allow committers to handle patches without maintainers' approval. Not being a committer, I don't want to remove any maintainer, but if someone doesn't want to and is no longer able to act as maintainer, then please post a patch yourselves updating MAINTAINERS accordingly. I for one would be happy to hand over Cocoa maintenance to someone using it more frequently than me these days, just like I would be happy to let someone else maintain Solaris/illumos support with a little bit more experience. The BSDs might also want to formalize what person or mailing list patches should go through. Patch 14 documents me volunteering to maintain the 0.15 stable tree. Regards, Andreas Cc: Anthony Liguorianth...@codemonkey.ws Cc: Paul Brookp...@codesourcery.com Cc: Aurélien Jarnoaurel...@aurel32.net Andreas Färber (14): MAINTAINERS: Fix PC file pattern MAINTAINERS: Fix virtio-9p file pattern MAINTAINERS: Fix TCI file pattern MAINTAINERS: Indicate type of SCM MAINTAINERS: Fix SCM tree for virtio-9p MAINTAINERS: Document all stable trees Drop darwin-user MAINTAINERS: Upgrade PReP to Maintained MAINTAINERS: Upgrade Cocoa to Maintained MAINTAINERS: Upgrade IDE to Maintained MAINTAINERS: Upgrade NBD to Maintained MAINTAINERS: Downgrade target-m68k to Odd Fixes MAINTAINERS: Downgrade target-mips and target-sh4 to Odd Fixes MAINTAINERS: Take over 0.15 maintenance MAINTAINERS | 64 +- Makefile.target | 28 - configure| 25 - darwin-user/commpage.c | 357 darwin-user/ioctls.h |4 - darwin-user/ioctls_types.h |1 - darwin-user/machload.c | 902 --- darwin-user/main.c | 1027 -- darwin-user/mmap.c | 409 - darwin-user/qemu.h | 178 darwin-user/signal.c | 452 -- darwin-user/syscall.c| 1566 -- darwin-user/syscalls.h | 384 - default-configs/i386-darwin-user.mak |1 - default-configs/ppc-darwin-user.mak |3 - qemu-doc.texi| 90 -- qemu-tech.texi |3 +- 17 files changed, 44 insertions(+), 5450 deletions(-) delete mode 100644 darwin-user/commpage.c delete mode 100644 darwin-user/ioctls.h delete mode 100644 darwin-user/ioctls_types.h delete mode 100644 darwin-user/machload.c delete mode 100644 darwin-user/main.c delete mode 100644 darwin-user/mmap.c delete mode 100644 darwin-user/qemu.h delete mode 100644 darwin-user/signal.c delete mode 100644 darwin-user/syscall.c delete mode 100644 darwin-user/syscalls.h delete mode 100644 default-configs/i386-darwin-user.mak delete mode 100644 default-configs/ppc-darwin-user.mak
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
Am 16.04.2012 18:00, schrieb Anthony Liguori: On 04/16/2012 09:31 AM, Andreas Färber wrote: Hello, Sparked by conversations with Anthony and the discussion on a recent KVM call, I've started overhauling our MAINTAINERS file. Patches 1-5 fix syntax issues. Patch 6 documents our orphaned stable trees, as requested by Anthony. Patch 7 drops the orphaned and by now completely busted darwin-user emulation. Apparently we have unwritten eligibility criteria for new maintainers in terms of qemu-devel participation and patch handling quality, but no formal mechanism to handle replacing lost maintainers. The current practice has become for Anthony to expect people listed in a MAINTAINERS section with S: Maintained to handle patches in that area themselves and to supply a [PULL] request to get those changes into qemu.git. This has the downside that patches falling into an area, where a maintainer is listed but not responding, simply bitrot on the list. I think we ought to aggressively downgrade subsystems if this is really a problem. Unfortunately, it's hard to judge whether this is a problem until someone complains about a specific subsystem. Patches 8-11 therefore propose to upgrade some actively maintained sections to Maintained to formalize the Maintained vs. Odd Fixes semantics: Maintained means PULLs from maintainer expected. Yes. More specifically, if something is Maintained, I would expect the patch to always come in through that specific tree. Odd Fixes means Reviewed-by/Acked-by or committer's gut feeling is sufficient. Yes. Odd Fixes and below means the patch is fair game but that the listed M: probably ought to at least be consulted. The current status descriptions seem to be a copy from Linux. Could you address Kevin's comment by proposing a change to the descriptions in our copy? Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
On 16 April 2012 17:03, Andreas Färber afaer...@suse.de wrote: Am 16.04.2012 18:00, schrieb Anthony Liguori: On 04/16/2012 09:31 AM, Andreas Färber wrote: Hello, Sparked by conversations with Anthony and the discussion on a recent KVM call, I've started overhauling our MAINTAINERS file. Patches 1-5 fix syntax issues. Patch 6 documents our orphaned stable trees, as requested by Anthony. Patch 7 drops the orphaned and by now completely busted darwin-user emulation. Apparently we have unwritten eligibility criteria for new maintainers in terms of qemu-devel participation and patch handling quality, but no formal mechanism to handle replacing lost maintainers. The current practice has become for Anthony to expect people listed in a MAINTAINERS section with S: Maintained to handle patches in that area themselves and to supply a [PULL] request to get those changes into qemu.git. This has the downside that patches falling into an area, where a maintainer is listed but not responding, simply bitrot on the list. I think we ought to aggressively downgrade subsystems if this is really a problem. Unfortunately, it's hard to judge whether this is a problem until someone complains about a specific subsystem. Patches 8-11 therefore propose to upgrade some actively maintained sections to Maintained to formalize the Maintained vs. Odd Fixes semantics: Maintained means PULLs from maintainer expected. Yes. More specifically, if something is Maintained, I would expect the patch to always come in through that specific tree. Odd Fixes means Reviewed-by/Acked-by or committer's gut feeling is sufficient. Yes. Odd Fixes and below means the patch is fair game but that the listed M: probably ought to at least be consulted. The current status descriptions seem to be a copy from Linux. Could you address Kevin's comment by proposing a change to the descriptions in our copy? Here's my stab at it: Supported: Someone is actually paid to look after this. This is the same as Maintained (see below) but you might have a greater chance of faster response times. Maintained: Someone actually looks after it. The maintainer will have a git subtree for this area and patches are expected to go through it. Bug reports will generally be investigated. Odd Fixes: It has a maintainer but they don't have time to do much other than throw the odd patch in. Patches are applied directly to master; there is no subtree, and no requirement for an Ack from the maintainer for a patch to be applied. Bug reports without reasonable quality patches attached are likely to go unfixed. Orphan: No current maintainer. Send patches to qemu-devel; persistence may be required to get something reviewed and committed, especially if it's a large change. Obsolete:Old code. Something tagged obsolete generally means it has been replaced by a better system and you should be using that. (I dropped the parenthetical about becoming a maintainer of an orphan area; if we want to keep that I think an expanded section about how to demonstrate that you might be capable of the job, how/when to add sections for new code/drivers/boards, etc would be more useful.) -- PMM
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
On 16 April 2012 18:17, Peter Maydell peter.mayd...@linaro.org wrote: Here's my stab at it: Supported: Someone is actually paid to look after this. This is the same as Maintained (see below) but you might have a greater chance of faster response times. Maintained: Someone actually looks after it. The maintainer will have a git subtree for this area and patches are expected to go through it. Bug reports will generally be investigated. ...or we could just drop Supported altogether as not being a particularly useful distinction for us. -- PMM
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
On 04/16/2012 12:17 PM, Peter Maydell wrote: On 16 April 2012 17:03, Andreas Färberafaer...@suse.de wrote: Am 16.04.2012 18:00, schrieb Anthony Liguori: On 04/16/2012 09:31 AM, Andreas Färber wrote: Hello, Sparked by conversations with Anthony and the discussion on a recent KVM call, I've started overhauling our MAINTAINERS file. Patches 1-5 fix syntax issues. Patch 6 documents our orphaned stable trees, as requested by Anthony. Patch 7 drops the orphaned and by now completely busted darwin-user emulation. Apparently we have unwritten eligibility criteria for new maintainers in terms of qemu-devel participation and patch handling quality, but no formal mechanism to handle replacing lost maintainers. The current practice has become for Anthony to expect people listed in a MAINTAINERS section with S: Maintained to handle patches in that area themselves and to supply a [PULL] request to get those changes into qemu.git. This has the downside that patches falling into an area, where a maintainer is listed but not responding, simply bitrot on the list. I think we ought to aggressively downgrade subsystems if this is really a problem. Unfortunately, it's hard to judge whether this is a problem until someone complains about a specific subsystem. Patches 8-11 therefore propose to upgrade some actively maintained sections to Maintained to formalize the Maintained vs. Odd Fixes semantics: Maintained means PULLs from maintainer expected. Yes. More specifically, if something is Maintained, I would expect the patch to always come in through that specific tree. Odd Fixes means Reviewed-by/Acked-by or committer's gut feeling is sufficient. Yes. Odd Fixes and below means the patch is fair game but that the listed M: probably ought to at least be consulted. The current status descriptions seem to be a copy from Linux. Could you address Kevin's comment by proposing a change to the descriptions in our copy? Here's my stab at it: Supported: Someone is actually paid to look after this. This is the same as Maintained (see below) but you might have a greater chance of faster response times. Let's drop Supported as a status. Maintained: Someone actually looks after it. The maintainer will have a git subtree for this area and patches are expected to go through it. Bug reports will generally be investigated. * For something to be marked Maintained, there must be a person on M: and there must be a git tree for the subsystem. Odd Fixes: It has a maintainer but they don't have time to do much other than throw the odd patch in. Patches are applied directly to master; there is no subtree, and no requirement for an Ack from the maintainer for a patch to be applied. Bug reports without reasonable quality patches attached are likely to go unfixed. Orphan: No current maintainer. Send patches to qemu-devel; persistence may be required to get something reviewed and committed, especially if it's a large change. Obsolete:Old code. Something tagged obsolete generally means it has been replaced by a better system and you should be using that. I like these descriptions a lot more. Regards, Anthony Liguori (I dropped the parenthetical about becoming a maintainer of an orphan area; if we want to keep that I think an expanded section about how to demonstrate that you might be capable of the job, how/when to add sections for new code/drivers/boards, etc would be more useful.) -- PMM
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
On 04/16/2012 12:40 PM, Peter Maydell wrote: On 16 April 2012 18:17, Peter Maydellpeter.mayd...@linaro.org wrote: Here's my stab at it: Supported: Someone is actually paid to look after this. This is the same as Maintained (see below) but you might have a greater chance of faster response times. Maintained: Someone actually looks after it. The maintainer will have a git subtree for this area and patches are expected to go through it. Bug reports will generally be investigated. ...or we could just drop Supported altogether as not being a particularly useful distinction for us. Ack. Regards, Anthony Liguori -- PMM
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
On 16 April 2012 18:42, Anthony Liguori anth...@codemonkey.ws wrote: On 04/16/2012 12:17 PM, Peter Maydell wrote: Here's my stab at it: Maintained: Someone actually looks after it. The maintainer will have a git subtree for this area and patches are expected to go through it. Bug reports will generally be investigated. * 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 don't particularly object to providing a T: line for target-arm.next/arm-devs.next, but I'm not sure it's particularly useful, since we don't have the same tendency the kernel does to having subtrees which can diverge significantly because of large amounts of change waiting for a merge window. I wouldn't expect people to base arm patches against arm-devs.next rather than master, for instance. (Maybe I should??) Anyway, I think if we have T: lines in MAINTAINERS it should be because (and we should clearly say that) that is the tree that we expect patches in that area to apply to. -- PMM
Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read
On 04/16/2012 04:24 PM, Peter Maydell wrote: On 16 April 2012 18:42, Anthony Liguorianth...@codemonkey.ws wrote: On 04/16/2012 12:17 PM, Peter Maydell wrote: Here's my stab at it: Maintained: Someone actually looks after it. The maintainer will have a git subtree for this area and patches are expected to go through it. Bug reports will generally be investigated. * 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. I'd prefer if we kept an official whitelist of trees in MAINTAINERS verses relying on my local .git/config. I don't particularly object to providing a T: line for target-arm.next/arm-devs.next, but I'm not sure it's particularly useful, since we don't have the same tendency the kernel does to having subtrees which can diverge significantly because of large amounts of change waiting for a merge window. I wouldn't expect people to base arm patches against arm-devs.next rather than master, for instance. (Maybe I should??) Anyway, I think if we have T: lines in MAINTAINERS it should be because (and we should clearly say that) that is the tree that we expect patches in that area to apply to. I think we should (and do already?) say that all patches on qemu-devel should be against qemu.git unless otherwise indicated in the patch subject. Regards, Anthony Liguori -- PMM
