Re: [Qemu-devel] [PULL 00/47] virtio, pc: fixes and features

[Igor Mammedov]

On Tue, 1 Nov 2016 15:55:36 +0200
"Michael S. Tsirkin" <address@hidden> 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" <address@hidden> 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" <address@hidden> 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.