[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Xiao Guangrong
Subject: Re: [Qemu-devel] [PULL 00/47] virtio, pc: fixes and features
Date: Wed, 2 Nov 2016 00:03:11 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0

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

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?
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!

reply via email to

[Prev in Thread] Current Thread [Next in Thread]