Re: nvme emulation merge process
On Thu, Jul 02, 2020 at 01:29:26PM -0700, Andrzej Jakowski wrote: > > 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. Thanks for your help. Klaus and I will be setting up an external tree for qemu-nvme development (tentatively on git.infradead.org) and pull-request. I'm just waiting for the server admin to upload our public keys. If I don't hear back by Monday, I will use an alternate server in the interim.
Re: nvme emulation merge process
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
Re: nvme emulation merge process (was: Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces)
On Wed, Jul 01, 2020 at 03:57:27PM +0200, Philippe Mathieu-Daudé wrote: > On 7/1/20 3:18 PM, Klaus Jensen wrote: > > 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. I definitely want to continue at least having the opprotunity to review, though you may have noticed I am a bit low on time for more thorough maintenance this project deserves. The recent development pace for nvme would benefit from having its own tree, so I'm open to either co-maintenance, or handing off this to others. Please allow me to send a few queries off-list today to check-in with potentially interested parties. > 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.
Re: nvme emulation merge process (was: Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces)
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. > >> 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. >
Re: nvme emulation merge process (was: Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces)
On Wed, 2020-07-01 at 15:18 +0200, 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 ;) > > 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. > > > 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. > It my fault as well - I need to get back to reviewing these. (I'll review few of them today I hope) Best regards, Maxim Levitsky
Re: nvme emulation merge process (was: Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces)
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 ;) 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. > 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.
nvme emulation merge process (was: Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces)
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. 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. 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. Kevin
