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
RE: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces
> -Original Message-
> From: Klaus Jensen
> Sent: Tuesday, June 30, 2020 4:30 PM
> To: Niklas Cassel
> Cc: qemu-block@nongnu.org; Klaus Jensen ;
> qemu-de...@nongnu.org; Keith Busch ; Max Reitz
> ; Kevin Wolf ; Javier Gonzalez
> ; Maxim Levitsky ;
> Philippe Mathieu-Daudé ; Dmitry Fomichev
> ; Damien Le Moal
> ; Matias Bjorling
> Subject: Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned
> namespaces
>
> On Jun 30 12:59, Niklas Cassel wrote:
> > On Tue, Jun 30, 2020 at 12:01:29PM +0200, Klaus Jensen wrote:
> > > From: Klaus Jensen
> > >
> > > Hi all,
> >
> > Hello Klaus,
> >
>
> Hi Niklas,
>
> > >
> > > * the controller uses timers to autonomously finish zones (wrt. FRL)
> >
> > AFAICT, Dmitry's patches does this as well.
> >
>
> Hmm, yeah. Something is going on at least. It's not really clear to me
> why it works or what is happening with that admin completion queue
> timer, but I'll dig through it.
>
> > >
> > > I've been on paternity leave for a month, so I havn't been around to
> > > review Dmitry's patches, but I have started that process now. I would
> > > also be happy to work with Dmitry & Friends on merging our versions to
> > > get the best of both worlds if it makes sense.
> > >
> > > This series and all preparatory patch sets (the ones I've been posting
> > > yesterday and today) are available on my GitHub[2]. Unfortunately
> > > Patchew got screwed up in the middle of me sending patches and it
> never
> > > picked up v2 of "hw/block/nvme: support multiple namespaces" because
> it
> > > was getting late and I made a mistake with the CC's. So my posted series
> > > don't apply according to Patchew, but they actually do if you follow the
> > > Based-on's (... or just grab [2]).
> > >
> > >
> > > [1]: Message-Id: <20200617213415.22417-1-dmitry.fomic...@wdc.com>
> > > [2]: https://github.com/birkelund/qemu/tree/for-master/nvme
> > >
> > >
> > > Based-on: <20200630043122.1307043-1-...@irrelevant.dk>
> > > ("[PATCH 0/3] hw/block/nvme: bump to v1.4")
> >
> > Is this the only patch series that this series depends on?
> >
> > In the beginning of the cover letter, you mentioned
> > "NVMe v1.4 mandatory support", "SGLs", "multiple namespaces",
> > and "and mostly just overall clean up".
> >
>
> No, its a string of series that all has a Based-on tag (that is, "[PATCH
> 0/3] hw/block/nvme: bump to v1.4" has another Based-on tag that points
> to the dependency of that). The point was to have patchew nicely apply
> everything, but it broke midway...
>
> As Philippe pointed out, all of the patch sets are integrated in the
> GitHub tree, applied to QEMU master.
>
> >
> > I think that you have done a great job getting the NVMe
> > driver out of a frankenstate, and made it compliant with
> > a proper spec (NVMe 1.4).
> >
> > I'm also a big fan of the refactoring so that the driver
> > handles more than one namespace, and the new bus model.
> >
>
> Well, thanks! :)
>
> > I know that you first sent your
> > "nvme: support NVMe v1.3d, SGLs and multiple namespaces"
> > patch series July, last year.
> >
> > Looking at your outstanding patch series on patchwork:
> > https://patchwork.kernel.org/project/qemu-devel/list/?submitter=188679
> >
> > (Feel free to correct me if I have misunderstood anything.)
> >
> > I see that these are related to your patch series from July last year:
> > hw/block/nvme: bump to v1.3
> > hw/block/nvme: support scatter gather lists
> > hw/block/nvme: support multiple namespaces
> > hw/block/nvme: bump to v1.4
> >
>
> Yeah this stuff has been around for a while so the history on patchwork
> is a mess.
>
> >
> > This patch series seems minor and could probably be merged immediately:
> > hw/block/nvme: handle transient dma errors
> >
>
> Sure, but it's nicer in combination with the previous series
> ("hw/block/nvme: AIO and address mapping refactoring"). What I /can/ do
> is rip out "hw/block/nvme: allow multiple aios per command" as that
> patch might require more time for reviews. The rest of that series are
> clean ups and a couple of bug fixes.
>
> >
> > This patch series looks a bit weird:
> > hw/block/nvme: AIO and address mapping refactoring
> >
> > Since it looks like a V1 post, and was fi
Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces
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. The "hw/block/nvme: handle transient dma errors" series would really benefit from most of the patches in my "hw/block/nvme: AIO and address mapping refactoring" series. The elephant in the room is the AIO part (the "hw/block/nvme: allow multiple aios per command" patch), so I will get rid of it and leave the cleanup patches there and post it as a new series together with the "handle transient dma errors" fixes. This would make it a series of around ~17-18 patches, but I think they are all quite reviewable. The bump to v1.3 should also pretty much be ready for merging.
Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces
On Jun 30 12:59, Niklas Cassel wrote:
> On Tue, Jun 30, 2020 at 12:01:29PM +0200, Klaus Jensen wrote:
> > From: Klaus Jensen
> >
> > Hi all,
>
> Hello Klaus,
>
Hi Niklas,
> >
> > * the controller uses timers to autonomously finish zones (wrt. FRL)
>
> AFAICT, Dmitry's patches does this as well.
>
Hmm, yeah. Something is going on at least. It's not really clear to me
why it works or what is happening with that admin completion queue
timer, but I'll dig through it.
> >
> > I've been on paternity leave for a month, so I havn't been around to
> > review Dmitry's patches, but I have started that process now. I would
> > also be happy to work with Dmitry & Friends on merging our versions to
> > get the best of both worlds if it makes sense.
> >
> > This series and all preparatory patch sets (the ones I've been posting
> > yesterday and today) are available on my GitHub[2]. Unfortunately
> > Patchew got screwed up in the middle of me sending patches and it never
> > picked up v2 of "hw/block/nvme: support multiple namespaces" because it
> > was getting late and I made a mistake with the CC's. So my posted series
> > don't apply according to Patchew, but they actually do if you follow the
> > Based-on's (... or just grab [2]).
> >
> >
> > [1]: Message-Id: <20200617213415.22417-1-dmitry.fomic...@wdc.com>
> > [2]: https://github.com/birkelund/qemu/tree/for-master/nvme
> >
> >
> > Based-on: <20200630043122.1307043-1-...@irrelevant.dk>
> > ("[PATCH 0/3] hw/block/nvme: bump to v1.4")
>
> Is this the only patch series that this series depends on?
>
> In the beginning of the cover letter, you mentioned
> "NVMe v1.4 mandatory support", "SGLs", "multiple namespaces",
> and "and mostly just overall clean up".
>
No, its a string of series that all has a Based-on tag (that is, "[PATCH
0/3] hw/block/nvme: bump to v1.4" has another Based-on tag that points
to the dependency of that). The point was to have patchew nicely apply
everything, but it broke midway...
As Philippe pointed out, all of the patch sets are integrated in the
GitHub tree, applied to QEMU master.
>
> I think that you have done a great job getting the NVMe
> driver out of a frankenstate, and made it compliant with
> a proper spec (NVMe 1.4).
>
> I'm also a big fan of the refactoring so that the driver
> handles more than one namespace, and the new bus model.
>
Well, thanks! :)
> I know that you first sent your
> "nvme: support NVMe v1.3d, SGLs and multiple namespaces"
> patch series July, last year.
>
> Looking at your outstanding patch series on patchwork:
> https://patchwork.kernel.org/project/qemu-devel/list/?submitter=188679
>
> (Feel free to correct me if I have misunderstood anything.)
>
> I see that these are related to your patch series from July last year:
> hw/block/nvme: bump to v1.3
> hw/block/nvme: support scatter gather lists
> hw/block/nvme: support multiple namespaces
> hw/block/nvme: bump to v1.4
>
Yeah this stuff has been around for a while so the history on patchwork
is a mess.
>
> This patch series seems minor and could probably be merged immediately:
> hw/block/nvme: handle transient dma errors
>
Sure, but it's nicer in combination with the previous series
("hw/block/nvme: AIO and address mapping refactoring"). What I /can/ do
is rip out "hw/block/nvme: allow multiple aios per command" as that
patch might require more time for reviews. The rest of that series are
clean ups and a couple of bug fixes.
>
> This patch series looks a bit weird:
> hw/block/nvme: AIO and address mapping refactoring
>
> Since it looks like a V1 post, and was first posted yesterday.
> However, 2 out of the 17 patches in are Acked-by: Keith.
> (Perhaps some of your previously posted patches was put inside
> this new patch series?)
>
Yes that and reviewers requested a lot of separation, so basically the
patch set ballooned.
>
> This patch series:
> hw/block/nvme: namespace types and zoned namespaces
>
> Which was first posted today. Up until earlier today, we haven't seen
> any patches from you related to ZNS (only overall NVMe cleanups).
> Dmitry's ZNS patches have been on the list since 2020-06-16.
>
Yeah, as I mentioned in my cover letter, I was on leave, so I wasn't
around for the big ZNS release day either. But, honestly, I think this
is irrelevant - code should be merged based on technical reasons (not
technicalities).
>
> Just a friendly suggestion, how about:
>
> 1) We get your
>
> hw/block/nvme: bump to v1.3
> hw/block/nvme: support scatter gather lists
> hw/block/nvme: support multiple namespaces
> hw/block/nvme: bump to v1.4
>
> patch series merged.
>
Blowing my own horn here, but yeah, it seems like everyone would like to
see this merged.
> 2) We get Dmitry's patch series merged.
>
> Shared 4:th) If there is any feature that you miss in Dmitry's patch series,
> perhaps you could send patches to add what you are missing.
>
Looks like the two version are pretty much on par in terms of features.
Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces
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.
Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces
On Tue, Jun 30, 2020 at 12:01:29PM +0200, Klaus Jensen wrote:
> From: Klaus Jensen
>
> Hi all,
Hello Klaus,
>
> This series adds support for TP 4056 ("Namespace Types") and TP 4053
> ("Zoned Namespaces") and is an alternative implementation to the one
> submitted by Dmitry[1].
>
> While I don't want this to end up as a discussion about the merits of
> each version, I want to point out a couple of differences from Dmitry's
> version. At a glance, my version
>
> * builds on my patch series that adds fairly complete NVMe v1.4
> mandatory support, as well as nice-to-have feature such as SGLs,
> multiple namespaces and mostly just overall clean up. This finally
> brings the nvme device into a fairly compliant state on which we can
> add new features. I've tried hard to get these compliance and
> clean-up patches merged for a long time (in parallel with developing
> the emulation of NST and ZNS) and I would be really sad to see them
> by-passed since they have already been through many iterations and
> already carries Acked- and Reviewed-by's for the bulk of the
> patches. I think the nvme device is already in a "frankenstate" wrt.
> the implemented nvme version and the features it currently supports,
> so I think this kind of cleanup is long overdue.
>
> * uses an attached blockdev and standard blk_aio for persistent zone
> info. This is the same method used in our patches for Write
> Uncorrectable and (separate and extended lba) metadata support, but
> I've left those optional features out for now to ease the review
> process.
>
> * relies on the universal dulbe support added in ("hw/block/nvme: add
> support for dulbe") and sparse images for handling reads in gaps
> (above write pointer and below ZSZE); that is - the size of the
> underlying blockdev is in terms of ZSZE, not ZCAP
>
> * the controller uses timers to autonomously finish zones (wrt. FRL)
AFAICT, Dmitry's patches does this as well.
>
> I've been on paternity leave for a month, so I havn't been around to
> review Dmitry's patches, but I have started that process now. I would
> also be happy to work with Dmitry & Friends on merging our versions to
> get the best of both worlds if it makes sense.
>
> This series and all preparatory patch sets (the ones I've been posting
> yesterday and today) are available on my GitHub[2]. Unfortunately
> Patchew got screwed up in the middle of me sending patches and it never
> picked up v2 of "hw/block/nvme: support multiple namespaces" because it
> was getting late and I made a mistake with the CC's. So my posted series
> don't apply according to Patchew, but they actually do if you follow the
> Based-on's (... or just grab [2]).
>
>
> [1]: Message-Id: <20200617213415.22417-1-dmitry.fomic...@wdc.com>
> [2]: https://github.com/birkelund/qemu/tree/for-master/nvme
>
>
> Based-on: <20200630043122.1307043-1-...@irrelevant.dk>
> ("[PATCH 0/3] hw/block/nvme: bump to v1.4")
Is this the only patch series that this series depends on?
In the beginning of the cover letter, you mentioned
"NVMe v1.4 mandatory support", "SGLs", "multiple namespaces",
and "and mostly just overall clean up".
>
> Klaus Jensen (10):
> hw/block/nvme: support I/O Command Sets
> hw/block/nvme: add zns specific fields and types
> hw/block/nvme: add basic read/write for zoned namespaces
> hw/block/nvme: add the zone management receive command
> hw/block/nvme: add the zone management send command
> hw/block/nvme: add the zone append command
> hw/block/nvme: track and enforce zone resources
> hw/block/nvme: allow open to close transitions by controller
> hw/block/nvme: allow zone excursions
> hw/block/nvme: support reset/finish recommended limits
>
> block/nvme.c |6 +-
> hw/block/nvme-ns.c| 397 +-
> hw/block/nvme-ns.h| 148 +++-
> hw/block/nvme.c | 1676 +++--
> hw/block/nvme.h | 76 +-
> hw/block/trace-events | 43 +-
> include/block/nvme.h | 252 ++-
> 7 files changed, 2469 insertions(+), 129 deletions(-)
>
> --
> 2.27.0
>
I think that you have done a great job getting the NVMe
driver out of a frankenstate, and made it compliant with
a proper spec (NVMe 1.4).
I'm also a big fan of the refactoring so that the driver
handles more than one namespace, and the new bus model.
I know that you first sent your
"nvme: support NVMe v1.3d, SGLs and multiple namespaces"
patch series July, last year.
Looking at your outstanding patch series on patchwork:
https://patchwork.kernel.org/project/qemu-devel/list/?submitter=188679
(Feel free to correct me if I have misunderstood anything.)
I see that these are related to your patch series from July last year:
hw/block/nvme: bump to v1.3
hw/block/nvme: support scatter gather lists
hw/block/nvme: support multiple namespaces
hw/block/nvme: bump to v1.4
This patch series seems minor and could probably
Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces
On 6/30/20 2:59 PM, Niklas Cassel wrote:
> On Tue, Jun 30, 2020 at 12:01:29PM +0200, Klaus Jensen wrote:
>> From: Klaus Jensen
>>
>> Hi all,
>
> Hello Klaus,
>
>>
>> This series adds support for TP 4056 ("Namespace Types") and TP 4053
>> ("Zoned Namespaces") and is an alternative implementation to the one
>> submitted by Dmitry[1].
>>
>> While I don't want this to end up as a discussion about the merits of
>> each version, I want to point out a couple of differences from Dmitry's
>> version. At a glance, my version
>>
>> * builds on my patch series that adds fairly complete NVMe v1.4
>> mandatory support, as well as nice-to-have feature such as SGLs,
>> multiple namespaces and mostly just overall clean up. This finally
>> brings the nvme device into a fairly compliant state on which we can
>> add new features. I've tried hard to get these compliance and
>> clean-up patches merged for a long time (in parallel with developing
>> the emulation of NST and ZNS) and I would be really sad to see them
>> by-passed since they have already been through many iterations and
>> already carries Acked- and Reviewed-by's for the bulk of the
>> patches. I think the nvme device is already in a "frankenstate" wrt.
>> the implemented nvme version and the features it currently supports,
>> so I think this kind of cleanup is long overdue.
>>
>> * uses an attached blockdev and standard blk_aio for persistent zone
>> info. This is the same method used in our patches for Write
>> Uncorrectable and (separate and extended lba) metadata support, but
>> I've left those optional features out for now to ease the review
>> process.
>>
>> * relies on the universal dulbe support added in ("hw/block/nvme: add
>> support for dulbe") and sparse images for handling reads in gaps
>> (above write pointer and below ZSZE); that is - the size of the
>> underlying blockdev is in terms of ZSZE, not ZCAP
>>
>> * the controller uses timers to autonomously finish zones (wrt. FRL)
>
> AFAICT, Dmitry's patches does this as well.
>
>>
>> I've been on paternity leave for a month, so I havn't been around to
>> review Dmitry's patches, but I have started that process now. I would
>> also be happy to work with Dmitry & Friends on merging our versions to
>> get the best of both worlds if it makes sense.
>>
>> This series and all preparatory patch sets (the ones I've been posting
>> yesterday and today) are available on my GitHub[2]. Unfortunately
>> Patchew got screwed up in the middle of me sending patches and it never
>> picked up v2 of "hw/block/nvme: support multiple namespaces" because it
>> was getting late and I made a mistake with the CC's. So my posted series
>> don't apply according to Patchew, but they actually do if you follow the
>> Based-on's (... or just grab [2]).
>>
>>
>> [1]: Message-Id: <20200617213415.22417-1-dmitry.fomic...@wdc.com>
>> [2]: https://github.com/birkelund/qemu/tree/for-master/nvme
>>
>>
>> Based-on: <20200630043122.1307043-1-...@irrelevant.dk>
>> ("[PATCH 0/3] hw/block/nvme: bump to v1.4")
>
> Is this the only patch series that this series depends on?
>
> In the beginning of the cover letter, you mentioned
> "NVMe v1.4 mandatory support", "SGLs", "multiple namespaces",
> and "and mostly just overall clean up".
>
>>
>> Klaus Jensen (10):
>> hw/block/nvme: support I/O Command Sets
>> hw/block/nvme: add zns specific fields and types
>> hw/block/nvme: add basic read/write for zoned namespaces
>> hw/block/nvme: add the zone management receive command
>> hw/block/nvme: add the zone management send command
>> hw/block/nvme: add the zone append command
>> hw/block/nvme: track and enforce zone resources
>> hw/block/nvme: allow open to close transitions by controller
>> hw/block/nvme: allow zone excursions
>> hw/block/nvme: support reset/finish recommended limits
>>
>> block/nvme.c |6 +-
>> hw/block/nvme-ns.c| 397 +-
>> hw/block/nvme-ns.h| 148 +++-
>> hw/block/nvme.c | 1676 +++--
>> hw/block/nvme.h | 76 +-
>> hw/block/trace-events | 43 +-
>> include/block/nvme.h | 252 ++-
>> 7 files changed, 2469 insertions(+), 129 deletions(-)
>>
>> --
>> 2.27.0
>>
>
> I think that you have done a great job getting the NVMe
> driver out of a frankenstate, and made it compliant with
> a proper spec (NVMe 1.4).
>
> I'm also a big fan of the refactoring so that the driver
> handles more than one namespace, and the new bus model.
>
> I know that you first sent your
> "nvme: support NVMe v1.3d, SGLs and multiple namespaces"
> patch series July, last year.
>
> Looking at your outstanding patch series on patchwork:
> https://patchwork.kernel.org/project/qemu-devel/list/?submitter=188679
>
> (Feel free to correct me if I have misunderstood anything.)
>
> I see that these are related to your patch series from July last year:
> hw/block/nvme: bump
