[PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces

Klaus Jensen posted 10 patches 3 years, 9 months ago
Failed in applying to current master (apply log)
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(-)
[PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces
Posted by Klaus Jensen 3 years, 9 months ago
From: Klaus Jensen <k.jensen@samsung.com>

Hi all,

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)

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.fomichev@wdc.com>
  [2]: https://github.com/birkelund/qemu/tree/for-master/nvme


Based-on: <20200630043122.1307043-1-its@irrelevant.dk>
("[PATCH 0/3] hw/block/nvme: bump to v1.4")

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


Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces
Posted by Niklas Cassel 3 years, 9 months ago
On Tue, Jun 30, 2020 at 12:01:29PM +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> 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.fomichev@wdc.com>
>   [2]: https://github.com/birkelund/qemu/tree/for-master/nvme
> 
> 
> Based-on: <20200630043122.1307043-1-its@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 be merged immediately:
hw/block/nvme: handle transient dma errors


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?)


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.


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.

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.

Shared 4:th) Your other patch series:
hw/block/nvme: AIO and address mapping refactoring could get merged.


Please don't take this suggestion the wrong way, I'm simply trying
to come up with a way to move forward from here.


Kind regards,
Niklas
Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces
Posted by Philippe Mathieu-Daudé 3 years, 9 months ago
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 <k.jensen@samsung.com>
>>
>> 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.fomichev@wdc.com>
>>   [2]: https://github.com/birkelund/qemu/tree/for-master/nvme
>>
>>
>> Based-on: <20200630043122.1307043-1-its@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 be merged immediately:
> hw/block/nvme: handle transient dma errors
> 
> 
> 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?)
> 
> 
> 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.
> 
> 
> 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.
> 
> 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.
> 
> Shared 4:th) Your other patch series:
> hw/block/nvme: AIO and address mapping refactoring could get merged.
> 
> 
> Please don't take this suggestion the wrong way, I'm simply trying
> to come up with a way to move forward from here.

Few months ago Klaus sent a bomb series with ~80 patches, we asked
him to split in digestible series of ~20 patches.

Earlier in this cover Klaus provided a link to his git repository
with all the patches sorted [2]:
https://github.com/birkelund/qemu/tree/for-master/nvme

This seems enough to get the big picture.

Niklas Cassel, it would be helpful if you or Dmitry can review
Klaus patches. I see Klaus is already reviewing Dmitry ones.

Both Keith and Kevin are quite busy recently.

To help them I suggest once you reviewed your patches each other,
one of you could send the big series with all patches together.

Anyway soft-freeze is next week, so you have to decide what is
critical.

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

[3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg718086.html

> 
> 
> Kind regards,
> Niklas
> 


Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces
Posted by Keith Busch 3 years, 9 months ago
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
Posted by Klaus Jensen 3 years, 9 months ago
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.

nvme emulation merge process (was: Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces)
Posted by Kevin Wolf 3 years, 9 months ago
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: nvme emulation merge process (was: Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces)
Posted by Klaus Jensen 3 years, 9 months ago
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.

Re: nvme emulation merge process (was: Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces)
Posted by Maxim Levitsky 3 years, 9 months ago
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)
Posted by Philippe Mathieu-Daudé 3 years, 9 months ago
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)
Posted by Keith Busch 3 years, 9 months ago
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
Posted by Andrzej Jakowski 3 years, 8 months ago
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.
>>
> 


Re: nvme emulation merge process
Posted by Keith Busch 3 years, 8 months ago
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: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces
Posted by Klaus Jensen 3 years, 9 months ago
On Jun 30 12:59, Niklas Cassel wrote:
> On Tue, Jun 30, 2020 at 12:01:29PM +0200, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> > 
> > 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.fomichev@wdc.com>
> >   [2]: https://github.com/birkelund/qemu/tree/for-master/nvme
> > 
> > 
> > Based-on: <20200630043122.1307043-1-its@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.

> Shared 4:th) Your other patch series:
> hw/block/nvme: AIO and address mapping refactoring could get merged.
> 

As stated above I think its only a single commit ("hw/block/nvme: allow
multiple aios per command") that is controversial in that series.

> 
> Please don't take this suggestion the wrong way, I'm simply trying
> to come up with a way to move forward from here.
> 

Absolutely - I totally get that you want to move forward with Dmitry's
series, but I'd like to finish my review before committing to anything.


Cheers,
Klaus

RE: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces
Posted by Dmitry Fomichev 3 years, 9 months ago

> -----Original Message-----
> From: Klaus Jensen <its@irrelevant.dk>
> Sent: Tuesday, June 30, 2020 4:30 PM
> To: Niklas Cassel <Niklas.Cassel@wdc.com>
> Cc: qemu-block@nongnu.org; Klaus Jensen <k.jensen@samsung.com>;
> qemu-devel@nongnu.org; Keith Busch <kbusch@kernel.org>; Max Reitz
> <mreitz@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Javier Gonzalez
> <javier.gonz@samsung.com>; Maxim Levitsky <mlevitsk@redhat.com>;
> Philippe Mathieu-Daudé <philmd@redhat.com>; Dmitry Fomichev
> <Dmitry.Fomichev@wdc.com>; Damien Le Moal
> <Damien.LeMoal@wdc.com>; Matias Bjorling <Matias.Bjorling@wdc.com>
> 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 <k.jensen@samsung.com>
> > >
> > > 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.fomichev@wdc.com>
> > >   [2]: https://github.com/birkelund/qemu/tree/for-master/nvme
> > >
> > >
> > > Based-on: <20200630043122.1307043-1-its@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.
> 
> > Shared 4:th) Your other patch series:
> > hw/block/nvme: AIO and address mapping refactoring could get merged.
> >
> 
> As stated above I think its only a single commit ("hw/block/nvme: allow
> multiple aios per command") that is controversial in that series.
> 
> >
> > Please don't take this suggestion the wrong way, I'm simply trying
> > to come up with a way to move forward from here.
> >
> 
> Absolutely - I totally get that you want to move forward with Dmitry's
> series, but I'd like to finish my review before committing to anything.
> 

Klaus,

I see that ZNS series from WDC is pretty much orthogonal to most of your
patches from "bump to 1.3/bump to 1.4" series... with a few notable
exceptions - I had to add support for Get Log Page command, Command
Effects Log and AERs to fulfill ZNS protocol requirements. I understand that
you've had that functionality already staged in your "bump to 1.3"
patchset and beyond and I don't insist that the implementations of these
features from our ZNS series are necessarily to be used. I think Niklas' plan
appears to be quite productive - for us to rebase on top of "bump to 1.3..1.4"
and review these patches in the process, apply our series and then
incorporate some of the ideas from your ZNS/NSTypes patchset on top of that.
This way, very little work will go to waste.

Cheers,
Dmitry

> 
> Cheers,
> Klaus