From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59356) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfBgK-00011P-7k for qemu-devel@nongnu.org; Thu, 14 Jun 2012 11:10:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SfBgD-0007lc-T4 for qemu-devel@nongnu.org; Thu, 14 Jun 2012 11:09:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37340) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfBgD-0007l5-Kj for qemu-devel@nongnu.org; Thu, 14 Jun 2012 11:09:49 -0400 Message-ID: <1339686587.24818.42.camel@ul30vt> From: Alex Williamson Date: Thu, 14 Jun 2012 09:09:47 -0600 In-Reply-To: <20120614145025.GA18126@redhat.com> References: <20120614044751.11034.87619.stgit@bling.home> <20120614045146.11034.95991.stgit@bling.home> <20120614102444.GC5778@redhat.com> <1339683699.24818.27.camel@ul30vt> <20120614145025.GA18126@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Mime-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v2 4/6] msix: Split PBA into it's own MemoryRegion List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: jan.kiszka@siemens.com, qemu-devel@nongnu.org On Thu, 2012-06-14 at 17:50 +0300, Michael S. Tsirkin wrote: > On Thu, Jun 14, 2012 at 08:21:39AM -0600, Alex Williamson wrote: > > On Thu, 2012-06-14 at 13:24 +0300, Michael S. Tsirkin wrote: > > > On Wed, Jun 13, 2012 at 10:51:47PM -0600, Alex Williamson wrote: > > > > These don't have to be contiguous. Size them to only what > > > > they need and use separate MemoryRegions for the vector > > > > table and PBA. > > > > > > > > Signed-off-by: Alex Williamson > > > > > > Why is this still using NATIVE? > > > > Because the bug already exists, > > We have lots of broken code. The way progress happens here is > such code is in a kind of freeze until fixed. This way whoever needs new > features gets to fix the bugs too. In other words, you impose a toll and inhibit forward progress until someone fixes it? I have no place telling you how to be a maintainer, but I personally find that this style makes attempting to contribute code to anything pci/msi/msix related a huge pain. There are far too many of these land mines in the code and simple fixes easily explode into tangentially related changes off your todo list. > > this patch doesn't make it worse, so at best it's a tangentially related additional fix. > > It may seem like a s/NATIVE/LITTLE/ to you, but to me it's asking to completely scrub > > msix.c for endian correctness. Is this going to be the carrot you hold > > out to accept the rest of the series? > > > > Alex > > Unfortunately no promises yet, and that is because you basically decided > to rewrite lots of code in your preferred style while also adding new functionality. > If changes were done in small steps, then I could apply things we can > agree on and defer the ones we don't. Sometimes it's hard, but clearly > not in this case. Patches can always be reduced into smaller changes, but at some point we have to call it good enough. I split one patch into 6 and thought that did a pretty good job. Should I remove everywhere that I've added a new line to avoid imposing my style on the rest of the code? The next version will eliminate the add_config move thanks to Jan's constructive suggestion, so I hope it meets your standards. Thanks, Alex