[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 16/16] nvme: support multiple namespaces
From: |
Ross Lagerwall |
Subject: |
Re: [Qemu-devel] [PATCH 16/16] nvme: support multiple namespaces |
Date: |
Mon, 4 Nov 2019 09:11:01 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
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