RE: [PATCH v3 01/15] hw/block/nvme: Define 64 bit cqe.result

[Dmitry Fomichev]

> -----Original Message-----
> From: Keith Busch <kbusch@kernel.org>
> Sent: Friday, September 18, 2020 5:10 PM
> To: Klaus Jensen <its@irrelevant.dk>
> Cc: Dmitry Fomichev <Dmitry.Fomichev@wdc.com>; Fam Zheng
> <fam@euphon.net>; Kevin Wolf <kwolf@redhat.com>; Damien Le Moal
> <Damien.LeMoal@wdc.com>; qemu-block@nongnu.org; Niklas Cassel
> <Niklas.Cassel@wdc.com>; Klaus Jensen <k.jensen@samsung.com>; qemu-
> devel@nongnu.org; Alistair Francis <Alistair.Francis@wdc.com>; Philippe
> Mathieu-Daudé <philmd@redhat.com>; Matias Bjorling
> <Matias.Bjorling@wdc.com>
> Subject: Re: [PATCH v3 01/15] hw/block/nvme: Define 64 bit cqe.result
> 
> On Tue, Sep 15, 2020 at 10:48:49PM +0200, Klaus Jensen wrote:
> > On Sep 15 20:44, Dmitry Fomichev wrote:
> > >
> > > It is not necessary to change it in NST patch since result64 field is not 
> > > used
> > > in that patch. But if you insist, I can move it to NST patch :)
> >
> > You are right of course, but since it is a change to the "spec" related
> > data structures that go into include/block/nvme.h, I think it belongs in
> > "hw/block/nvme: Introduce the Namespace Types definitions".
> 
> Just my $.02, unless you're making exernal APIs, I really find it easier
> to review internal changes inline with the patches that use it.
> 
> Another example, there are patches in this series that introduce trace
> points, but the patch that use them come later. I find that harder to
> review since I need to look at two different patches to understand its
> value.
> 

I decided to split the trace events to be separate because I felt that it
could make the reviewing process simpler. Since the majority of the patches
in this series are on the large side, I thought it would be easier to open
them in separate sessions rather than to review a large single diff.
Let me know if you'd like me to fold the tracing stuff together with
the code...

DF

> I realize this particular patch is implementing a spec feature, but I'd
> prefer to see how it's used over of making a round trip to the spec.