[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH V3 8/8] hw/block/nvme: Add Identify Active Namespace ID L
From: |
Minwoo Im |
Subject: |
Re: [RFC PATCH V3 8/8] hw/block/nvme: Add Identify Active Namespace ID List |
Date: |
Thu, 21 Jan 2021 06:58:19 +0900 |
User-agent: |
Mutt/1.11.4 (2019-03-13) |
> 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.
> So they handle detached namespaces correctly for both:
> NVME_ID_CNS_NS, NVME_ID_CNS_CS_NS,
> NVME_ID_CNS_NS_ACTIVE_LIST, NVME_ID_CNS_CS_NS_ACTIVE_LIST
> as well as for:
> NVME_ID_CNS_NS_PRESENT, NVME_ID_CNS_CS_NS_PRESENT,
> NVME_ID_CNS_NS_PRESENT_LIST, NVME_ID_CNS_CS_NS_PRESENT_LIST
>
>
> Kind regards,
> Niklas
- [RFC PATCH V3 3/8] hw/block/nvme: add CMIC enum value for Identify Controller, (continued)
- [RFC PATCH V3 3/8] hw/block/nvme: add CMIC enum value for Identify Controller, Minwoo Im, 2021/01/19
- [RFC PATCH V3 4/8] hw/block/nvme: support for multi-controller in subsystem, Minwoo Im, 2021/01/19
- [RFC PATCH V3 5/8] hw/block/nvme: add NMIC enum value for Identify Namespace, Minwoo Im, 2021/01/19
- [RFC PATCH V3 7/8] hw/block/nvme: add 'detached' param not to attach namespace, Minwoo Im, 2021/01/19
- [RFC PATCH V3 6/8] hw/block/nvme: support for shared namespace in subsystem, Minwoo Im, 2021/01/19
- [RFC PATCH V3 8/8] hw/block/nvme: Add Identify Active Namespace ID List, Minwoo Im, 2021/01/19
- Re: [RFC PATCH V3 0/8] hw/block/nvme: support multi-path for ctrl/ns, Klaus Jensen, 2021/01/19