From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48511) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1ZXU-0001io-5h for qemu-devel@nongnu.org; Tue, 01 Nov 2016 09:55:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1ZXP-0007av-9w for qemu-devel@nongnu.org; Tue, 01 Nov 2016 09:55:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60406) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c1ZXP-0007an-4Y for qemu-devel@nongnu.org; Tue, 01 Nov 2016 09:55:39 -0400 Date: Tue, 1 Nov 2016 15:55:36 +0200 From: "Michael S. Tsirkin" Message-ID: <20161101155350-mutt-send-email-mst@kernel.org> References: <1477850917-1214-1-git-send-email-mst@redhat.com> <20161031105031.2ea5061d@nial.brq.redhat.com> <20161101004526-mutt-send-email-mst@kernel.org> <20161101142107.6bc0f5c5@nial.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161101142107.6bc0f5c5@nial.brq.redhat.com> Subject: Re: [Qemu-devel] [PULL 00/47] virtio, pc: fixes and features List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-devel@nongnu.org, Peter Maydell On Tue, Nov 01, 2016 at 02:21:07PM +0100, Igor Mammedov wrote: > On Tue, 1 Nov 2016 00:48:11 +0200 > "Michael S. Tsirkin" wrote: > > > On Mon, Oct 31, 2016 at 10:50:31AM +0100, Igor Mammedov wrote: > > > On Sun, 30 Oct 2016 23:23:18 +0200 > > > "Michael S. Tsirkin" wrote: > > > > > > > The following changes since commit 5b2ecabaeabc17f032197246c4846b9ba95ba8a6: > > > > > > > > Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20161028-1' into staging (2016-10-28 17:59:04 +0100) > > > > > > > > are available in the git repository at: > > > > > > > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream > > > > > > > > for you to fetch changes up to f082ec0225bd15c71e0b4697d2df3af7bad65d7f: > > > > > > > > acpi: fix assert failure caused by commit 35c5a52d (2016-10-30 20:06:25 +0200) > > > > > > > > ---------------------------------------------------------------- > > > > virtio, pc: fixes and features > > > > > > > > nvdimm hotplug support > > > Michael, > > > > > > Could you drop nvdimm hotplug from pull request (I should review at least once as it touches not only NVDIMMs but a generic hotplug infrastructure) > > > > > > and keep only nvdimm fixes/cleanups for now? > > > > If I drop it now it won't be in the next QEMU and it seems like > > a valuable feature. The comments so far are about minor style > > improvements that IMO can be addressed by patches on top. > wrt nvdimm hotplug support it's not about style issues but rather > design issues: for example: > - it extends general hotplug framework unnecessarily instead of > figuring out how it works. > - adds not needed locks > maybe there is more and all of that was posted just a day before > this pull request so I haven't even had a chance to review it properly. > > > > > We can always revert if you see bigger issues, but let's enable the > > testing of this feature. > if it didn't mess with general infrastructure, I wouldn't care much. > But it does so I'd rather avoid merging not yet ready series just for > the sake of getting it in. > > I haven't reviewed 28-35 patches either but they are all cleanups/ > fixes of current nvdimm code and local to it so don't mind them > getting merged. > > However I suggest dropping 36-39 patches from this pull request > as not yet ready for merging. So I think it's ok to keep them from now as that should help the feature converge faster, on the understanding the style issues are addressed quickly. Let's merge as is and I'll revert if this does not happen within two weeks. Ack? -- MST