[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [v8][PATCH 04/17] update the existing hypercall to support XEN_DOMCTL_set_rdm
> From: Tim Deegan [mailto:tim@xxxxxxx] > Sent: Tuesday, December 09, 2014 6:11 PM > > At 08:19 +0000 on 09 Dec (1418109561), Jan Beulich wrote: > > Why do you always pick other than the simplest possible solution? > > Jan, please don't make personal comments like this in code review. It > can only offend and demoralize contributors, and deter other people > from joining in. > > I understand that it can be frustrating, and I'm sure I have lashed > out at people on-list in the past. But remember that people who are > new to the project need time to learn, and keep the comments to the > code itself. > > I can see that this series has been going for a long time, and is > still getting hung up on coding style issues. Might it be useful to > have a round of internal review from some of the more xen-experienced > engineers at Intel before Jan looks at it again? > Thanks Tim. Some of my thoughts: 1. It's more efficient for new people to start from a small, well-defined task in one area, and then spanning to adjacent areas gradually. Patience must be given by the community to help them grow; 2. Unfortunately this RMRR effort grows from original two patches (very VT-d focused) to now v8 17 patches spanning many areas (including hypercall, mmu, domain builder, hvmloader, etc.), and thus imposing both long learning curve and lots of frustrations being no achievement returned. Though having a complete solution is desired, we need to help split the big task into step-by-step approach as long as: - the overall design is agreed - each step is self-contained - each step won't be wasted moving forward. That way new people can see incremental achievements, instead of being hit down before final success. Take this RMRR for example. Maybe we can split into steps like: step-1: setup RMRR identify mapping in hypervisor. fail if confliction. no user space changes step-2: expose RMRR knowledge to userspace and detect confliction in domain builder and hvmloader step-3: reconcile e820/mmio to avoid confliction in a best effort step-4: miscellaneous enhancements, each can be ACK-ed individually: - handle guest CPU access to RMRR regions - handle conflicting RMRR regions - handle group RMRRs - re-enable USB device assignment That way Tiejun can focus on a self-contained task at one time, and then observe continuous acknowledgements for his work. We don't need to claim RMRR fully ready in Xen until last patch is accepted, but that at least provides a way to ack new people when working on complex features and also allow for partial usage on his work. 3. Maintainers sometimes didn't give definitive guidance to the new people, and the high-level design is not closed in the 1st place. e.g. when I thought this v8 version has everyone agreed on the design, there's another comment from Jan about using XENMEM_set_memory_map might be better, while back to Aug. when Tiejun raised that option the answer is "I'm not sure". Note I understand as a maintainer he might not have definite answers for all opens. But w/o a definitive guide new people may waste lots of effort on chasing a wrong option, which is definitely frustrating. So I'd suggest for such non-trivial task for a new people, all maintainers in involved areas (xen, mmu, tools, vtd, etc) should first spend time to agree on the high level design. At that stage, let's skip the coding problems to save both time. After agreed design, then we can help the new people to improve the coding to reach check-in criteria, which then becomes a converging process. for this RMRR issue, let's close the design first, and then use staged approach to get this patch series in. 4. Regarding to coding style, Intel will definitely help our new people internally, and we also like all the discussion happened publicly, to also benefit from the community, especially regarding to current scope extended out of VT-d area. In the meantime, if w/ above suggestions, our new people can then focus on incremental deliverables and then pay more attention to coding styles. Thanks Kevin _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |