Re: [PATCH RFC v3 00/12] hw/block/nvme: metadata and end-to-end data protection support

[Klaus Jensen <its@irrelevant.dk>]

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

On Feb 16 16:19, Keith Busch wrote:
> On Mon, Feb 15, 2021 at 12:02:28AM +0100, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> > 
> > This is RFC v3 of a series that adds support for metadata and end-to-end data
> > protection.
> > 
> > First, on the subject of metadata, in v1, support was restricted to
> > extended logical blocks, which was pretty trivial to implement, but
> > required special initialization and broke DULBE. In v2, metadata is
> > always stored continuously at the end of the underlying block device.
> > This has the advantage of not breaking DULBE since the data blocks
> > remains aligned and allows bdrv_block_status to be used to determinate
> > allocation status. It comes at the expense of complicating the extended
> > LBA emulation, but on the other hand it also gains support for metadata
> > transfered as a separate buffer.
> > 
> > The end-to-end data protection support blew up in terms of required
> > changes. This is due to the fact that a bunch of new commands has been
> > added to the device since v1 (zone append, compare, copy), and they all
> > require various special handling for protection information. If
> > potential reviewers would like it split up into multiple patches, each
> > adding pi support to one command, shout out.
> > 
> > The core of the series (metadata and eedp) is preceeded by a set of
> > patches that refactors mapping (yes, again) and tries to deal with the
> > qsg/iov duality mess (maybe also again?).
> > 
> > Support fro metadata and end-to-end data protection is all joint work
> > with Gollu Appalanaidu.
> 
> Patches 1 - 8 look good to me:
> 
> Reviewed-by: Keith Busch <kbusch@kernel.org>
> 
> I like the LBA format and protection info support too, but might need
> some minor changes.
> 

Cool, thanks for the reviews Keith!

> The verify implementation looked fine, but lacking a generic backing for
> it sounds to me the use cases aren't there to justify taking on this
> feature.

Please check my reply on the verify patch - can you elaborate on
"generic backing"? I'm not sure I understand what you have in mind,
API-wise.

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