Re: [Qemu-devel] Large patch set advice

[Thomas Huth]

On 25.04.2018 21:57, Warner Losh wrote:
> Greetings,
> 
> I’ve foolishly volunteered to rebase all the changes that the bad-user mode 
> folks have done to a recent master rev to get these changes upstreamed.

Great that finally someone dares to do this step! But I hope the "bad"
was just a typo ;-)

[...]
> So as with everything that’s developed over time, the early patches no longer 
> are bistable because there’s later patches that fix the old APIs that were 
> current at the start of the project to use newer APIs.

bistable = bisectable? ... well, I guess that doesn't really matter here
since the upstream bsd-user code is completely broken anyway.

[...]
> Oh, and the number of SoBs in the original code is somewhat lacking. A 
> tedious detail I need to chase down independent of all the other stuff.

Yes, please chase that down first. QEMU project is quite strict about SoBs.

[...]
> My first question seems almost rhetorical: that’s not an acceptable state of 
> affairs, and curation is needed to upstream. Right?

I'd say this does not sound like an acceptable state, but it's finally
Peter who decides what patches get merged.

> My curation plans:
> 
> (1) Find where each of the bits of my ‘true up’ commits at the end are needed 
> and to merge those bits back to those commits (I’m guessing it’s due to 
> conflicts, or false conflict detection in git’s merging logic, since it was 4 
> hours and there attepts to get through the ‘git rebase -i master’ phase). 
> These aren’t true changes anyway: just my screw up. Most of them are easy to 
> find where they belong.
> 
> (2) back merge the API changes as best I can to as early in the stream as 
> possible. One wrinkle here is that there’s a lot of code motion in the early 
> patches to get the MI/MD stuff right, but even in the original commits, it 
> never was very pure at all. And these changes are 2-3k lines long, but 
> completely confined to bsd-user so maybe that’s OK. (I say completely, but 
> sometimes there’s this or that thing added to a Makefile).
> 
> (3) Keep the classes of syscall implementation commits separate, but merge 
> back the API and/or related new syscalls that were added. It’s a tradeoff 
> between having 200 diffs that are minnows and 20 diffs that are related 
> schools of fish vs one huge whale that’s just too big.
> 
> (4) Deal with the cases where multiple people have worked on a patch by 
> putting SoBs for all of them (or reworking them if I can’t get a hold of the 
> original authors) on the commits that are merged. I didn’t see a specific 
> policy for this, but this seems sane. The alternative seems worse (having 
> elements that don’t compile)

AFAIK, merging patches is ok, just mention it in the patch description.
For example if you have a patch with a description like this:

"
 Implement new feature XYZ

 This patch implements a new feature that is very cool.

 Signed-off-by: Alice
"

and then a fixup patch later:

"
 Fix XYZ

 This fixes a bug in XYZ

 Signed-off-by: Bob
"

You could merge them by supplying a note in square brackets:

"
 Implement new feature XYZ

 This patch implements a new feature that is very cool.

 Signed-off-by: Alice
 Signed-off-by: Bob
 [imp: merged fix from Bob into the patch from Alice]
 Signed-off-by: Warner Losh <address@hidden>
"

I think it depends on the contents of Bob's patch whether you should
also include the complete patch description of Bob's patch or not.

> (5) Send them in small batches. I’d go insane trying to manage 200 concurrent 
> code reviews, and I can’t image that I’m unique in this. I also image that 
> fixes in the early part of the series may have ripples to the later parts if 
> my past experience with these things has any relevance.

Yes, please keep the batch sizes small and digestible.

 Thanks,
  Thomas