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

[Xiao Guangrong <guangrong.xiao@linux.intel.com>]

On 11/01/2016 10:14 PM, Igor Mammedov wrote:
> On Tue, 1 Nov 2016 15:55:36 +0200
> "Michael S. Tsirkin" <mst@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" <mst@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" <mst@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.

So happy to hear that. :)

I will address your comment after Peter merges these patches.

Thank you all!