On Tue, 1 Nov 2016 15:55:36 +0200 "Michael S. Tsirkin" <m...@redhat.com> 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" <m...@redhat.com> 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" <m...@redhat.com> 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. PS: Looks like with new soft-freeze rules we just have one big hard-freeze window and people trying to frantically squeeze in features last minute. I think previous soft-freeze rules worked better where maintainers were still allowed merge features at their discretion if series would converge in soft-freeze time frame and be important/low risk one. Like the case here.
