[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 16/16] nvme: support multiple namespaces
From: |
Klaus Birkelund |
Subject: |
Re: [Qemu-devel] [PATCH 16/16] nvme: support multiple namespaces |
Date: |
Mon, 4 Nov 2019 10:04:49 +0100 |
User-agent: |
Mutt/1.12.2 (2019-09-21) |
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/