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

[Xiao Guangrong]

On 11/01/2016 10:14 PM, Igor Mammedov wrote:
> 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.

So happy to hear that. :)

I will address your comment after Peter merges these patches.

Thank you all!