From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47957) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1ZUO-0008W0-4O for qemu-devel@nongnu.org; Tue, 01 Nov 2016 09:52:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1ZUJ-0005aP-9n for qemu-devel@nongnu.org; Tue, 01 Nov 2016 09:52:32 -0400 Received: from mga01.intel.com ([192.55.52.88]:49612) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c1ZUJ-0005aG-0p for qemu-devel@nongnu.org; Tue, 01 Nov 2016 09:52:27 -0400 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> From: Xiao Guangrong Message-ID: Date: Tue, 1 Nov 2016 21:45:06 +0800 MIME-Version: 1.0 In-Reply-To: <20161101142107.6bc0f5c5@nial.brq.redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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 , "Michael S. Tsirkin" Cc: Peter Maydell , qemu-devel@nongnu.org On 11/01/2016 09:21 PM, 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. Igor, Thank for your hard work to review this patchset. Only two patches related to general infrastructure that are: [PULL 37/47] nvdimm acpi: introduce fit buffer [PULL 39/47] pc: memhp: enable nvdimm device hotplug The issue you pointed out in [PULL 37/47] can be easily improved on the top of this patchset. Could you please look at the 39th, if there is no big issue, could it be merged first?