qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Keith Busch
Subject: Re: [PATCH v3 01/15] hw/block/nvme: Define 64 bit cqe.result
Date: Fri, 18 Sep 2020 14:10:27 -0700

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 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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]