Re: nvme emulation merge process

[Andrzej Jakowski]

On 7/1/20 6:57 AM, Philippe Mathieu-Daudé wrote:
> On 7/1/20 3:18 PM, Klaus Jensen wrote:
>> On Jul  1 12:34, Kevin Wolf wrote:
>>> Am 30.06.2020 um 22:36 hat Klaus Jensen geschrieben:
>>>> On Jun 30 08:42, Keith Busch wrote:
>>>>> On Tue, Jun 30, 2020 at 04:09:46PM +0200, Philippe Mathieu-Daudé wrote:
>>>>>> What I see doable for the following days is:
>>>>>> - hw/block/nvme: Fix I/O BAR structure [3]
>>>>>> - hw/block/nvme: handle transient dma errors
>>>>>> - hw/block/nvme: bump to v1.3
>>>>>
>>>>>
>>>>> These look like sensible patches to rebase future work on, IMO. The 1.3
>>>>> updates had been prepared a while ago, at least.
>>>>
>>>> I think Philippe's "hw/block/nvme: Fix I/O BAR structure" series is a
>>>> no-brainer. It just needs to get in asap.
>>>
>>> I think we need to talk about how nvme patches are supposed to get
>>> merged. I'm not familiar with the hardware nor the code, so the model
>>> was that I just blindly merge patches that Keith has reviewed/acked,
>>> just to spare him the work to prepare a pull request. But obviously, we
>>> started doing things this way when there was a lot less activity around
>>> the nvme emulation.
>>>
>>> If we find that this doesn't scale any more, maybe we need to change
>>> something.
>>
>> Honestly, I do not think the current model has worked very well for some
>> time; especially for larger series where I, for one, has felt that my
>> work was largely ignored due to a lack of designated reviewers. Things
>> only picked up when Beata, Maxim and Philippe started reviewing my
>> series - maybe out of pity or because I was bombing the list, I don't
>> know ;)
> 
> I have no interest in the NVMe device emulation, but one of the first
> thing I notice when I look at the wiki the time I wanted to send my
> first patch, is the "Return the favor" paragraph:
> https://wiki.qemu.org/Contribute/SubmitAPatch#Return_the_favor
> 
>  "Peer review only works if everyone chips in a bit of review time.
>   If everyone submitted more patches than they reviewed, we would
>   have a patch backlog. A good goal is to try to review at least as
>   many patches from others as what you submit. Don't worry if you
>   don't know the code base as well as a maintainer; it's perfectly
>   fine to admit when your review is weak because you are unfamiliar
>   with the code."
> 
> So as some reviewed my patches, I try to return the favor to the
> community, in particular when I see someone is stuck waiting for
> review, and the patch topic is some area I can understand.
> 
> I don't see that as an "out of pity" reaction.
> 
> Note, it is true bomb series scares reviewers. You learned it the
> bad way. But you can see, after resending the first part of your
> "bomb", even if it took 10 versions, the result is a great
> improvement!
> 
>> We've also seen good patches from Andrzej linger on the list for quite a
>> while, prompting a number of RESENDs. I only recently allocated more
>> time and upped my review game, but I hope that contributors feel that
>> stuff gets reviewed in a timely fashion by now.
>>
>> Please understand that this is in NO WAY a criticism of Keith who
>> already made it very clear to me that he did not have a lot time to
>> review, but only ack the odd patch.
>>
>>> Depending on how much time Keith can spend on review in the
>>> near future and how much control he wants to keep over the development,
>>> I could imagine adding Klaus to MAINTAINERS, either as a co-maintainer
>>> or as a reviewer. Then I could rely on reviews/acks from either of you
>>> for merging series.
>>>
>>
>> I would be happy to step up (officially) to help maintain the device
>> with Keith and review on a daily basis, and my position can support
>> this.
> 
> Sounds good to me, but it is up to Keith Busch to accept.
> 
> It would be nice to have at least one developer from WDC listed as
> designated reviewer too.
> 
> Maxim is candidate for designated reviewer but I think he doesn't
> have the time.
> 
> It would also nice to have Andrzej Jakowski listed, if he is interested.

Thx! Of course I am interested in helping and I think it is actually great 
idea to have couple of designated maintainers/reviewers as it would be easier
for folks to receive feedback vs requesting it in polling manner :)
And please don't get me wrong -- I'm not complaining about anything -- I
think it is just reality that everybody is stretched out into multiple 
directions
struggling to allocate time for multiple things. Having many people will
actually increase likelihood of introducing high quality improvements.

Also, +1 on separate tree for nvme emulation.

> 
>>
>>> Of course, the patches don't necessarily have to go through my tree
>>> either if this only serves to complicate things these days. If sending
>>> separate pull requests directly to Peter would make things easier, I
>>> certainly wouldn't object.
>>>
>>
>> I don't think there is any reason to by-pass your tree. I think the
>> volume would need to increase even further for that to make sense.
>>
>