Re: [PATCH v11 00/13] hw/block/nvme: Support Namespace Types and Zoned Namespace Command Set

[Klaus Jensen <its@irrelevant.dk>]

[-- Attachment #1: Type: text/plain, Size: 2839 bytes --]

On Dec 10 19:25, Dmitry Fomichev wrote:
> > -----Original Message-----
> > From: Klaus Jensen <its@irrelevant.dk>
> > Sent: Wednesday, December 9, 2020 4:58 AM
> > To: Dmitry Fomichev <Dmitry.Fomichev@wdc.com>
> > Cc: Keith Busch <kbusch@kernel.org>; Klaus Jensen
> > <k.jensen@samsung.com>; Kevin Wolf <kwolf@redhat.com>; Philippe
> > Mathieu-Daudé <philmd@redhat.com>; Max Reitz <mreitz@redhat.com>;
> > Maxim Levitsky <mlevitsk@redhat.com>; Fam Zheng <fam@euphon.net>;
> > Niklas Cassel <Niklas.Cassel@wdc.com>; Damien Le Moal
> > <Damien.LeMoal@wdc.com>; qemu-block@nongnu.org; qemu-
> > devel@nongnu.org; Alistair Francis <Alistair.Francis@wdc.com>; Matias
> > Bjorling <Matias.Bjorling@wdc.com>
> > Subject: Re: [PATCH v11 00/13] hw/block/nvme: Support Namespace Types
> > and Zoned Namespace Command Set
> > 
> > Hi Dmitry,
> > 
> > By and large, this looks OK to me. There are still some issues here and
> > there, and some comments of mine that you did not address, but I will
> > follow up with patches to fix that. Let's get this merged.
> > 
> > It looks like the nvme-next you rebased on is slightly old and missing
> > two commits:
> > 
> >   "hw/block/nvme: remove superfluous NvmeCtrl parameter" and
> >   "hw/block/nvme: pull aio error handling"
> > 
> > It caused a couple of conflicts, but nothing that I couldn't fix up.
> > 
> > Since I didn't manage to convince anyone about the zsze and zcap
> > parameters being in terms of LBAs, I'll revert that to be
> > 'zoned.zone_size' and 'zoned.zone_capacity'.
> > 
> > Finally, would you accept that we skip "hw/block/nvme: Add injection of
> > Offline/Read-Only zones" for now? I'd like to discuss it a bit since I
> > think the random injects feels a bit ad-hoc. Back when I did OCSSD
> > emulation with Hans, we did something like this for setting up state
> > through a descriptor text file - I think we should explore something
> > like that before we lock down the two parameters. I'll amend the final
> > documentation commit to not include those parameters.
> > 
> > Sounds good?
> 
> Klaus,
> 
> Sounds great! Sure, we can leave out the injection patch. It  was made
> to increase our internal test coverage, but it is not ideal. Since the zones
> are injected randomly, there is no consistency between test runs and
> it is impossible to reliably create many specific test cases (e.g. the first or
> the last zone is offline).

Yes, exactly.

> The descriptor input file seems like a much more
> flexible and capable approach. If you have something in works, I'll be
> happy to discuss or review.
> 

Sure, I'll rip some stuff from OCSSD and cook up a patch.

> Thank you for your very thorough reviews!
> 

Thanks for contributing this.

Keith, you wanna take a look an give this an Ack or so?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]