Re: [Qemu-devel] [PATCH 16/16] nvme: support multiple namespaces

[Ross Lagerwall]

On 11/4/19 9:04 AM, Klaus Birkelund wrote:
> On Mon, Nov 04, 2019 at 08:46:29AM +0000, Ross Lagerwall wrote:
>> On 8/23/19 9:10 AM, Klaus Birkelund wrote:
>>> On Thu, Aug 22, 2019 at 02:18:05PM +0100, Ross Lagerwall wrote:
>>>> On 7/5/19 8:23 AM, Klaus Birkelund Jensen wrote:
>>>>
>>>> I tried this patch series by installing Windows with a single NVME
>>>> controller having two namespaces. QEMU crashed in get_feature /
>>>> NVME_VOLATILE_WRITE_CACHE because req->ns was NULL.
>>>>
>>>
>>> Hi Ross,
>>>
>>> Good catch!
>>>
>>>> nvme_get_feature / nvme_set_feature look wrong to me since I can't see how
>>>> req->ns would have been set. Should they have similar code to nvme_io_cmd 
>>>> to
>>>> set req->ns from cmd->nsid?
>>>
>>> Definitely. I will fix that for v2.
>>>
>>>>
>>>> After working around this issue everything else seemed to be working well.
>>>> Thanks for your work on this patch series.
>>>>
>>>
>>> And thank you for trying out my patches!
>>>
>>
>> One more thing... it doesn't handle inactive namespaces properly so if you
>> have two namespaces with e.g. nsid=1 and nsid=3 QEMU ends up crashing in
>> certain situations. The patch below adds support for inactive namespaces.
>>
>> Still hoping to see a v2 some day :-)
>>
>  
> Hi Ross,
> 
> v2[1] is actually out, but only CC'ed Paul. Sorry about that! It fixes
> the support for discontiguous nsid's, but does not handle inactive
> namespaces correctly in identify.
> 
> I'll incorporate that in a v3 along with a couple of other fixes I did.
> 
> Thanks!
> 
> 
>   [1]: https://patchwork.kernel.org/cover/11190045/
> 

Thanks for pointing me at that, I missed it.

Regards,
-- 
Ross Lagerwall