From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33924) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1bdu-0002iB-Pl for qemu-devel@nongnu.org; Tue, 01 Nov 2016 12:10:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1bdq-0000d1-K2 for qemu-devel@nongnu.org; Tue, 01 Nov 2016 12:10:30 -0400 Received: from mga05.intel.com ([192.55.52.43]:10317) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c1bdq-0000cU-CF for qemu-devel@nongnu.org; Tue, 01 Nov 2016 12:10:26 -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> <20161101155350-mutt-send-email-mst@kernel.org> <20161101151434.76180423@nial.brq.redhat.com> From: Xiao Guangrong Message-ID: <15fec71d-6a4b-a514-fa0b-a283173cc005@linux.intel.com> Date: Wed, 2 Nov 2016 00:03:11 +0800 MIME-Version: 1.0 In-Reply-To: <20161101151434.76180423@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 10:14 PM, Igor Mammedov wrote: > On Tue, 1 Nov 2016 15:55:36 +0200 > "Michael S. Tsirkin" wrote: > >> 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? > Ack, > let's merge and revert if author won't fix issues in 1-2 weeks. So happy to hear that. :) I will address your comment after Peter merges these patches. Thank you all!