Re: [RFC PATCH V3 8/8] hw/block/nvme: Add Identify Active Namespace ID List

[Niklas Cassel <Niklas.Cassel@wdc.com>]

On Thu, Jan 21, 2021 at 06:58:19AM +0900, Minwoo Im wrote:
> > Hello Minwoo,
> > 
> > By introducing a detached parameter,
> > you are also implicitly making the following
> > NVMe commands no longer be spec compliant:
> > 
> > NVME_ID_CNS_NS, NVME_ID_CNS_CS_NS,
> > NVME_ID_CNS_NS_ACTIVE_LIST, NVME_ID_CNS_CS_NS_ACTIVE_LIST
> > 
> > When these commands are called on a detached namespace,
> > they should usually return a zero filled data struct.
> 
> Agreed.
> 
> > Dmitry and I had a patch on V8 on the ZNS series
> > that tried to fix some the existing NVMe commands
> > to be spec compliant, by handling detached namespaces
> > properly. In the end, in order to make it easier to
> > get the ZNS series accepted, we decided to drop the
> > detached related stuff from the series.
> > 
> > Feel free to look at that patch for inspiration:
> > https://github.com/dmitry-fomichev/qemu/commit/251c0ffee5149c739b1347811fa7e32a1c36bf7c
> 
> I've seen this patch and as Klaus said, only thing patches need go with
> is to put some of nvme_ns_is_attached() helper among the Identify
> handlers.
> 
> > I'm not sure if you want to modify all the functions that
> > our patch modifies, but I think that you should at least
> > modify the following nvme functions:
> > 
> > nvme_identify_ns()
> > nvme_identify_ns_csi()
> > nvme_identify_nslist()
> > nvme_identify_nslist_csi()
> 
> Yes, pretty makes sense to update them.  But now it seems like
> 'attach/detach' scheme should have been separated out of this series
> which just introduced the multi-path for controllers and namespace
> sharing.  I will drop this 'detach' scheme out of this series and make
> another series to support all of the Identify you mentioned above
> cleanly.

Hello Minwoo,

thank you for putting in work on this!

Kind regards,
Niklas